USWDS - In-Page Navigation: Fix scrolling to nested headers #5878
+154
−3
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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, theoffsetTop
must be added to theparentOffset
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 thewindow.scroll
function withsinon
.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.