feat(web): add scroll-spy active indicator to TOC#657
feat(web): add scroll-spy active indicator to TOC#657sebassg wants to merge 1 commit intonodejs:mainfrom
Conversation
Use IntersectionObserver to track the currently visible heading and highlight it in the MetaBar table of contents. Passes activeSlug via Context to a custom ActiveLink component rendered through the existing 'as' prop of MetaBar, requiring no changes to @node-core/ui-components.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Pull request overview
This PR implements a scroll-spy feature for the Table of Contents (TOC) in the MetaBar component. It uses IntersectionObserver to track which heading is currently visible in the viewport and highlights the corresponding TOC entry with a visual indicator (bold text, primary color, and a left border).
Changes:
- Added a
useScrollSpyhook that tracks the currently visible heading viaIntersectionObserverwith a 30% viewport detection zone - Created an
ActiveLinkcomponent that uses React Context to conditionally apply active styles to TOC links matching the visible heading - Added CSS styles and unit tests for the
getActiveSlughelper function
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
src/generators/web/ui/hooks/useScrollSpy.mjs |
New hook using IntersectionObserver to track the active heading slug |
src/generators/web/ui/hooks/__tests__/useScrollSpy.test.mjs |
Unit tests for the getActiveSlug pure function |
src/generators/web/ui/components/MetaBar/index.module.css |
New .tocActive CSS class for highlighting the active TOC item |
src/generators/web/ui/components/MetaBar/index.jsx |
Integrates scroll-spy via Context + ActiveLink component passed as as prop to upstream MetaBar |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
| border-left: 2px solid var(--color-primary, #0078d4); | ||
| padding-left: 0.25rem; |
There was a problem hiding this comment.
The .tocActive class adds border-left: 2px solid and padding-left: 0.25rem only to the active item. When the active heading changes during scrolling, the newly active link gains this border+padding while the previously active link loses it, causing a visible horizontal layout shift. Consider adding a transparent border (e.g., border-left: 2px solid transparent) and the same padding-left to the default/non-active link state so that only the border color changes on activation.
| border-left: 2px solid var(--color-primary, #0078d4); | |
| padding-left: 0.25rem; | |
| box-shadow: inset 2px 0 0 var(--color-primary, #0078d4); |
| margin-left: 0.25rem; | ||
| } | ||
|
|
||
| .tocActive { |
There was a problem hiding this comment.
that should be do in the ui-component package which is disponible on nodejs/nodejs.org repo
There was a problem hiding this comment.
Yes, thanks! I’ll first make the change in the ui-component package, and then update the implementation here afterward
There was a problem hiding this comment.
This is a tough one, team (cc @nodejs/nodejs-website) should this active ToC entry be implemented straight on node-core/ui-components? MetBar is customizable and not necessarily has a ToC entry, as it is simply an entry as any other on the MetaBar, although feels something that could be reused there.
There was a problem hiding this comment.
And if that's the case, @sebassg it'd mean this whole PR (or good chunk of it, like the effect, styles, etc) probably should land on ui-components package, although I'm unsure in which shape or format.
There was a problem hiding this comment.
On my side is was proposing to add new props to toc component
| <a | ||
| href={href} | ||
| aria-current={href === `#${activeSlug}` ? 'true' : undefined} | ||
| className={[className, href === `#${activeSlug}` ? styles.tocActive : ''] |
There was a problem hiding this comment.
nit: use the already bundled classNames package?
| * Anchor element that highlights itself when it matches the active TOC slug. | ||
| * @param {{ href: string, className?: string }} props | ||
| */ | ||
| const ActiveLink = ({ href, className, ...props }) => { |
There was a problem hiding this comment.
Do we really need a provider? Couldn't you just make the component ActiveLink defined within the defaultExport and then having it indirectly having the ref of the activeSlug?
const activeSlug = useScrollSpy(headings);
const ActiveLink = ({ href, className, ...props }) => {
...
component code
};
... remaining of MetaBar code
| * @returns {string | null} The slug of the active heading | ||
| */ | ||
| export const useScrollSpy = headings => { | ||
| const [activeSlug, setActiveSlug] = useState(null); |
There was a problem hiding this comment.
| const [activeSlug, setActiveSlug] = useState(null); | |
| const [activeSlug, setActiveSlug] = useState(); |
| entries => { | ||
| const slug = getActiveSlug(entries); | ||
|
|
||
| if (slug !== null) { |
There was a problem hiding this comment.
| if (slug !== null) { | |
| if (typeof activeSlug === 'string') { |
| setActiveSlug(slug); | ||
| } | ||
| }, | ||
| { rootMargin: '0px 0px -70% 0px', threshold: 0 } |
There was a problem hiding this comment.
could you add some inline comments // explaining these? Like why threshold 0 and what these values are supposed to mean for rootMargin? :)
| headings.forEach(({ slug }) => { | ||
| const el = document.getElementById(slug); | ||
|
|
||
| if (el) { |
There was a problem hiding this comment.
Any chance el wouldn't exist?
Simplify to;
observe.observe(el);
| }); | ||
|
|
||
| return () => observer.disconnect(); | ||
| }, [headings]); |
There was a problem hiding this comment.
If headings is an array, this effect will be recalculate every time the context calling this effect re-renders, because React doesn't do deep comparison of arrays/objects.
Description
Use IntersectionObserver to track the currently visible heading and highlight it in the MetaBar table of contents. Passes activeSlug via Context
to a custom ActiveLink component rendered through the existing 'as' prop of MetaBar, requiring no changes to @node-core/ui-components.
Validation
useScrollSpyhook:src/generators/web/ui/hooks/__tests__/useScrollSpy.test.mjsRelated Issues
Closes #529
Check List
that follow the guideline.
node --run testand all tests passed.node --run format&node --run lint.