fix: prevent nav hash duplication on repeated click → reload → click#118
Conversation
Plain `<Link href="/#section">` clicks appended the hash instead of replacing it when the URL already carried it (Next 16 App Router under output: "export" + trailingSlash), growing `/#method#method#method…` across click → reload → click cycles (#116). Intercept same-page anchor clicks on the desktop wordmark and section Links and resolve them locally with replaceState + smooth scroll, mirroring MobileMenu's handleClick. Modified clicks (new tab, middle-click) and cross-route clicks fall through to the native <Link>. Extract scrollToAnchor + its single-flight rAF state into shared lib/scroll.ts (now used by both the desktop nav and the mobile drawer) and add a pure homeHashSectionId helper with unit tests. Closes #116 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Fixes a hash-duplication bug (#116) where repeated click → reload → click on home-page nav links caused fragments like /#method#method#method. Intercepts same-page anchor <Link> clicks in Chrome.tsx and uses history.replaceState + a shared smooth-scroll helper, while extracting the existing scrollToAnchor from MobileMenu.tsx into a new shared lib/scroll.ts module.
Changes:
- New
lib/scroll.tswithhomeHashSectionId(pure URL parser) andscrollToAnchor(moved from MobileMenu, now module-level state shared across desktop nav + drawer). Chrome.tsxaddshandleNavClickthat bails on modified/cross-route clicks and otherwise prevents default, writes a single hash viareplaceState, and triggers the shared smooth scroll.- Unit tests for
homeHashSectionIdand MobileMenu refactored to import the shared helper.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| lib/scroll.ts | New module hosting shared scrollToAnchor state and homeHashSectionId href parser. |
| lib/scroll.test.ts | Vitest coverage for homeHashSectionId happy-path and null cases. |
| components/site/MobileMenu.tsx | Removes local copy of scrollToAnchor; imports from @/lib/scroll. |
| components/site/Chrome.tsx | Adds handleNavClick to wordmark + section Links to prevent hash accumulation. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Deploying website with
|
| Latest commit: |
c75db89
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://b49aaa17.website-70y.pages.dev |
| Branch Preview URL: | https://fix-116-nav-hash-duplication.website-70y.pages.dev |
There was a problem hiding this comment.
Code Review
This pull request refactors the scroll logic by moving the scrollToAnchor function to a shared utility file and introducing homeHashSectionId to handle same-page anchor navigation in the Chrome component. This change prevents hash accumulation in the URL during navigation cycles. Feedback was provided to improve the scroll animation by implementing quadratic easing and preserving the horizontal scroll position to ensure a smoother user experience.
| const tick = (now: number) => { | ||
| const t = Math.min(1, (now - startTime) / duration); | ||
| window.scrollTo(0, startY + distance * t); |
There was a problem hiding this comment.
The current implementation uses linear easing, which can feel mechanical compared to native browser smooth-scrolling. Additionally, it's best practice to preserve the current horizontal scroll position to prevent the page from jumping. Consider using an easing function (like quadratic ease-in-out) and window.scrollX for a more natural and robust feel.
const tick = (now: number) => {
const t = Math.min(1, (now - startTime) / duration);
const ease = t < 0.5 ? 2 * t * t : -1 + (4 - 2 * t) * t;
window.scrollTo(window.scrollX, startY + distance * ease);References
- Preserve the current scroll position when manipulating scroll state to prevent the page from jumping.
There was a problem hiding this comment.
Good call, but deferring it out of this PR: scrollToAnchor is moved byte-identical from MobileMenu.tsx into lib/scroll.ts here, and changing the easing/axis inside a pure move-refactor would obscure the diff and is unrelated to the #116 hash-accumulation fix. Tracked as #121 (quadratic ease-in-out + window.scrollX) to be reviewed on its own merits.
The #116 fix routed desktop nav clicks through scrollToAnchor, whose linear 600ms rAF (and forced scroll-behavior: auto) replaced the browser's native eased smooth scroll with a harsher constant-velocity animation. Use el.scrollIntoView({ block: "start" }) for the desktop path instead: with <html>'s motion-safe:scroll-smooth this is the browser's eased curve (the pre-fix feel), respects each section's scroll-mt, and auto-honours reduced-motion. The history.replaceState hash fix is unchanged. The mobile drawer keeps scrollToAnchor (its custom rAF is needed after the synchronous body-unlock). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* test(scroll): extract handleNavClick modifier guard Closes #120 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(scroll): ease scrollToAnchor and preserve horizontal scroll Closes #121 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(a11y): move focus to section on in-page nav jump Closes #119 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(scroll): read scrollX live instead of pinning a captured value Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(a11y): only set tabindex when target isn't already focusable Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(a11y): keep section focus on mobile anchor-close Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * docs(scroll): correct ease comment and clarify focus-pattern wording Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Summary
Fixes #116. On the home page, clicking the wordmark or a desktop nav section link, hard-reloading, then clicking the same link again grew the URL fragment (
/#method#method#method…) every cycle — invalid hash, no scroll on later loads, unshareable URL.Plain
next/link<Link href="/#section">clicks appended the hash instead of replacing it when the URL already carried it (Next 16 App Router underoutput: "export"+trailingSlash: true). The mobile drawer was unaffected because it already intercepts.Changes
lib/scroll.ts(new) —scrollToAnchor+ its single-flight rAF state moved verbatim out ofMobileMenu.tsx;MobileMenuis its only caller (desktop uses nativescrollIntoView). Adds pure, DOM-freehomeHashSectionId(href).lib/scroll.test.ts(new) — vitest unit tests forhomeHashSectionId, including the unsanitized multi-hash / trailing-slash contract (callers always pass a clean/#${id}).components/site/MobileMenu.tsx— removed the local copy, imports from@/lib/scroll; behaviour unchanged.components/site/Chrome.tsx—handleNavClickon the wordmark + section Links: bails on modified clicks (new tab / middle-click) so the native<Link>handles them; for same-page anchorspreventDefault+history.replaceState(null, "", "#"+id)(exact single hash — can't accumulate) + nativescrollIntoView; for cross-route anchors (section absent, e.g./blog/*)preventDefault+window.location.assign("/#"+id), a full reload so the destination resolves the hash deterministically — matching the mobile drawer's deliberate mitigation rather than re-entering the unreliable soft-nav hash path.replaceState(notpushState) is deliberate — a same-hash click should be a URL no-op, matching the existing MobileMenu precedent.Review remediation (post bot + multi-agent review)
el === nullnow does a full reload instead of silently falling through to the buggy native soft-nav (was an asymmetry vsMobileMenu.tsx).lib/scroll.ts(only the drawer callsscrollToAnchor).homeHashSectionId's unsanitized-input contract with assertions + a header note.Chrome.tsxcomments perAGENTS.md(dropped issue/caller refs and transitional phrasing).Tracked, out of scope here: #119 (focus/a11y parity on hash-anchor click, pre-existing), #120 (extract
shouldInterceptNavClickpure predicate), #121 (Gemini's quadratic easing +window.scrollXforscrollToAnchor).Verification
pnpm test— 86 passed (incl. the newhomeHashSectionIdcontract cases)pnpm lint— cleanpnpm format:check— cleanpnpm build— static export succeeded/blog/<slug>/a desktop nav/#methodclick lands on/at#methodwith a single clean hash; mobile drawer regression check.🤖 Generated with Claude Code