-
Notifications
You must be signed in to change notification settings - Fork 28
feat(web): add scroll-spy active indicator to TOC #657
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,9 +1,12 @@ | ||
| /* eslint-disable react-x/no-use-context, react-x/no-context-provider -- preact/compat does not export `use`, so React 19 patterns are unavailable at runtime */ | ||
| import { CodeBracketIcon, DocumentIcon } from '@heroicons/react/24/outline'; | ||
| import Badge from '@node-core/ui-components/Common/Badge'; | ||
| import MetaBar from '@node-core/ui-components/Containers/MetaBar'; | ||
| import GitHubIcon from '@node-core/ui-components/Icons/Social/GitHub'; | ||
| import { createContext, useContext } from 'react'; | ||
|
|
||
| import styles from './index.module.css'; | ||
| import { useScrollSpy } from '../../hooks/useScrollSpy.mjs'; | ||
|
|
||
| const iconMap = { | ||
| JSON: CodeBracketIcon, | ||
|
|
@@ -23,6 +26,8 @@ const STABILITY_KINDS = ['error', 'warning', null, 'info']; | |
| const STABILITY_LABELS = ['D', 'E', null, 'L']; | ||
| const STABILITY_TOOLTIPS = ['Deprecated', 'Experimental', null, 'Legacy']; | ||
|
|
||
| const ActiveSlugContext = createContext(null); | ||
|
|
||
| /** | ||
| * Renders a heading value with an optional stability badge | ||
| * @param {{ value: string, stability: number }} props | ||
|
|
@@ -54,6 +59,25 @@ const HeadingValue = ({ value, stability }) => { | |
| ); | ||
| }; | ||
|
|
||
| /** | ||
| * Anchor element that highlights itself when it matches the active TOC slug. | ||
| * @param {{ href: string, className?: string }} props | ||
| */ | ||
| const ActiveLink = ({ href, className, ...props }) => { | ||
| const activeSlug = useContext(ActiveSlugContext); | ||
|
|
||
| return ( | ||
| <a | ||
| href={href} | ||
| aria-current={href === `#${activeSlug}` ? 'true' : undefined} | ||
| className={[className, href === `#${activeSlug}` ? styles.tocActive : ''] | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: use the already bundled classNames package? |
||
| .filter(Boolean) | ||
| .join(' ')} | ||
| {...props} | ||
| /> | ||
| ); | ||
| }; | ||
|
|
||
| /** | ||
| * MetaBar component that displays table of contents and page metadata | ||
| * @param {MetaBarProps} props - Component props | ||
|
|
@@ -64,42 +88,49 @@ export default ({ | |
| readingTime, | ||
| viewAs = [], | ||
| editThisPage, | ||
| }) => ( | ||
| <MetaBar | ||
| heading="Table of Contents" | ||
| headings={{ | ||
| items: headings.map(({ value, stability, ...heading }) => ({ | ||
| ...heading, | ||
| value: <HeadingValue value={value} stability={stability} />, | ||
| })), | ||
| }} | ||
| items={{ | ||
| 'Reading Time': readingTime, | ||
| 'Added In': addedIn, | ||
| 'View As': ( | ||
| <ol> | ||
| {viewAs.map(([title, path]) => { | ||
| const Icon = iconMap[title]; | ||
|
|
||
| return ( | ||
| <li key={title}> | ||
| <a href={path}> | ||
| {Icon && <Icon className={styles.icon} />} | ||
|
|
||
| {title} | ||
| </a> | ||
| </li> | ||
| ); | ||
| })} | ||
| </ol> | ||
| ), | ||
| Contribute: ( | ||
| <> | ||
| <GitHubIcon className="fill-neutral-700 dark:fill-neutral-100" /> | ||
|
|
||
| <a href={editThisPage}>Edit this page</a> | ||
| </> | ||
| ), | ||
| }} | ||
| /> | ||
| ); | ||
| }) => { | ||
| const activeSlug = useScrollSpy(headings); | ||
|
|
||
| return ( | ||
| <ActiveSlugContext.Provider value={activeSlug}> | ||
| <MetaBar | ||
| heading="Table of Contents" | ||
| as={ActiveLink} | ||
| headings={{ | ||
| items: headings.map(({ value, stability, ...heading }) => ({ | ||
| ...heading, | ||
| value: <HeadingValue value={value} stability={stability} />, | ||
| })), | ||
| }} | ||
| items={{ | ||
| 'Reading Time': readingTime, | ||
| 'Added In': addedIn, | ||
| 'View As': ( | ||
| <ol> | ||
| {viewAs.map(([title, path]) => { | ||
| const Icon = iconMap[title]; | ||
|
|
||
| return ( | ||
| <li key={title}> | ||
| <a href={path}> | ||
| {Icon && <Icon className={styles.icon} />} | ||
|
|
||
| {title} | ||
| </a> | ||
| </li> | ||
| ); | ||
| })} | ||
| </ol> | ||
| ), | ||
| Contribute: ( | ||
| <> | ||
| <GitHubIcon className="fill-neutral-700 dark:fill-neutral-100" /> | ||
|
|
||
| <a href={editThisPage}>Edit this page</a> | ||
| </> | ||
| ), | ||
| }} | ||
| /> | ||
| </ActiveSlugContext.Provider> | ||
| ); | ||
| }; | ||
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
|
|
@@ -9,3 +9,10 @@ | |||||||
| display: inline-block; | ||||||||
| margin-left: 0.25rem; | ||||||||
| } | ||||||||
|
|
||||||||
| .tocActive { | ||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. that should be do in the ui-component package which is disponible on
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, thanks! I’ll first make the change in the ui-component package, and then update the implementation here afterward
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. On my side is was proposing to add new props to toc component |
||||||||
| color: var(--color-primary, #0078d4); | ||||||||
| font-weight: 600; | ||||||||
| border-left: 2px solid var(--color-primary, #0078d4); | ||||||||
| padding-left: 0.25rem; | ||||||||
|
Comment on lines
+16
to
+17
|
||||||||
| border-left: 2px solid var(--color-primary, #0078d4); | |
| padding-left: 0.25rem; | |
| box-shadow: inset 2px 0 0 var(--color-primary, #0078d4); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,38 @@ | ||
| import { strictEqual } from 'node:assert'; | ||
| import { describe, it } from 'node:test'; | ||
|
|
||
| import { getActiveSlug } from '../useScrollSpy.mjs'; | ||
|
|
||
| describe('getActiveSlug', () => { | ||
| it('should return null for an empty entries array', () => { | ||
| strictEqual(getActiveSlug([]), null); | ||
| }); | ||
|
|
||
| it('should return null when no entry is intersecting', () => { | ||
| const entries = [ | ||
| { isIntersecting: false, target: { id: 'intro' } }, | ||
| { isIntersecting: false, target: { id: 'usage' } }, | ||
| ]; | ||
|
|
||
| strictEqual(getActiveSlug(entries), null); | ||
| }); | ||
|
|
||
| it('should return the id of the first intersecting entry', () => { | ||
| const entries = [ | ||
| { isIntersecting: false, target: { id: 'intro' } }, | ||
| { isIntersecting: true, target: { id: 'usage' } }, | ||
| { isIntersecting: true, target: { id: 'api' } }, | ||
| ]; | ||
|
|
||
| strictEqual(getActiveSlug(entries), 'usage'); | ||
| }); | ||
|
|
||
| it('should return the id when only one entry is intersecting', () => { | ||
| const entries = [ | ||
| { isIntersecting: false, target: { id: 'intro' } }, | ||
| { isIntersecting: true, target: { id: 'config' } }, | ||
| ]; | ||
|
|
||
| strictEqual(getActiveSlug(entries), 'config'); | ||
| }); | ||
| }); |
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,49 @@ | ||||||
| import { useState, useEffect } from 'react'; | ||||||
|
|
||||||
| /** | ||||||
| * Determines the active heading slug from IntersectionObserver entries. | ||||||
| * Exported as a named function to allow unit testing without a DOM environment. | ||||||
| * | ||||||
| * @param {IntersectionObserverEntry[]} entries | ||||||
| * @returns {string | null} | ||||||
| */ | ||||||
| export const getActiveSlug = entries => { | ||||||
| const visible = entries.find(e => e.isIntersecting); | ||||||
|
|
||||||
| return visible ? visible.target.id : null; | ||||||
| }; | ||||||
|
|
||||||
| /** | ||||||
| * Tracks which heading is currently visible in the viewport using IntersectionObserver. | ||||||
| * | ||||||
| * @param {Array<{ slug: string }>} headings | ||||||
| * @returns {string | null} The slug of the active heading | ||||||
| */ | ||||||
| export const useScrollSpy = headings => { | ||||||
| const [activeSlug, setActiveSlug] = useState(null); | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
|
|
||||||
| useEffect(() => { | ||||||
| const observer = new IntersectionObserver( | ||||||
| entries => { | ||||||
| const slug = getActiveSlug(entries); | ||||||
|
|
||||||
| if (slug !== null) { | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
| setActiveSlug(slug); | ||||||
| } | ||||||
| }, | ||||||
| { rootMargin: '0px 0px -70% 0px', threshold: 0 } | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. could you add some inline comments |
||||||
| ); | ||||||
|
|
||||||
| headings.forEach(({ slug }) => { | ||||||
| const el = document.getElementById(slug); | ||||||
|
|
||||||
| if (el) { | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Any chance Simplify to; observe.observe(el); |
||||||
| observer.observe(el); | ||||||
| } | ||||||
| }); | ||||||
|
|
||||||
| return () => observer.disconnect(); | ||||||
| }, [headings]); | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. |
||||||
|
|
||||||
| return activeSlug; | ||||||
| }; | ||||||
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.
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?