fix(docs): heading anchor links now include the hash so it doesnt scoll all the way up, follows navbar logic#5791
Conversation
…ll all the way up, follows navbar logic
|
@pullfrog review |
There was a problem hiding this comment.
Important
The one-line href fix is correct, but the PR also introduces an indentation regression on the commented-out code block (8 → 6 spaces). Ideally, all the dead commented-out code in this file should just be removed rather than reformatted.
Task list (5/5 completed)
- Read the PR diff
- Read the changed file in full for context
- Check the linked issue #5788
- Verify the fix matches existing TOC nav pattern
- Submit review
Claude Opus ｜ 𝕏
| data-card="" | ||
| // href={`#${props.id}`} | ||
| href={`?id=${props.id}`} | ||
| href={`?id=${props.id}#${props.id}`} |
There was a problem hiding this comment.
The fix is correct — this matches the pattern used by the TOC nav and ensures every click is a real navigation event.
| // shallow={true} | ||
| // onClick={(e) => { | ||
| // // function __handleScroll(){ | ||
| // // if id query parameter is present, scroll to the element with that id | ||
| // const params = new URLSearchParams(window.location.search); | ||
| // console.dir(params, { depth: null }); | ||
| // const id = params.get("id"); | ||
| // console.dir(params, { depth: null }); | ||
| // if (id) { | ||
| // console.dir(document.getElementById(id), { depth: null }); | ||
| // document.getElementById(id)?.scrollIntoView(); | ||
| // } | ||
| // // } | ||
| // }} |
There was a problem hiding this comment.
This block was indented at 8 spaces (aligned as <Link> props) and is now at 6. This is an unintentional formatting regression.
More broadly — this entire commented-out block (and the other stale comments on lines 21 and 23) are dead code that should be removed rather than carried forward. If you clean those up, the file becomes much simpler:
<Link
data-card=""
href={`?id=${props.id}#${props.id}`}
className="peer"
>|
I can get the indentation fixed! |
|
Thanks for the PR! Just wanted to flag that there's already a Going to close this out for now, but happy to revisit if there's a repro where the |
|
I appreciate your time reviewing this! |
|
Sorry, I understand the problem a little better now. Thanks for the PR! |
|
I'm so glad!! |
Fixes #5788
Problem
Clicking a heading (e.g. "Coercion") on a docs page like
/apisets theURL to
?id=coercion. Clicking the same heading a second time causes thepage to jump to the top instead of staying at the section.
This happens because the
<Link>only sets a query param (?id=coercion).When the URL hasn't changed, Next.js treats it as a no-op navigation —
no scroll happens, and the page snaps to top via scroll restoration.
The right-side TOC nav already uses
?id=coercion#coercion(both queryparam and hash), which works reliably on every click.
Fix
Add the hash to the heading anchor href to match what the TOC nav already does:
?id=${props.id}?id=${props.id}#${props.id}This means every click is always a real URL change (the hash is always
present), and the browser's native anchor scrolling handles positioning
correctly and idempotently.