New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: refactor TOC highlighting + handle edge cases #5361
Conversation
✔️ [V2] 🔨 Explore the source changes: 203652a 🔍 Inspect the deploy log: https://app.netlify.com/sites/docusaurus-2/deploys/6117ea95fea15f0007acc3d3 😎 Browse the preview: https://deploy-preview-5361--docusaurus-2.netlify.app |
⚡️ Lighthouse report for the changes in this PR:
Lighthouse ran on https://deploy-preview-5361--docusaurus-2.netlify.app/ |
Size Change: +308 B (0%) Total Size: 803 kB
ℹ️ View Unchanged
|
} | ||
|
||
function getAnchors() { | ||
// For toc highlighting, we only consider h2/h3 anchors |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the future, we'll make headings levels displayed in TOC customizable, so maybe now we can avoid hard-coded h2/h3 only?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
too late :D
Agree that this should be configurable, but let's see how to add this in another PR like #4310
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think anchors created in TOC should have an extra class like anchor__toc
or something, because otherwise we'd still get weird behaviors if user render headings with JSX/React and they don't appear in the TOC
Motivation
The TOC highlighting hook was quite messy and hard to understand
This is a total refactor with logic that is easier to reason about, and also solving some edge cases
Fix #5318 where the first anchor can highlighted even if it's way below the viewport.
Fix highlighting bugs due to not ignoring h4/h5/h6 headings
Fix unnecessary usage of state, leading to useless React re-renders on scroll
Have you read the Contributing Guidelines on pull requests?
yes
Test Plan
preview