Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions .changeset/tablist-aria-role.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
---
'@astryxdesign/core': minor
---

[breaking] TabList: the strip now speaks the WAI-ARIA tabs pattern — `role="tablist"` on the strip, `role="tab"` and `aria-selected` on the tabs, and `aria-controls` pointing at the panel each tab opens, from a new `panelId` prop on `Tab`. There is no new prop for it: `TabList` declares `role?: AriaRole` and reads it. `role="tablist"` asks for the pattern, any other role passes through to the element untouched, and **left unset the strip picks for itself** — the tabs pattern when it holds nothing but tabs, the navigation landmark when it holds anything else. The keyboard behaviour the pattern asks for was already there: arrows move between tabs, Tab leaves the strip.

**This changes the default.** A strip of plain tabs is a `<nav>` landmark today and becomes a tablist: the selected tab is announced as a selected tab rather than the current item, `aria-current` gives way to `aria-selected`, and — because a tablist reports itself as horizontal — ArrowUp and ArrowDown stop moving between tabs and scroll the page instead. ArrowLeft, ArrowRight, Home, End, Tab and the roving tab stop are unchanged, as is everything the strip looks like. A strip that really is page navigation keeps the landmark by saying so with `role="navigation"`.

Two things the strip will not do to you on its own. A tab with an `href` navigates, so a strip holding one stays navigation — an `href` is only ignored where `role="tablist"` was asked for explicitly, and then a development warning says so. And a tab that controls nothing is only worth mentioning to a consumer who asked for the pattern, so that warning too is limited to the explicit role; either `panelId` or an `aria-controls` you wrote yourself satisfies it, and a hand-written one is never overwritten. `aria-controls` is emitted only when you supply the id: pointing at a panel that does not exist is an invalid attribute value, which is worse than saying nothing.

The pick is made from the rendered DOM rather than from `children`, so a menu behind a conditional, inside a `.map`, or wrapped in a component of your own still counts, and it follows the strip if its contents change later — without remounting the tabs.

