feat(core): TabList reads its role and speaks the ARIA tabs pattern - #5349
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
meta-cla
Bot
added
the
CLA Signed
0628b8e to
34f5c6d
Compare
cixzhang
changed the title
b384572 to
7b025fa
Compare
de952fe to
27a181e
Compare
cixzhang
left a comment
cixzhang
left a comment
There was a problem hiding this comment.
Thanks — reading role is the right shape, and that half is an approve from me.
I'd rather land it alone. Picking the pattern when no role is given is a second decision, and not the review's to settle: every caller passes no role, so they all become tablists — and none pass panelId, so a screen reader announces a selected tab with no panel to move to. The explicit role answers a problem the body states; I can't find one written down for the default change, and the 20 moved tests and the [breaking] are all its.
Send the explicit-role half on its own and it lands; the auto pick wants its own PR, with that reason in the body.
[Reviewed by Robohands]
| // else does not get to be a tablist behind the caller's back. | ||
| const isTabListAsserted = role === 'tablist'; | ||
| const isPatternAuto = role === undefined; | ||
| const canAutoTabs = useAutoPattern(stripRef, isPatternAuto); |
There was a problem hiding this comment.
Hmm, this is the default change — every caller passes no role today. Probably wants its own PR.
| 'a tab in a role="tablist" TabList controls nothing: pass panelId with ' + | ||
| 'the id of the panel it opens, so assistive technology can associate ' + | ||
| 'the two.', | ||
| isTabRole && !tabListCtx.isPatternAuto && controls == null, |
There was a problem hiding this comment.
Suppressed here, so an auto-picked tablist never asks for a panel. Those are the strips that have none.
cixzhang
left a comment
cixzhang
left a comment
There was a problem hiding this comment.
Thanks — this is the split the earlier review asked for, and the explicit-role half stands on its own. Approve from me once you undraft.
[Reviewed by Robohands]
| 'a tab in a role="tablist" TabList controls nothing: pass panelId with ' + | ||
| 'the id of the panel it opens, so assistive technology can associate ' + | ||
| 'the two.', | ||
| isTabRole && controls == null, |
There was a problem hiding this comment.
Hmm, panelId with no role is dropped silently. The other three mistakes here warn.
There was a problem hiding this comment.
Taken — it warns now: panelId outside a role="tablist" strip says so and points at the role. Tab.tsx:295.
a43dbe9 to
1391d6d
Compare
TabList declares role?: AriaRole and reads it. role="tablist" gives the WAI-ARIA tabs pattern -- role="tab", aria-selected, aria-controls from a new panelId on Tab, horizontal-only arrows, and an href ignored with a dev warning. Any other role passes through to the element untouched. An omitted role is unchanged: the <nav> landmark with aria-current that TabList has always rendered.
PR Analysis Report📚 Storybook PreviewView Storybook for this PR 🧪 Sandbox PreviewView Sandbox for this PR Modified ComponentsTabList (@astryxdesign/core) · View in Storybook
Bundle Size Summary
Accessibility AuditStatus: No accessibility violations detected. Generated by PR Enrichment workflow | Storybook | Sandbox | View full report |
1 participant
What
TabListcan speak the WAI-ARIA tabs pattern when a caller asks for it — and there is no new prop for the asking.TabListdeclaresrole?: AriaRoleand reads it, the wayLayoutHeader,LayoutContentandLayoutPanelalready declare and document theirs:role"tablist"role="tablist"on the strip,role="tab"andaria-selectedon the tabs,aria-controlsfrom a newpanelIdonTab<nav>landmark witharia-currentit has always beenStacked on #5348 — review that one first; this branch contains it.
Why no new prop
rolealready reaches the DOM:{...restProps}spreads it onto the wrapper, so a caller can passrole="tablist"today and get a tablist whose children are still<button>s witharia-current— invalid markup, noaria-selected, nav-style arrow keys, and no warning. Reading the role turns that silent breakage into the correct behaviour, and declaring it explicitly is what puts it in the type, the prop table and the docs, where a behaviour keyed on an anonymous passthrough would be invisible.Nothing changes for anyone who does not ask
A caller who passes no
rolegets exactly what they got before: same element, same classes, samearia-current, same both-axis arrow keys, same pixels. Measured rather than asserted — seven configurations of a no-role strip (plain, divider +lg+fill, with a menu, withhreftabs, icon-onlysm,overflow="visible", with action buttons) serialise byte-identically against this PR's base, 31,326 bytes each. Againstmainthe DOM differs only where #5348 changed it, and the role and ARIA attribute set is identical there too.Under the asserted role the strip takes only the horizontal arrows and leaves ArrowUp and ArrowDown to the page, because a tablist reports itself as horizontal. ArrowLeft, ArrowRight, Home, End, Tab and the roving tab stop are unchanged.
Two things the asserted role tells you about
A tab with an
hrefstops navigating, and a development warning says so. Activating a tab swaps a panel in place, so a tab that navigates is a false statement — but it is also the caller's link, so it is only ever ignored where the pattern was asked for by name.A tab that controls nothing is asked for a
panelId. EitherpanelIdor anaria-controlsyou wrote yourself satisfies it —Tabalready spreads yours to the DOM, so both spellings count and a hand-written one is never overwritten. Nothing validates that the target exists: a panel may mount later.aria-controlsis emitted only when an id is supplied. Generating one would ship a dangling reference, which axe rates critical (aria-valid-attr-value); measured in Chromium on three hand-built tablists, dangling → 1 critical violation, resolved → clean, absent → clean.A menu or any other non-tab in a tablist strip is invalid markup, and warns too — read off the rendered DOM in development, so a menu behind a conditional, inside a
.map, or wrapped in a component of your own is still caught.Testing
Accessibility trees read from Chromium's own tree (CDP), not inferred from the DOM:
rolenavigation("Tabs")<nav>aria-current="true"on the selected onerole="tablist"+panelIdtablist("Project views")<div>tabs,aria-selected,aria-controls, 3tabpanelsrole="tablist", nopanelIdtablist("Project views")<div>tabs, noaria-controls, one warning eachrole="navigation"navigation("Project sections")<nav role="navigation">aria-current="true"The no-
rolerow is identical, row for row, to the same story onmain.axe is clean on every one of them, discounting
region, which the repo's own audit disables for story isolation (.github/scripts/accessibility-audit.js:54).86 unit tests green across
TabListand the edge-compensation suite; every pre-existing TabList test is untouched — the test file's diff against the base is additions only. Typecheck and lint clean.