Fix repeat fragment link clicks doing nothing - #233
Merged
Conversation
…hem to the browser Clicking a fragment link a second time was a dead click: the target stayed where it was and the page snapped back to wherever it had been scrolled. Fragment links were left to the browser, which works exactly once. The browser pushes an entry with no scroll key, the router recognizes the popstate that follows as a fragment navigation rather than a traversal — and stamps a scroll key onto that entry on its way through. A repeat click reuses the entry, so the key is now present, the popstate reads as a back/forward traversal, and saveScrollPosition() at the top of the handler has already recorded the current position under that key. restoreOrResetScroll then faithfully restores it. popstate alone cannot separate "pushed a fragment entry" from "traversed to a fragment entry" — the Navigation API's navigationType would, but it is not universally available. So the click handler now owns fragment links: it pushes the entry, scrolls to the target and moves focus there, which makes a repeat click scroll every time and leaves popstate to mean "traversal", which is what the scroll-key logic already assumes. The popstate guard stays as the fallback for fragment entries created another way (location.hash = "…"). Along the way: - A bare `#fragment` href now resolves against the full current URL. Resolving against the origin alone dropped the path, which the previous early return had hidden. - Fragment links are handled before the prerender-speculation check, since no document is fetched and there is nothing to activate. - `hashchange` is dispatched for the intercepted navigation, because pushState fires none and app listeners would otherwise stop hearing about it. - An unmatched fragment now matches the browser: top of document only for the empty fragment and the legacy `#top`, no scroll otherwise. - `data-pracht-preserve-scroll` on a fragment link updates the URL without moving the viewport. Covered by jsdom tests for the click path (including the repeat click, the history-entry count and hashchange) and by an e2e test that reproduces the reported sequence in a real browser — verified failing against the pre-fix build. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013UFWjaW9edaC5UZeNgbaMH
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
Fragment links were left to the browser, which works once: the browser pushes an entry with no scroll key, and the router recognizes the
popstatethat follows as a fragment navigation rather than a traversal — but stamps a scroll key onto that entry in the process. Clicking the same link again reuses the entry, so the key is now there, thepopstatereads as a back/forward traversal, and the position saved for the entry is faithfully restored. The click was dead.The client router now commits fragment link clicks itself — pushing the history entry, scrolling to the target, and moving focus there — so a repeat click always scrolls, and
popstateis left to mean "traversal", which is what the scroll-key logic assumes.hashchangeis dispatched for the intercepted navigation, sincepushStatefires none. Thepopstateguard stays as the fallback for fragment entries created another way (location.hash = "…").Testing
pnpm e2e— Added e2e test for repeat fragment link clickspnpm formatpnpm lintpnpm test— Added comprehensive unit tests covering:hashchangefires for first click but not repeat clickspreserveScrollattribute skips fragment scrollingChecklist
https://claude.ai/code/session_013UFWjaW9edaC5UZeNgbaMH