@cixzhang
15 changes: 0 additions & 15 deletions .github/a11y-baseline.json
Original file line number Diff line number Diff line change
Expand Up @@ -893,11 +893,6 @@
"impact": "moderate",
"helpUrl": "https://dequeuniversity.com/rules/axe/4.12/landmark-unique?application=playwright"
},
{
"key": "TabList::Size Variants::landmark-unique",
"impact": "moderate",
"helpUrl": "https://dequeuniversity.com/rules/axe/4.12/landmark-unique?application=playwright"
},
{
"key": "Table::In Card Densities::color-contrast",
"impact": "serious",
Expand Down Expand Up @@ -1048,16 +1043,6 @@
"impact": "serious",
"helpUrl": "https://dequeuniversity.com/rules/axe/4.12/color-contrast?application=playwright"
},
{
"key": "Toolbar::Composition: Tab Navigation::landmark-unique",
"impact": "moderate",
"helpUrl": "https://dequeuniversity.com/rules/axe/4.12/landmark-unique?application=playwright"
},
{
"key": "ToolbarEdgeCompensation::Tabs in toolbar (all sizes)::landmark-unique",
"impact": "moderate",
"helpUrl": "https://dequeuniversity.com/rules/axe/4.12/landmark-unique?application=playwright"
},
{
"key": "useContainerReveal::Inverted Conceal::color-contrast",
"impact": "serious",
Expand Down
94 changes: 94 additions & 0 deletions apps/storybook/stories/TabList.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,10 @@ const meta: Meta<typeof TabList> = {
export default meta;
type Story = StoryObj<typeof TabList>;

/**
* With no `role`, a strip of nothing but tabs speaks the WAI-ARIA tabs
* pattern: `role="tablist"` / `role="tab"` and `aria-selected`.
*/
export const Default: Story = {
args: {
size: 'md',
Expand All @@ -38,6 +42,32 @@ export const Default: Story = {
},
};

/**
* A strip that really is page navigation says so with `role="navigation"`,
* and stays a `<nav>` landmark marking the current item with `aria-current`
* however few or many tabs it holds.
*/
export const NavigationLandmark: Story = {
render: () => {
const [value, setValue] = useState('overview');
return (
<TabList
value={value}
onChange={setValue}
role="navigation"
aria-label="Project sections">
<Tab value="overview" label="Overview" />
<Tab value="activity" label="Activity" />
<Tab value="members" label="Members" />
</TabList>
);
},
};

/**
* A menu is not a tab, so a strip holding one stays a `<nav>` landmark with
* `aria-current` — the tabs pattern would be invalid markup here.
*/
export const WithMenu: Story = {
args: {
size: 'md',
Expand Down Expand Up @@ -194,6 +224,11 @@ export const IconOnly: Story = {
* Demonstrates a common page header pattern: large tab list items on the left
* with action buttons on the right, separated by a full-width divider underneath.
*/
/**
* Action buttons rendered among the tabs are not tabs, so the strip keeps the
* navigation pattern. Put them outside the `TabList` if you want the tabs
* pattern as well.
*/
export const WithActions: Story = {
render: () => {
const [value, setValue] = useState('all');
Expand Down Expand Up @@ -404,3 +439,62 @@ export const OverflowNone: Story = {
);
},
};

/**
* `role="tablist"` asks for the WAI-ARIA tabs pattern: `role="tablist"` /
* `role="tab"`, `aria-selected`, and each tab pointing at the panel it
* controls. A screen reader announces "tab 2 of 3, selected" and can move to
* the panel it opens. A strip of nothing but tabs reaches the same pattern on
* its own — the explicit role is how you keep it when the strip also holds
* something else, and how you ask to be warned when it does.
*/
export const TabsPattern: Story = {
render: () => {
const [value, setValue] = useState('overview');
const panels = {
overview: 'Everything at a glance.',
activity: 'What happened recently.',
members: 'Who has access.',
};
return (
<div style={{display: 'grid', gap: '12px', maxWidth: '400px'}}>
<TabList
value={value}
onChange={setValue}
role="tablist"
aria-label="Project views"
hasDivider>
<Tab
value="overview"
label="Overview"
id="tab-overview"
panelId="panel-overview"
/>
<Tab
value="activity"
label="Activity"
id="tab-activity"
panelId="panel-activity"
/>
<Tab
value="members"
label="Members"
id="tab-members"
panelId="panel-members"
/>
</TabList>
{Object.entries(panels).map(([key, text]) => (
<div
key={key}
id={`panel-${key}`}
role="tabpanel"
aria-labelledby={`tab-${key}`}
tabIndex={0}
hidden={key !== value}>
{text}
</div>
))}
</div>
);
},
};
Original file line number Diff line number Diff line change
Expand Up @@ -64,18 +64,17 @@ describe('Edge Compensation', () => {
<Tab value="tab1" label="Tab 1" />
</TabList>,
);
const tab = screen.getByRole('button', {name: 'Tab 1'});
const tab = screen.getByRole('tab', {name: 'Tab 1'});
expect(tab).toHaveAttribute(EDGE_COMP_ATTR);
});

it('applies edge comp attribute to TabList wrapper', () => {
render(
const {container} = render(
<TabList value="" onChange={() => {}} aria-label="Tabs">
<Tab value="tab1" label="Tab 1" />
</TabList>,
);
const nav = screen.getByRole('navigation', {name: 'Tabs'});
expect(nav).toHaveAttribute(EDGE_COMP_ATTR);
expect(container.querySelector('nav')).toHaveAttribute(EDGE_COMP_ATTR);
});
});

Expand Down
14 changes: 13 additions & 1 deletion packages/core/src/TabList/Tab.doc.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,13 @@ export const docs = {
name: 'href',
type: 'string',
description:
'URL to navigate to; when provided, the tab renders as an anchor element.',
'URL to navigate to; when provided, the tab renders as an anchor element and keeps the strip on the navigation pattern. Ignored in a TabList given an explicit role="tablist".',
},
{
name: 'panelId',
type: 'string',
description:
'Id of the panel this tab controls, wired up as aria-controls where the TabList speaks the tabs pattern. Put the same id on the panel element. No effect under the navigation pattern, including where the strip picked that pattern because it holds something that is not a tab.',
},
{
name: 'as',
Expand Down Expand Up @@ -138,6 +144,12 @@ export const docsZh = {
type: 'string',
description: '要导航到的 URL;提供时,标签渲染为锚点元素。',
},
{
name: 'panelId',
type: 'string',
description:
'Id of the panel this tab controls, wired up as aria-controls where the TabList speaks the tabs pattern. Put the same id on the panel element. No effect under the navigation pattern, including where the strip picked that pattern because it holds something that is not a tab.',
},
{
name: 'as',
type: 'LinkComponentType',
Expand Down
75 changes: 68 additions & 7 deletions packages/core/src/TabList/Tab.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,8 @@
* @input Uses React, StyleX, TabListContext
* @output Exports Tab component and TabProps type
* @position Core tab item; renders as button or anchor in navigation with a
* divider-overlay selected indicator
* divider-overlay selected indicator. Where the TabList speaks the tabs
* pattern it is a button with role="tab".
*
* SYNC: When modified, update:
* - /packages/core/src/TabList/TabList.doc.mjs
Expand Down Expand Up @@ -35,6 +36,7 @@ import {tabScope} from './tab.markers.stylex';
import {useLinkComponent} from '../Link/useLinkComponent';
import type {LinkComponentType} from '../Link/types';
import {mergeProps} from '../utils';
import {useDevWarning} from '../hooks/useDevWarning';
import {EDGE_COMP_ATTR} from '../Layout/edgeCompensation.stylex';
import {themeProps} from '../utils/themeProps';
import {focusOutlineProps} from '../utils/focusOutline.stylex';
Expand Down Expand Up @@ -64,8 +66,22 @@ export interface TabProps extends BaseProps<HTMLButtonElement> {
isLabelHidden?: boolean;
/**
* URL to navigate to. When provided, renders as an anchor element.
*
* A tab that navigates keeps the strip on the navigation pattern. Ignored
* only in a TabList given an explicit `role="tablist"`: activating a tab
* there swaps a panel in place, so a tab that navigates would be a false
* statement.
*/
href?: string;
/**
* Id of the panel this tab controls, wired up as `aria-controls` where the
* TabList speaks the tabs pattern. Put the same id on the panel element.
*
* Has no effect under the navigation pattern, where there is no panel to
* associate — including where the strip picked that pattern because it
* holds something that is not a tab.
*/
panelId?: string;
/**
* Icon element shown when tab is not selected.
*/
Expand Down Expand Up @@ -228,6 +244,7 @@ export function Tab({
label,
isLabelHidden = false,
href,
panelId,
icon,
selectedIcon,
endContent,
Expand All @@ -242,13 +259,43 @@ export function Tab({
const isSelected = tabListCtx.value === value;
const size: TabListSize = tabListCtx.size;
const isFill = tabListCtx.layout === 'fill';
const isTabsPattern = tabListCtx.pattern === 'tabs';
// An href gives way to the tabs pattern only where the consumer asked for
// that pattern. Where the strip picked it, the link stays a link: the strip
// reads its own children back and settles on the navigation pattern, so a
// tab that navigates never ends up inside a tablist.
const isLink = href != null && (!isTabsPattern || tabListCtx.isPatternAuto);
const isTabRole = isTabsPattern && !isLink;
const displayIcon = isSelected && selectedIcon ? selectedIcon : icon;
const hasVisibleLabel = !isLabelHidden && label !== '';

const handleSelect = useCallback(() => {
tabListCtx.onChange(value);
}, [tabListCtx, value]);

useDevWarning(
'Tab',
'href is ignored in a role="tablist" TabList — a tab swaps a panel in ' +
'place rather than navigating. Drop the href, or drop the role and let ' +
'the strip pick its own pattern.',
isTabRole && href != null,
);

// A consumer who wired aria-controls by hand already said which panel this
// is, so panelId is the sugar, not the only way in.
const controls = panelId ?? restProps['aria-controls'];

// Only where the tabs pattern was asked for: a strip left to pick reaches it
// through tabs that were written for the navigation pattern, and scolding
// the consumer for a panel they were never asked for is noise.
useDevWarning(
'Tab',
'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,
);

const iconElement = displayIcon ? (
<span {...stylex.props(styles.icon, iconSizeStyles[size])}>
{displayIcon}
Expand All @@ -260,11 +307,25 @@ export function Tab({
...(isLabelHidden ? {'aria-label': label} : {}),
[EDGE_COMP_ATTR]: '',
'data-tab-value': value,
// Generic `true` ("the current item within a set"), not `page`: the strip
// switches views in place at least as often as it navigates, and claiming
// "current page" when no page changed is a false statement to a screen
// reader. Stays truthful for the `href` case too, just less specific.
'aria-current': isSelected ? ('true' as const) : undefined,
...(isTabRole
? {
role: 'tab' as const,
'aria-selected': isSelected,
// Only when there is a panel to point at: an aria-controls whose
// target does not exist is an invalid attribute value, which is a
// worse state than saying nothing. The dev warning above asks for
// the id instead.
'aria-controls': controls,
}
: {
// Generic `true` ("the current item within a set"), not `page`: the
// strip switches views in place at least as often as it navigates,
// and claiming "current page" when no page changed is a false
// statement to a screen reader. Stays truthful for the `href` case
// too, just less specific. A tab role states this with
// aria-selected instead.
'aria-current': isSelected ? ('true' as const) : undefined,
}),
// Roving tabindex: the tab strip is a single Tab stop. The selected tab is
// the tabbable one; the rest are reachable via arrow keys (handled by
// TabList's onKeyDown). When no tab is selected, TabList's repair effect
Expand Down Expand Up @@ -321,7 +382,7 @@ export function Tab({
<span {...stylex.props(styles.endContentWrapper)}>{endContent}</span>
) : null;

if (href != null) {
if (isLink) {
return (
<LinkComponent
ref={ref}
Expand Down
Loading