Skip to content

feat(table-of-contents): scroll-tracking hook + wire docs TOC to it#645

Merged
mattrothenberg merged 7 commits into
cloudflare:mainfrom
subalakshmi-murugesan:feat/UI-Table-of-contents-kumo-scrollfix
Jul 20, 2026
Merged

feat(table-of-contents): scroll-tracking hook + wire docs TOC to it#645
mattrothenberg merged 7 commits into
cloudflare:mainfrom
subalakshmi-murugesan:feat/UI-Table-of-contents-kumo-scrollfix

Conversation

@subalakshmi-murugesan

Copy link
Copy Markdown
Contributor

Summary

The TableOfContents component is presentational, so consumers had to wire up their own scroll tracking. This adds a shared useTableOfContentsActiveId hook to @cloudflare/kumo and rewires the docs "On this page" TOC to use it.

The hook derives the active section from scroll position via an IntersectionObserver (useScrollspy) and pins a section on click / hash deep-link, resuming scrollspy once the smooth scroll settles.

  • Highlights the topmost section actually in view — no longer stays pinned to the first item.
  • Never force-jumps to the last item at the page bottom, so short trailing sections stay reachable.
  • selectSection keeps a clicked / deep-linked section active even when it's too short to reach the activation line.

This mirrors the scroll-tracking fix landed for the dashboard TOC (stratus !40583, DESENG-1024), extracting the shared hook so the behavior isn't re-implemented per consumer.

Changes

  • @cloudflare/kumo (minor): new useTableOfContentsActiveId hook (+ internal useScrollspy primitive, types, unit tests), exported from the package root.
  • @cloudflare/kumo-docs-astro (patch): the docs TableOfContents now consumes the shared hook instead of its own bespoke IntersectionObserver (which force-jumped to the last item at the page bottom). The island is mounted client:only because the hook builds an IntersectionObserver during render, which isn't available during Astro SSR.

Testing

  • pnpm --filter @cloudflare/kumo test — 25 passing (incl. 5 new hook tests)

  • pnpm --filter @cloudflare/kumo typecheck + docs astro check + pnpm lint — clean

  • pnpm --filter @cloudflare/kumo build and docs astro build — succeed (71 pages)

  • Manually verified on the docs dev server: the active item tracks scroll, no last-item jump, and click / #hash pins the section.

  • Reviews

  • bonk has reviewed the change

  • automated review not possible because: opened via CLI by an AI agent; bonk + human review requested on the PR

  • Tests

  • Tests included/updated

  • Automated tests not possible - manual testing has been completed as follows:

  • Additional testing not necessary because:

subalakshmi-murugesan and others added 5 commits July 20, 2026 11:49
…ng hook

The TableOfContents component is presentational, so consumers had to wire up their own scroll tracking. Add a shared hook that derives the active section from scroll position via an IntersectionObserver (useScrollspy) and pins a section on click / hash deep-link, resuming scrollspy once the smooth scroll settles. It highlights the topmost section in view and never force-jumps to the last item at the page bottom.
Replace the docs TOC's bespoke IntersectionObserver (and its force-jump-to-last-item clamp) with the shared useTableOfContentsActiveId hook, so the active section tracks scroll position and short trailing sections stay reachable. Mount the island client:only since the hook creates an IntersectionObserver during render, which is unavailable during Astro SSR.
Rework the scroll-tracking hook so it is SSR-safe and simpler to consume:

- Create the IntersectionObserver only inside effects (never during render),
  so consumers can server-render again — docs islands revert from
  client:only back to client:load.
- API takes section `ids` (plus `offset`, `root`, `trackHash`) instead of
  resolved Element targets, and handles hash deep-links internally.
- Replace the scrollend-based pin/resume with a debounced scroll listener,
  fixing a permanently-stuck active state on browsers without scrollend
  (Safari < 18.2).
- Collapse the useIntersectionObserver/useScrollspy layers into the hook and
  rename the return type to UseTableOfContentsActiveIdResult.
- Test the real observer logic via a mock IntersectionObserver instead of
  mocking the scrollspy layer wholesale.
- TableOfContents.Group: attach onClick to the group label link instead of
  the wrapping <li>, so clicks on nested items no longer bubble into the
  group handler and override the child's selection.
- Document the hook on the TableOfContents docs page with a live
  custom-scroll-container demo.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@mattrothenberg

Copy link
Copy Markdown
Collaborator

/bonk please review and approve

@pkg-pr-new

pkg-pr-new Bot commented Jul 20, 2026

Copy link
Copy Markdown
npm i https://pkg.pr.new/@cloudflare/kumo@645

commit: ba04a52

