Skip to content

fix(ui): tabs arrow-key nav changes selection but does not move focus #1078

Description

@vivek7405

Problem

Keyboard navigation in the @webjsdev/ui tabs component is broken. Pressing
ArrowRight / ArrowLeft (or ArrowDown / ArrowUp in vertical orientation), Home,
or End on a focused tab trigger changes the selected tab but does not move
keyboard focus
to the newly selected trigger. This violates the WAI-ARIA APG
Tabs pattern (focus must follow the roving tab stop) and leaves keyboard users
with the focus ring stranded on the previously focused trigger while a different
panel is shown.

Root cause (packages/ui/packages/registry/components/tabs.ts, UiTabsTrigger._onKeyDown).
The handler ends with target.focus(), where target is the
<ui-tabs-trigger> host custom element. The actually-focusable element is
the inner <button role="tab"> that the trigger renders. The host itself has no
tabindex, so HTMLElement.focus() on it is a no-op and focus stays put. The
roving tabindex (0 on active, -1 on inactive) is correctly applied to the
inner button, so Tab-into the tablist works. Only arrow/Home/End focus movement
is broken.

The gap is isolated to tabs. toggle-group works because its item host is the
role="button" element (item.focus() focuses a genuinely focusable host).
dropdown-menu works because it queries and focuses the inner [role="menuitem"]
elements. tabs is the one Tier-2 keyboard component that focuses the wrong node.

There are currently no keyboard-navigation browser tests for tabs at all.
test/components/browser/ui-stateful.test.js covers rendering / click / hidden
only. test/components/browser/ui-a11y.test.js covers tabs ARIA wiring but not
keyboard focus (the only keyboard-nav test in the suite is for toggle-group).
That coverage hole is why this regressed silently.

Design / approach

Focus the inner focusable control, not the host. In _onKeyDown, resolve the
target trigger's inner button before focusing, for example
(target.querySelector('[role="tab"]') as HTMLElement | null)?.focus().

Keep the existing APG "automatic activation" behaviour (arrow both selects and
moves focus). That is the correct pattern for tabs and matches shadcn/Radix.
Verify no double-render focus loss. Setting value triggers a microtask
re-broadcast (requestUpdate() on the triggers), so confirm lit-html reuses the
inner <button> so programmatic focus survives the re-render (it should, since
the template structure is stable). If focus is lost across the re-render, move
the focus call into the post-update path or re-query after the update settles.

While here, audit the other Tier-2 keyboard components for the same
"focus-the-host-instead-of-the-inner-control" class of bug and confirm each
focuses a genuinely focusable node (toggle-group and dropdown-menu already do,
they just lack full coverage).

Implementation notes (for the implementing agent)

  • Where to edit: packages/ui/packages/registry/components/tabs.ts,
    UiTabsTrigger._onKeyDown (around L240 to L262). The one-line fix is the final
    target.focus(), changed to focus target.querySelector('[role="tab"]').
  • Website copy regenerates automatically: packages/ui/packages/website/components/
    is gitignored and rebuilt from packages/registry/ by scripts/copy-registry.js.
    Do NOT hand-edit packages/website/components/tabs.ts (see packages/ui/AGENTS.md
    ui-website footgun).
  • Landmines:
    • The host <ui-tabs-trigger> is NOT focusable (light DOM, no tabindex on the
      host). Only the inner <button role="tab"> is. This is the whole bug.
    • Value change fires UiTabs.updated() on a queueMicrotask and calls
      _broadcast(), which runs requestUpdate() on every trigger/panel. Any
      focus call must land on the final button element and survive that re-render.
    • Do not regress SSR. render() runs server-side, so keep the typeof closest
      guards. Keyboard handlers are client-only (@keydown), which is fine.
  • Invariants: Tier-2 components own their ARIA (packages/ui/AGENTS.md
    "Accessibility"). Light DOM plus Tailwind, no shadow root.
  • Tests + docs surfaces:
    • Add browser keyboard-nav tests for tabs in
      packages/ui/test/components/browser/ui-a11y.test.js (the ui-tabs a11y
      suite) mirroring the existing toggle-group tests. ArrowRight moves
      document.activeElement to the next trigger's [role="tab"] AND updates
      data-state/value. ArrowLeft wraps. Home/End jump. Vertical orientation
      uses ArrowUp/Down. Include a counterfactual (a test that fails against the
      current target.focus(), asserting document.activeElement is the new inner
      button, which fails today).
    • Add matching keyboard-nav coverage for dropdown-menu (ArrowUp/Down moves
      focus, Escape closes) to close the "no coverage" half of this report, even
      though it currently passes.
    • Run via npm run test:browser.
    • No public API change, so AGENTS.md needs only a note if behaviour docs
      reference keyboard nav. The tabs JSDoc keyboard block is already correct (it
      documents the intended behaviour that was not being met).

Acceptance criteria

  • ArrowRight/Left (horizontal) and ArrowDown/Up (vertical) move
    document.activeElement to the newly selected trigger's inner [role="tab"].
  • Home / End move focus to the first / last trigger.
  • The selected value / data-state="active" and the focused button stay in
    sync after keyboard nav (no stranded focus ring).
  • A counterfactual test fails against the current target.focus() and passes
    after the fix.
  • Browser keyboard-nav tests added for tabs, plus matching coverage added for
    dropdown-menu.
  • Other Tier-2 keyboard components audited and confirmed to focus a
    genuinely focusable node.
  • Docs / AGENTS.md updated if any public surface changed (expected: none).

Metadata

Metadata

Assignees

Labels

bugSomething isn't working

Type

No type

Projects

Status
Done

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions