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

USWDS - In-Page Navigation: Fix scrolling to nested headers #5878

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

jhancock532
Copy link

@jhancock532 jhancock532 commented Apr 22, 2024

Summary

Fixed issue with in-page navigation being unable to scroll to nested headings. The in-page navigation can now smooth scroll to headings within the summary box and card components.

Breaking change

This is not a breaking change.

Related issue

Closes #5255

Related pull requests

N/A

Preview link

Preview link: N/A

Problem statement

The in-page navigation should navigate the user to the section of the page with the indicated heading when the heading link is clicked.

Currently, clicking on links that refer to nested headings, within the summary box or card components (or any other custom, non-standard USWDS component) causes the page to reset its scroll.

This poor user experience hinders users from accessing content they are seeking quickly and may mislead users to thinking desired content doesn't exist on the page. Erroneous behaviour reduces users trust in the site and USWDS branded sites as a whole.

Solution

The existing code uses the offsetTop value of the element to identify how much to scroll down the page. This value is relative to the parent container. To correctly scroll to the location of the element, the offsetTop must be added to the parentOffset of all the elements parent components. This MR adds a function to calculate the elements true offset in this manner.

An alternative solution would be to refactor the code to use scrollIntoView instead. This would involve refactoring the approach we use to test this component, which is being supported using a mocked offsetTop value and watching the window.scroll function with sinon.

Major changes

No major changes to component implementation / testing approach made.

Testing and review

A new Storybook story in the in-page navigation component has been created for testing. If running storybook locally, go to http://localhost:6006/iframe.html?args=&id=components-in-page-navigation--test-nested-headers&viewMode=story and click on each link in the in-page navigation. The page should correctly scroll to each heading in turn.

Any feedback on the solution would be helpful. I would like to add a test case to cover this fix, but I'm not sure how I'd go about mocking offsetParent only for certain elements. It also feels like if I was to mock this for specific contexts, the test would be verifying a very specific scenario instead of being representative. Using a testing framework such as Playwright instead of Jest would be helpful here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

USWDS - Bug: In-Page Navigation scrolling does not work with Summary Box
1 participant