@github-actions

github-actions Bot commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

Docs Preview

View docs preview

Commit: ba04a52

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

@ask-bonk ask-bonk Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nice work — clean abstraction, good test coverage, and a real DX improvement for consumers. A few minor notes below, none blocking.

Approving.

Comment thread packages/kumo/src/components/table-of-contents/use-table-of-contents-active-id.ts Outdated
Comment thread packages/kumo-docs-astro/src/components/docs/TableOfContents.tsx
@ask-bonk

ask-bonk Bot commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

Review posted on PR #645.

Summary:

  • Approved the change. The hook is well-designed, SSR-safe, and the docs integration is a solid DX improvement.
  • Left 3 minor notes:
    1. The onClick fix in TableOfContentsGroup is correct, but it changes behavior for non-link groups (no href) where onClick is now silently dropped.
    2. A tiny nit on ids.join("\n") being theoretically ambiguous if ids contained newlines (not blocking).
    3. Praise for the ~60 lines of observer logic removed from the docs TOC by adopting the shared hook.

github run

@github-actions

github-actions Bot commented Jul 20, 2026

Copy link
Copy Markdown
Contributor
Visual Regression Report — 27 changed, 15 unchanged

27 screenshot(s) with visual changes:

Button / Basic

412 px (0.47%) changed

Before After Diff
Before After Diff

Button / Variant: Primary

631 px (0.71%) changed

Before After Diff
Before After Diff

Button / Variant: Outline

82 px (0.09%) changed

Before After Diff
Before After Diff

Button / With Icon

797 px (0.9%) changed

Before After Diff
Before After Diff

Button / Loading State

198 px (0.22%) changed

Before After Diff
Before After Diff

Dialog / Dialog With Actions

295 px (0.33%) changed

Before After Diff
Before After Diff

Dialog / Dialog Basic

316 px (0.36%) changed

Before After Diff
Before After Diff

Dialog / Dialog Sizes

2,891 px (3.26%) changed

Before After Diff
Before After Diff

Dialog / Dialog Alert

1,984 px (2.24%) changed

Before After Diff
Before After Diff

Dialog / Dialog Confirmation

2,827 px (3.19%) changed

Before After Diff
Before After Diff

Dialog / Custom Max Width

875 px (0.99%) changed

Before After Diff
Before After Diff

Dialog / Dialog With Select

130 px (0.15%) changed

Before After Diff
Before After Diff

Dialog / Dialog With Combobox

483 px (0.55%) changed

Before After Diff
Before After Diff

Dialog / Dialog With Dropdown

500 px (0.56%) changed

Before After Diff
Before After Diff

Dialog (Open)

96 px (0%) changed

Before After Diff
Before After Diff

Select / Select Basic

1,066 px (1.2%) changed

Before After Diff
Before After Diff

Select / Select Sizes

2,670 px (1.64%) changed

Before After Diff
Before After Diff

Select / Select Without Label

721 px (0.81%) changed

Before After Diff
Before After Diff

Select / Select With Description

0 px (0%) changed

Before After Diff
Before After Diff

Select / Select Custom Rendering

1,300 px (1.47%) changed

Before After Diff
Before After Diff

Select / Select Loading

2,360 px (1.34%) changed

Before After Diff
Before After Diff

Select / Select Complex

1,651 px (1.59%) changed

Before After Diff
Before After Diff

Select / Select Disabled Options

1,217 px (1.37%) changed

Before After Diff
Before After Diff

Select / Select Disabled Items

785 px (0.89%) changed

Before After Diff
Before After Diff

Select / Select Grouped With Disabled

1,313 px (1.48%) changed

Before After Diff
Before After Diff

Select / Select Long List

1,517 px (1.46%) changed

Before After Diff
Before After Diff

Select (Open)

1,407 px (0.01%) changed

Before After Diff
Before After Diff
15 screenshot(s) unchanged
  • Button / Variant: Secondary
  • Button / Variant: Ghost
  • Button / Variant: Destructive
  • Button / Variant: Secondary Destructive
  • Button / Sizes
  • Button / Icon Only
  • Button / Disabled State
  • Button / Title
  • Button / Link as Button
  • Button / Disabled Link
  • Select / Select With Error
  • Select / Select Placeholder
  • Select / Select With Tooltip
  • Select / Select Multiple
  • Select / Select Grouped

Generated by Kumo Visual Regression

- Use \0 as the ids cache-key separator (unambiguous for any valid id).
- Document why Group drops onClick when no href is provided.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@mattrothenberg
mattrothenberg merged commit 86ee08c into cloudflare:main Jul 20, 2026
15 checks passed
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.

2 participants