Fix fragment navigation and focus handling in client router - #231
Merged
Conversation
…rgets Clicking `<a href="#section">` fires popstate for a brand new history entry rather than a traversal. The router read every popstate as a traversal and restored the saved scroll position, scrolling the page straight back from the fragment the browser had just jumped to — so in-page anchors and skip links appeared not to work. Tell the two apart by the scroll key the router stamps into history.state for every entry it creates: a keyless entry whose path and query are unchanged is a fragment navigation, so the browser's own jump is left to stand. Requiring the path to match as well means an entry whose state was wiped by app code (a stray `history.replaceState(null, …)`) is still re-resolved when the route really did change. A traversal onto an entry with no saved position now falls back to the URL's fragment instead of hard-resetting to the top. Wherever the router scrolls to a fragment itself, it now also moves focus to the target, adding a temporary `tabindex="-1"` for elements that are not natively focusable and removing it again on blur. Chromium sets the sequential focus navigation starting point on its own for a native fragment navigation, but not when the router scrolls, and Safari has historically not set it at all. Without this a skip link scrolled but left the next Tab stop at the top of the page. `scrollIntoView()` is still called with no `behavior` option so a CSS `prefers-reduced-motion` rule can turn a smooth scroll off. The bug only manifests when the popstate-triggered navigate() is async: on a route with no loader the router's scroll reset lands before the browser's jump and is overwritten. The e2e fixture route therefore has a loader, without which the regression is invisible. Also unregister router event listeners between router-navigation tests — without it, routers from earlier tests stay subscribed to the shared jsdom window and answer events a later test dispatches. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WsGLt6Ro49zViRVddW1tHj
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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
The client router was undoing in-page fragment scrolls and not moving focus to fragment targets, breaking skip links and in-page anchors.
Root cause: Clicking
<a href="#section">firespopstatefor a brand new history entry, not a traversal. The router treated everypopstateas a back/forward navigation and restored the saved scroll position, which scrolled the page back from the fragment the browser had just jumped to.Solution: The router now distinguishes fragment navigations from traversals by checking if the scroll key (stamped into
history.statefor every entry the router creates) is missing AND the path/query are unchanged. When detected, the browser's own jump is left to stand.Additionally, when the router scrolls to a fragment itself (client-side navigation or traversal with no saved position), it now moves focus to the target by:
tabindex="-1"for non-focusable elements (headings, landmarks)tabindexattributesfocus({ preventScroll: true })to avoid interrupting CSSscroll-behaviorThis restores the sequential focus navigation starting point, which is essential for skip links to work correctly.
Changes:
fragment-navigation.tsmodule with helpers:decodeFragmentId(),findFragmentTarget(),focusFragmentTarget(),scrollToFragmentTarget()router.tsto detect and handle fragment navigations, and to focus fragment targets/fragmenttest route with skip linkTesting
pnpm e2e— New e2e tests verify fragment link scrolling, focus movement, and back navigationpnpm test— New unit tests for fragment navigation helpers and router popstate handlingpnpm formatpnpm lintChecklist
https://claude.ai/code/session_01WsGLt6Ro49zViRVddW1tHj