fix(#3726): highlight last TOC heading when scrolled to bottom#3727
fix(#3726): highlight last TOC heading when scrolled to bottom#3727chrisolsen merged 1 commit intodevfrom
Conversation
Spark450
left a comment
There was a problem hiding this comment.
I think it is certainly working better but it looks like the selection "bounces" a bit then settles.
https://jam.dev/c/08f2e673-e30e-4d7d-a609-a1a81e62637a
I think that's expected for what it's fixing and doing. This is a fix to make it change (override what's set from the scroll position) to the last item if scroll is at the bottom. It's kind of like, it follows the heading based on the scroll position, and then if the scroll is at the bottom, then set it to bottom as active. |
| // When scrolled to the bottom, activate the last visible heading | ||
| const atBottom = window.innerHeight + window.scrollY >= document.body.scrollHeight - 10; | ||
| if (atBottom) { | ||
| let lastVisibleId: string | null = null; | ||
| for (const heading of headings) { | ||
| if (!isVisible(heading)) continue; | ||
| const id = heading.getAttribute("id"); | ||
| if (id) lastVisibleId = id; | ||
| } | ||
| if (lastVisibleId) { | ||
| setActiveId(lastVisibleId); | ||
| return; | ||
| } | ||
| } |
There was a problem hiding this comment.
If you are scrolling to the bottom, is there any reason you can't just set the last of the headings as the current, rather than this logic + calculations? Is there ever a time that the last heading isn't visible when you scroll to the bottom of the screen?
const atBottom = window.innerHeight + window.scrollY >= document.body.scrollHeight - 10;
if (atBottom) {
const lastHeading = headings[headings.length - 1];
const lastVisibleId = lastHeading.getAttribute("id");
setActiveId(lastVisibleId);
return;
}
There was a problem hiding this comment.
Good point. The reason we need to filter is that headings here is all the matching headings in the DOM, not just the active tab. On component pages we have 4 tabs (Properties, Examples, Usage, Accessibility) and the headings from all of them are in the DOM at once. If we just grabbed the last one we'd always end up on a heading from the Accessibility tab no matter which tab the user is on.
You're right that we were doing more work than we needed to though. I refactored it to filter visible headings once at the top and both branches reuse that. It should be simpler now.
Spark450
left a comment
There was a problem hiding this comment.
If this approach is valid I think the behaviour is acceptable.
When the page is scrolled to the bottom, short sections near the end never reach the 120px threshold, so the TOC highlights the wrong item. Detect bottom-of-page and activate the last visible heading instead.
f8e1fb1 to
fcf3c5b
Compare
|
🎉 This PR is included in version 2.0.0-dev.6 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Summary
handleScroll: whenwindow.innerHeight + window.scrollY >= document.body.scrollHeight - 10, the last visible heading is set as active instead of relying on the 120px thresholdScreen.Recording.2026-03-31.at.3.07.21.PM.mov
Closes #3726
Test plan
https://claude.ai/code/session_01Rcc9hcmnjnE8vdXhiz7VMM