Skip to content

refactor(TreeList): let useTreeFocus own roving tabindex#3488

Draft
cixzhang wants to merge 2 commits into
mainfrom
navi/refactor/usetreefocus-roving
Draft

refactor(TreeList): let useTreeFocus own roving tabindex#3488
cixzhang wants to merge 2 commits into
mainfrom
navi/refactor/usetreefocus-roving

Conversation

@cixzhang

@cixzhang cixzhang commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

Summary

Make useTreeFocus own roving-tabindex management internally so TreeList no longer has to hand-roll it. This mirrors the useListFocus pattern (hasRovingTabIndex + handleFocus), where the hook stamps tabindex 0/-1 on the items directly, repairs the tab stop on mount/layout, and moves the stop alongside keyboard navigation.

Previously useTreeFocus only provided handleKeyDown; TreeList stamped tabindex on treeitem <li> elements itself via an activeId state, a resolvedActiveId derivation, and an isTabbable prop threaded through TreeListItem.

Changes

packages/core/src/hooks/useTreeFocus.ts

  • New hasRovingTabIndex?: boolean option (default false).
  • When enabled, focusItem stamps tabindex=0 on the focused treeitem and -1 on all others via setAttribute.
  • New syncTabStops utility + useIsomorphicLayoutEffect that repairs the stop on mount/commit: it preserves an existing tabindex="0" seed, otherwise promotes the first enabled treeitem.
  • New handleFocus return (always returned; a no-op unless hasRovingTabIndex is on) that keeps the stop in sync after clicks/programmatic focus.
  • When false, behavior is unchanged — the hook only moves focus and never touches tabindex.

packages/core/src/TreeList/TreeList.tsx

  • Passes hasRovingTabIndex: true and wires handleFocus on the role="tree" onFocus.
  • Removes the activeId state, the resolvedActiveId useMemo derivation, the onActiveChange wiring, and renames the seed helper to findInitialTabbableId.
  • Still seeds the initially-tabbable treeitem in render (selected item or first enabled); the hook's repair pass preserves that seed.

packages/core/src/TreeList/TreeListItem.tsx

  • isTabbable is now documented as the initial mount seed; the hook takes over dynamically.

Verification

  • typecheck (core + docs): pass
  • eslint: clean
  • vitest run packages/core/src/TreeList packages/core/src/hooks/useTreeFocus: 57/57 pass (arrow nav, expand/collapse, Home/End, typeahead, aria-level/posinset/setsize, roving-tabindex seeding + movement all preserved)

Add hasRovingTabIndex + handleFocus to useTreeFocus so it stamps and repairs the treeitem tab stop internally (matching useListFocus). TreeList drops its inline activeId state and resolvedActiveId derivation, seeds only the initial tabbable item in render, and wires handleFocus on the tree container.
@vercel

vercel Bot commented Jul 3, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
astryx Ready Ready Preview, Comment Jul 3, 2026 5:43am

Request Review

@meta-cla meta-cla Bot added the CLA Signed This label is managed by the Meta Open Source bot. label Jul 3, 2026
@github-actions

github-actions Bot commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

PR Analysis Report

📚 Storybook Preview

View Storybook for this PR
GitHub Pages may take up to a minute to hydrate after deploy.

🧪 Sandbox Preview

View Sandbox for this PR
GitHub Pages may take up to a minute to hydrate after deploy.

No new or modified components detected.

Bundle Size Summary

Package Size (ESM) Size (CJS) Gzipped
@astryxdesign/core N/A 4.6KB 0B

Accessibility Audit

Status: No accessibility violations detected.


Generated by PR Enrichment workflow | Storybook | Sandbox | View full report

github-actions Bot added a commit that referenced this pull request Jul 3, 2026
@josephfarina

Copy link
Copy Markdown
Contributor

Heads up: the test check is red, but the only failures are the two Table render-performance benchmarks in Table.perf.test.tsx (100 rows: 202ms vs 200ms budget, 500 rows: 551ms vs 500ms budget). Those are timing-sensitive budget assertions that flake under CI load and are unrelated to this TreeList/useTreeFocus change — TreeList.test.tsx and useTreeFocus.test.tsx both pass clean locally (57/57). Approving; just wanted it on your radar in case the perf budgets need a nudge.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Meta Open Source bot.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants