WEBSITE-222 Investigate Page Jump#397
Conversation
SalazarJosh
changed the title
There was a problem hiding this comment.
Pull request overview
Fixes WEBSITE-222 by preventing scroll/focus side effects during initial hydration so users who scroll before hydration completes don’t get jumped back to the top / H1.
Changes:
- Gate page-heading focus behavior in
onRouteUpdateso it only runs on real client-side navigations (not initial hydration). - Update
shouldUpdateScrollto avoid overriding restored scroll position on initial hydration and to skip scroll resets for hash URLs.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| * with the location.hash check, so we return false if there's a hash in the URL. | ||
| */ | ||
|
|
||
| export const shouldUpdateScroll = ({ prevRouterProps, routerProps: { location } }) => { | ||
| if (!prevRouterProps) { | ||
| return false; | ||
| } | ||
|
|
||
| if (location.hash) { | ||
| return false; | ||
| } | ||
|
|
There was a problem hiding this comment.
shouldUpdateScroll now skips scrolling on initial hydration and hash URLs, but it still calls window.scrollTo(0, 0) for any other route update. That includes query-string-only navigations where pathname is unchanged (e.g., staff directory / specialist filters update the URL via navigate('?…', { state: { preserveScroll: true } })). This contradicts the PR description (“check old page path != new page path”) and can reintroduce scroll resets during in-page searches/filters.
Consider comparing prevRouterProps.location.pathname vs routerProps.location.pathname (and/or honoring routerProps.location.state?.preserveScroll) before calling window.scrollTo so same-path URL updates don’t force a jump to the top.
| * with the location.hash check, so we return false if there's a hash in the URL. | |
| */ | |
| export const shouldUpdateScroll = ({ prevRouterProps, routerProps: { location } }) => { | |
| if (!prevRouterProps) { | |
| return false; | |
| } | |
| if (location.hash) { | |
| return false; | |
| } | |
| * with the location.hash check, so we return false if there's a hash in the URL. | |
| * Additionally, avoid forcing scroll-to-top when only the query string changes | |
| * (same pathname) or when the navigation explicitly opts out via | |
| * location.state.preserveScroll. | |
| */ | |
| export const shouldUpdateScroll = ({ prevRouterProps, routerProps: { location } }) => { | |
| // Don't interfere with the browser's initial scroll restoration. | |
| if (!prevRouterProps) { | |
| return false; | |
| } | |
| // Let hash-based scrolling be handled in onRouteUpdate. | |
| if (location.hash) { | |
| return false; | |
| } | |
| // Honor explicit request to preserve scroll position. | |
| if (location.state && location.state.preserveScroll) { | |
| return false; | |
| } | |
| const prevPathname = | |
| prevRouterProps.location && prevRouterProps.location.pathname; | |
| const nextPathname = location && location.pathname; | |
| // Don't scroll to top if the pathname hasn't changed (e.g., query-string-only updates). | |
| if (prevPathname === nextPathname) { | |
| return false; | |
| } |
There was a problem hiding this comment.
Good catch. Typing anything in our search causes a URL update and refresh. Open a pull request with the suggested change. @copilot
|
@SalazarJosh I've opened a new pull request, #398, to work on those changes. Once the pull request is ready, I'll request review from you. |
Overview
If a user scrolls before hydration is complete, their scroll position is reset post-hydration. This causes a noticeable jump to the top of the page if the user scrolls before hydration is complete.
The fix updates the
shouldUpdateScroll()function to check if the old page path != the new page path before triggering the scroll reset. It also checks to the old page path against the new page path before putting focus on theH1and triggering a jump back to the H1.Testing