fix(toc): Track hash in state to fix browser back navigation scrolling#18628
Draft
jaffrepaul wants to merge 7 commits into
Draft
fix(toc): Track hash in state to fix browser back navigation scrolling#18628jaffrepaul wants to merge 7 commits into
jaffrepaul wants to merge 7 commits into
Conversation
The TableOfContents component uses a ref to track whether it has scrolled to a hash anchor, preventing duplicate scrolls after the TOC renders. However, this flag was never reset, causing issues with browser back/forward navigation. When clicking back to restore a URL with a hash anchor, the browser correctly updated the URL but the page didn't scroll to the anchor section because the component thought it had already handled that scroll. Add a hashchange event listener that resets the scroll flag when the hash changes, allowing the component to properly handle browser back/forward navigation and other hash changes. Co-Authored-By: Claude <noreply@anthropic.com> Co-authored-by: Paul Jaffre <jaffrepaul@gmail.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Refactor the hash scroll logic to use React state instead of refs and event listeners. This is more idiomatic and maintainable: - Replace hasScrolledToHash ref with currentHash state - Remove lastHash ref tracking - Let React's dependency system handle re-execution when hash changes - Simplify the logic by eliminating manual flag management The effect now automatically re-runs when either the hash or treeItems change, making the behavior more predictable and easier to understand. Co-Authored-By: Claude <noreply@anthropic.com> Co-authored-by: Paul Jaffre <jaffrepaul@gmail.com>
Add cleanup function to cancel pending requestAnimationFrame callbacks when the effect re-runs. This prevents multiple scroll operations from queuing up if users rapidly click through TOC links or navigate quickly. The cleanup ensures only the latest scroll request executes, avoiding potential jank from competing scroll operations. Co-Authored-By: Claude <noreply@anthropic.com> Co-authored-by: Paul Jaffre <jaffrepaul@gmail.com>
Explicitly return undefined in early exit path to satisfy the consistent-return ESLint rule. Both branches of the useEffect now have explicit return statements. Co-Authored-By: Claude <noreply@anthropic.com> Co-authored-by: Paul Jaffre <jaffrepaul@gmail.com>
cursor
Bot
changed the title
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
Add comment explaining why we accept a redundant scroll when clicking TOC links. The Sentry bot flagged this as a potential performance issue, but attempts to optimize it by only scrolling on popstate events broke browser back/forward navigation. The redundant scroll (browser native + our effect) has negligible performance impact and ensures correct behavior for: - Initial page loads with hash (corrects layout shift) - Browser back/forward navigation (corrects scroll position) The alternative (only scrolling on popstate) broke core functionality, making this an acceptable tradeoff. Co-Authored-By: Claude <noreply@anthropic.com> Co-authored-by: Paul Jaffre <jaffrepaul@gmail.com>
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
BugBot identified that parent re-renders can cause unwanted scrolls: - Parent re-renders → new ignoreIds array reference - TOC rebuild effect runs → treeItems state changes - Scroll effect triggers → user yanked back to hash anchor Fix: Track which hash we've scrolled to via ref. When treeItems rebuild but hash hasn't changed, skip the scroll. Reset the ref on hash changes to allow scrolling for navigation/back/forward. This prevents users from being pulled back to anchors while reading, while still supporting: - Initial page load scroll correction - Browser back/forward navigation - TOC link clicks Co-Authored-By: Claude <noreply@anthropic.com> Co-authored-by: Paul Jaffre <jaffrepaul@gmail.com>
Contributor
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 6361092. Configure here.
Fix race condition identified by Bugbot: scrolledHashRef was set before the RAF executed, so if a re-render cancelled the RAF, the ref would incorrectly indicate the scroll happened when it didn't. This caused scrolls to be silently skipped. Move the ref assignment inside the RAF callback to ensure it's only marked as scrolled when the scroll actually executes. Fixes: High severity Bugbot issue (race condition) Co-Authored-By: Claude <noreply@anthropic.com> Co-authored-by: Paul Jaffre <jaffrepaul@gmail.com>
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.
DESCRIBE YOUR PR
This PR fixes a browser navigation bug where clicking the back button would restore the URL hash but not scroll to the correct anchor position on the page.
Root Cause:
Commit
8a810dae3(May 18, 2026) by Shannon Anahata added ahasScrolledToHashref to fix anchor links opened in new tabs landing on the wrong section. While that fix worked for its intended purpose, the ref was never reset during the page lifecycle, causing issues with browser back/forward navigation. When users clicked back to return to a URL with a hash anchor, the browser correctly updated the URL but the page didn't scroll because the component thought it had already handled that scroll.The Solution:
Instead of using refs and manually managing scroll flags, the fix uses React state to track the current hash. This leverages React's dependency system - when the hash changes (via browser back/forward or any other navigation), the effect automatically re-runs and scrolls to the correct section.
Additional Fixes from BugBot Analysis:
Redundant scroll on TOC clicks: BugBot correctly identified that clicking TOC links causes the browser to scroll, then our effect scrolls again. We accept this tradeoff because:
scrollIntoView()call)Unwanted scrolls during re-renders: BugBot identified that parent re-renders creating new
ignoreIdsarray references would trigger TOC rebuilds, causing users to be yanked back to hash anchors while reading. Fixed by:Testing:
Manually tested:
/product/ai-in-sentry/seer/autofix/#handoff-to-coding-agents✅ Scrolls correctlyAfter clicking back button - correctly scrolled to anchor
IS YOUR CHANGE URGENT?
PRE-MERGE CHECKLIST
To show artifacts inline, enable in settings.
Slack Thread