facebook / facebook/astryx

Accessibility & keyboard-management program: build shared interaction primitives, then migrate components

Open
#3,343 6 comments 0 reactions 0 assignees View on GitHub
bug enhancement
Dominant language
TypeScript
Stars
13k
Forks
1.1k
Avg merge
1d 15h
Merged PRs (30d)
690

Description

## Summary

This is the single tracking issue for a focused, staged program to close astryx's accessibility and keyboard-management gaps. It supersedes the ad-hoc per-finding issues: individual PRs reference this plan and each finding below.

An 8-domain audit (menus, selects/comboboxes, overlays, form controls, navigation/selection, complex widgets, infrastructure, testing) surfaced ~90 findings, adversarially re-verified. The important conclusion is **not** the length of the list — it's that almost every finding traces back to the same root cause.

## Root cause: thin, unevenly-patched shared infrastructure

astryx's entire keyboard/focus layer is four small hooks — `useFocusTrap`, `useListFocus`, `useGridFocus`, `useScrollLock` — wired differently into each component. There is:

- **no shared roving-tabindex / composite engine** — every widget that needs arrow-key navigation (Toolbar, menus, TabMenu, SegmentedControl, Calendar grid, Tokenizer chips) re-implements it, and each re-implementation has a different bug;
- **no typeahead** anywhere except the Selector family (which restarts from index 0);
- **no default focus restore** and **no modality tracking** — closing a layer routinely drops focus to ``;
- **no Escape/dismissal stack** — one `Escape` closes a popover *and* its host dialog because every trap adds an unconditional document-level listener;
- **no background inerting**, yet `role="dialog" aria-modal="true"` is stamped on **every** popup (`usePopover`), so AT is told the focused control is inert while focus stays outside the popup;
- **no direction context** — horizontal arrow keys are hardcoded physical, so RTL locales navigate backwards;
- **no `scrollIntoView`** of the highlighted option, no persistent live regions, no `VisuallyHidden` primitive.

The architecture bet itself (native `` + `showModal()`, the Popover API, native form controls) is **sound and worth keeping** — native modality, native inputs, and top-layer stacking are genuine wins. What's missing is the machinery that makes the ARIA vocabulary true per keystroke. Fixing the components one-by-one without the shared primitives just re-creates the same defects in parallel.

## Strategy

**Build the primitives once, centrally; then migrate components onto them.** Land the blocker fixes immediately (they fully exclude a user class), then invest in a small set of shared primitives, then do pattern rework on top.

### Primitives to build

| Primitive | Replaces / fixes | Notes |
|---|---|---|
| `useComposite{Root,Item}` — roving-tabindex engine | Toolbar, menus, TabMenu, SegmentedControl, Calendar grid, Tokenizer chips | managed `tabIndex`, disabled-index skipping (kept in map, not focused), optional wrap, both orientations, RTL key-flipping, text-input caret-boundary guard. Supersedes `useListFocus`/`useGridFocus` (kept as deprecated shims during migration). |
| `useTypeahead` | all menus + Selector family | 750ms reset buffer, Space-suppression mid-typing, match-from-current-item cycling, select-while-closed |
| `useActiveDescendant` combobox model | Selector, MultiSelector, BaseTypeahead, PowerSearch, Chat mention menu | The APG combobox pattern: the combobox/input keeps DOM focus, `aria-controls` points at a `role="listbox"`, and highlight moves via `aria-activedescendant` — **no `aria-modal` dialog wrapper**. This is the correct alternative to the current false-modal stamp for every listbox-bearing popup. Ref: W3C APG combobox-autocomplete examples. |
| Focus-restore + modality tracking in `useLayer`/`usePopover` | menus-3, overlays-7, infra-12, and the systemic "focus falls to body" defect | every layer records its opener and restores on close (opt-out), passing `focusVisible` so keyboard-closed popups restore with a ring |
| Dismissal / Escape stack in `LayerProvider` | infra-2, overlays-2/3, menus-2 | topmost-only Escape by construction; `isComposing`/`keyCode===229` IME guard; scoped to the container |
| `usePopover` `role`/`isModal` options | comboboxes-3, menus-7, overlays-4, infra-3, infra-13 (one blocker-class + four majors, ~10 components) | `role?: 'dialog' \| 'none'`, `isModal?: boolean` (default false). Listbox/menu popups pass `role: 'none'` (their own role is exposed); genuine dialogs keep `role: 'dialog'` and **must** pass a label. |
| `useAnnounce()` + persistently-mounted live regions + `VisuallyHidden` | comboboxes-6/7, forms-8/17, overlays-8, navigation-10, complex-16/17 | one polite+assertive region pair mounted in `LayerProvider`; `VisuallyHidden` replaces ~6 hand-rolled sr-only blocks (see #3338) |
| `DirectionProvider` | infra-7, forms-3, navigation-7 | logical key-flipping consumed by the composite engine (comes free once composite lands), Slider, SegmentedControl, layer slide keyframes |

## Finding register, by tier

### Tier 0 — Blockers (a user class has *zero* path)
- [x] `comboboxes-1` Selector/MultiSelector trigger `tabIndex={-1}` → **fixed (#3324)**
- [x] `menus-1` NavHeadingMenu onClick-only items no Enter/Space → **fixed (#3334)**
- [x] `comboboxes-2` Selector/MultiSelector clear buttons `tabIndex={-1}`, no Delete/Backspace fallback -> **#3365**
- [x] `complex-1` TreeList: items without onClick/href render nothing focusable; expand/collapse is click-only → **interim: render a real toggle `` whenever the item has children** -> **interim fixed #3344**

### Tier 1 — Systemic majors (quick wins, mostly line-level)
- [x] `comboboxes-3`/`menus-7`/`overlays-4`/`infra-3`/`infra-13` `usePopover` false `role="dialog" aria-modal="true"` on every popup → **`role`/`isModal` option + migrate the ~10 listbox/menu callsites to `role: 'none'`** -> **#3350**
- [x] `overlays-2`/`overlays-3`/`infra-2` Escape has no stacking + no IME guard (one Escape closes every layer) -> **#3357**
- [x] `menus-2`/`menus-3` ContextMenu: no Escape when `hasAutoFocus={false}`, no focus restore -> **#3361**
- [x] `menus-5` DropdownMenu: Tab moves focus into page while menu stays open -> **#3368**
- [x] `overlays-5` Tooltip fails WCAG 1.4.13 (no Escape dismiss, not hoverable) -> **#3363**
- [x] `comboboxes-5` highlighted option never scrolled into view -> **#3377**
- [x] `menus-4`/`navigation-5`/`infra-15` `useListFocus` freezes on disabled items; selector omits menuitemradio/checkbox -> **#3376**
- [x] `complex-4` Calendar parses `aria-label` with `new Date()` instead of the `data-date` attribute (locale-dependent break) -> **#3367**
- [x] `complex-7`/`complex-8`/`complex-18` scrollable regions (CodeBlock, Table, Markdown) not keyboard-focusable -> **#3369**
- [x] `complex-6` CodeBlock Copy click bubbles into collapse header (also nested-interactive) -> **#3369**
- [x] `navigation-6` SegmentedControl can lose its only tab stop on a stale/unmatched value -> **#3370**
- [x] `navigation-8` SideNavItem `aria-current` on non-interactive wrapper; hover-opened flyout announced as modal dialog -> **#3378** (aria-current placement); hover-flyout modal via #3350
- [x] `infra-4` (crash half) `showPopover()` called unconditionally → TypeError on Safari <17 / Firefox <125 -> **#3366**

### Tier 2 — Structural (build on the primitives)
- [ ] `navigation-3/4`, `menus-4/6`, `navigation-2/6/7`, `comboboxes-8`, `complex-2` → **composite / roving-tabindex primitive** (Toolbar first, then menus, SegmentedControl, Calendar grid, Tokenizer chips)
- [ ] `menus-11`, `comboboxes-10/11` → **`useTypeahead`**
- [ ] `navigation-1/2`, `menus-6`, `testing-6` → **TabList APG mode + TabMenu rework** (see #3335; keep the ``+`aria-current` variant for href link-tabs)
- [ ] `complex-1` (full) → **TreeList APG tree pattern** (`aria-level`/`posinset`/`setsize`, ArrowRight/Left expand/collapse)
- [ ] `complex-2/3` → **Calendar grid restructure** (valid grid/row/gridcell/columnheader tree + composite grid nav)
- [ ] `comboboxes-4/6/7`, `forms-8/17`, `overlays-8`, `navigation-10`, `complex-16/17` → **live-region utility + `VisuallyHidden`** (#3338)
- [ ] `menus-2/3`, `overlays-6/7`, `infra-2` → **overlay focus + dismissal coherence** (focus restore built-in, layer stack, Toast F6 hotkey + focus handoff + blur pause)
- [x] `menus-8` ContextMenu touch (long-press) + keyboard (Shift+F10) invocation -> **#3392**
- [ ] `menus-9` submenus (capability gap — schedule only if product demand)

### Tier 3 — Foundational
- [ ] **Testing infrastructure**: shared `popupConformance()` suite; Vitest browser mode for focus/top-layer; direct unit tests for the hooks (currently zero); make the axe CI gate blocking on new violations + scan open states; add `@storybook/addon-a11y`; add an a11y docs topic (`testing-1/3/4/7/11`, `testing-8`)
- [ ] **RTL plumbing** — `DirectionProvider` (`infra-7`, `forms-3`, `navigation-7`)
- [ ] **Form serialization strategy** — hidden native input pattern across Selector, MultiSelector, Tokenizer, Slider, Switch, Checkbox, RadioList; unify `required` vs `aria-required` (`forms-2/9/11/12`, `comboboxes-9`)
- [ ] **Browser baseline decision** — declare a supported floor *or* add a JS positioning fallback for browsers without CSS anchor positioning (`infra-4`)

Full minor/note findings (RTL details, scroll-lock compensation, shadow-DOM, i18n of a11y strings, per-component announcements) are tracked as sub-tasks under their owning primitive/component.

## Sequencing

1. **Now** — Tier 0: clear the last blocker (TreeList interim toggle) + clear-button keyboard path. Line-level, ship as a patch.
2. **Next** — Tier 1: the `usePopover` role option (removes the false-modal from ~10 components at once) + Escape/IME guards + Tooltip 1.4.13 + ContextMenu Escape/restore + scrollIntoView. Stand up the conformance-test harness in parallel so these land with regression coverage.
3. **Then** — Tier 2: composite primitive is the keystone (Toolbar → menus → SegmentedControl → Calendar → Tokenizer); `useTypeahead` rides along; then TabList/TreeList/Calendar pattern rework and the live-region/announcement work.
4. **Ongoing / decision-gated** — testing infra (early, so structural work lands protected), RTL, form serialization, browser baseline.

**Rationale for order:** blockers first (they fully exclude keyboard/switch users); the `usePopover` role option second (one option corrects the ARIA story for every combobox, menu, and nav flyout simultaneously); the composite primitive before any pattern rework (TabList, TreeList, Calendar, Toolbar, Tokenizer otherwise each re-implement the same roving-tabindex/disabled-skip/RTL logic); testing infra early because today **zero** of these defects are catchable by CI.

## Related issues / PRs
- #3324, #3334 — blockers already fixed
- #3287 — Popover dismiss knobs (folds into the dismissal work)
- #3335 — TabList APG (Tier 2)
- #3338 — VisuallyHidden (Tier 2 live-region work)
- #3240 — stable open-state test anchors for HoverCard/Tooltip (supports the conformance harness)

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.