Skip to content

fix(tabs): make NSTabs keyboard operable (WCAG 2.1.1) - #338

Open
DipeshRajoria007 wants to merge 1 commit into
masterfrom
fix/tabs-keyboard-accessibility
Open

fix(tabs): make NSTabs keyboard operable (WCAG 2.1.1)#338
DipeshRajoria007 wants to merge 1 commit into
masterfrom
fix/tabs-keyboard-accessibility

Conversation

@DipeshRajoria007

Copy link
Copy Markdown
Contributor

Description

NSTabs (ui/elements/Tabs) was not keyboard operable — a WCAG 2.1.1 (Keyboard) failure. The container carried role="tablist", but each tab item was a click-only div with no role="tab", tabIndex, aria-selected, or keyboard handler, so keyboard-only and screen-reader users could neither reach nor operate the tabs. Every consumer is affected (the Calendar view switcher; the KoyoGP time picker; downstream apps such as newton-web's TMS resources File/Link switch).

This transplants the accessible pattern grauity already ships in the sibling NSTabList/NSTab into NSTabs, with no change to the public contract or existing runtime behavior.

What changed

  • Each tab now renders role="tab", aria-selected, and a roving tabIndex (exactly one tab is tabbable; on mount it's the selected tab — clamped so an out-of-range initialActiveTab can never leave every tab untabbable).
  • Container-level keyboard handling: Arrow Left/Right (with wraparound), Home/End, and Enter/Space to select. Manual activation — arrows move focus, Enter/Space/click commit.
  • New :focus-visible ring reusing the same token as NSTab/NSTabList (--border-subtle-brand-default).
  • New optional ariaLabel prop (default 'Tab list') to give the tablist an accessible name.
  • Unit tests for roles / aria-selected / roving tabindex / keyboard nav, plus a KeyboardNavigation story so the Storybook a11y panel demonstrates the fix.

Zero-regression

  • TabsProps gains only an optional ariaLabel — nothing removed, renamed, or retyped.
  • onTabFocusChange fires exactly as before: once on mount (initial index) and on every commit (click / Enter / Space). Arrows do not fire it (manual activation), so consumers like the Calendar view switcher see identical callback timing.
  • Uncontrolled activeTab + initialActiveTab seeding, string/ReactNode item rendering, role="tablist" + className passthrough, and the click path are unchanged. The item stays a <div> (no UA button reset, no nested-interactive risk with arbitrary node items).

Out of scope (intentional)

Tab↔tabpanel aria-controls/id wiring — NSTabs renders no panels; that would need consumer-supplied ids (a future additive prop), not a regression fix.

Verification

  • npm run typecheck clean · npm run lint clean · npm run test green (46 suites / 423 tests, including the Calendar suite).
  • Manual: Tab into the strip; Arrow/Home/End move focus; Enter/Space/click select; focus ring visible; Storybook a11y panel reports no violations (Elements/Tabs → KeyboardNavigation).

Type of Change

  • 🐛 Bug fix (non-breaking change which fixes an issue)

Related Issues

Motivated by an accessibility finding during review of a downstream consumer (newton-web PR #8745). No grauity issue filed yet.

Checklist

  • PR name uses present imperative tense and specifically describes the changes
  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation (JSDoc on the new prop + a story)
  • My changes generate no new TypeScript warnings
  • My changes does not hide eslint warnings un-necessarily
  • My pull request maintains linear history with the master branch

Additional Notes

Behavior model chosen is manual activation (mirrors NSTabList) specifically to preserve onTabFocusChange's current firing timing for existing consumers. Automatic activation (selection-follows-focus) was considered and rejected because it would fire the callback on every arrow keypress, changing callback frequency for the Calendar view switcher.

NSTabs rendered role="tablist" on the container but each tab was a
click-only div with no role="tab", tabIndex, aria-selected, or keyboard
handler, so keyboard and screen-reader users could neither reach nor
operate the tabs. This transplants the accessible pattern grauity
already ships in the sibling NSTabList/NSTab, with no change to the
public contract or existing runtime behavior.

- each item now renders role="tab", aria-selected, and a roving tabIndex
  (exactly one tab tabbable; on mount the selected one, clamped so an
  out-of-range initialActiveTab can't leave every tab untabbable)
- container onKeyDown: Arrow Left/Right (wraparound), Home, End, and
  Enter/Space to select — manual activation (arrows move focus;
  Enter/Space/click commit), so onTabFocusChange fires on the exact
  same events as before (mount + commit, never on arrow)
- add a :focus-visible ring reusing the Tab/TabList brand token
- add an optional ariaLabel prop (default 'Tab list') to name the tablist
- tests for roles/aria-selected/roving tabindex/keyboard + a
  KeyboardNavigation story for the a11y addon

TabsProps only gains an optional ariaLabel; nothing removed, renamed, or
retyped. The item stays a div (no UA button reset, no nested-interactive
risk with arbitrary ReactNode items).
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.

1 participant