Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix AnchorLinks last link not mark as active if target is at end of page #316

Merged
merged 3 commits into from
Dec 31, 2024

Conversation

juancwu
Copy link
Contributor

@juancwu juancwu commented Dec 30, 2024

Description

This PR fixes the issue where the last link is never marked as active if the target element is right at the end of the page. The abnormal behaviour was that the second to last link is always the one marked as active because the AnchorLinks component chooses the closest one to the top when multiple targets are in view.

Linked Issues

Testing

  • Add new AnchorLinks component test components/test/AnchorLinks.test.tsx
  • The test only test for the existence of the links but not able to test scroll since jsDom does not implement layouts.

Checklist

Before opening this PR, make sure the PR:

  • Has an assignee or group of assignees.
  • Has a reviewer or a group of reviewers.
  • Is labelled properly.
  • Has the SPUR project assigned to it.
  • Has an assigned milestone.

Additionally, make sure that:

  • I have commented my code, particularly in hard-to-understand areas.
  • I have made corresponding changes to the documentation.
  • My changes generate no new warnings.
  • I have added tests that prove my fix is effective or that my feature works.
  • New and existing unit tests pass locally with my changes.
  • Any dependent changes have been merged and published in downstream modules.

@juancwu juancwu added bug Something isn't working frontend Related to the frontend of the project labels Dec 30, 2024
@juancwu juancwu added this to the Complete the project MVP milestone Dec 30, 2024
@juancwu juancwu self-assigned this Dec 30, 2024
Copy link
Member

@AmirAgassi AmirAgassi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@juancwu juancwu merged commit 319b0a4 into main Dec 31, 2024
4 checks passed
@juancwu juancwu deleted the fix/161/anchor-link-last-element-not-active branch December 31, 2024 20:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working frontend Related to the frontend of the project
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

[FE Bug] AnchorLinks does not set last link as active when last element is small in height
2 participants