Skip to content

Conversation

@tompng
Copy link
Member

@tompng tompng commented Nov 17, 2025

Fix this unstable active table-of-contents link calculation shown in this video:
Clicking "Plugin types", active toc link will be "Option Parsing" or "File Parsing", depend on previous scroll position.
https://github.com/user-attachments/assets/bfd5c92a-9f11-4c9b-9267-86ae55607541

IntersectionObserver only notifies changed intersections.
We need to track which heading tag currently intersects with viewport. (I use Set in this pull request)
Use the top-most intersecting heading to make user-clicked toc links match with active toc links.

IntersectionObserver only notifies changed intersections.
We need to track which heading tag currently intersects with viewport.
Use the top-most intersecting heading to make user-clicked toc links match with active toc links.
@tompng tompng temporarily deployed to fork-preview-protection November 17, 2025 18:24 — with GitHub Actions Inactive
@matzbot
Copy link
Collaborator

matzbot commented Nov 17, 2025

🚀 Preview deployment available at: https://9a80f375.rdoc-6cd.pages.dev (commit: 5b079b8)

Copy link
Member

@st0012 st0012 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

Copy link
Member

@kou kou left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

return intersectingHeadings.has(heading);
});
if (!firstIntersectingHeading) return;
var correspondingLink = document.querySelector('.toc-link[data-target="' + firstIntersectingHeading.id + '"]');
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This may be simpler:

Suggested change
var correspondingLink = document.querySelector('.toc-link[data-target="' + firstIntersectingHeading.id + '"]');
var correspondingLink = document.querySelector(`.toc-link[data-target="${firstIntersectingHeading.id}"]`);

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I want to keep it for now.
Currently this file doesn't use ES6 feature: const let ()=>{} and `${}`
I think we can use newer syntaxes, but I think it's better to do it in a separate pull request.

Remove assignment to unused variable

Co-authored-by: Sutou Kouhei <kou@cozmixng.org>
@tompng tompng temporarily deployed to fork-preview-protection November 18, 2025 11:22 — with GitHub Actions Inactive
@tompng tompng merged commit 08123c7 into ruby:master Nov 18, 2025
32 of 33 checks passed
@tompng tompng deleted the stable_active_toc branch November 18, 2025 11:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants