Skip to content
Merged
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
6 changes: 6 additions & 0 deletions .changeset/tabmenu-roving.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
'@astryxdesign/core': patch
---

[fix] TabList: the overflow tab menu now uses a roving tabindex — one tab stop with arrow-key navigation between items — instead of making every overflow item a separate tab stop. (#3728)
@bhamodi
186 changes: 185 additions & 1 deletion packages/core/src/TabList/TabList.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
*/

import {describe, it, expect, vi, beforeAll, afterAll} from 'vitest';
import {render, screen} from '@testing-library/react';
import {render, screen, fireEvent} from '@testing-library/react';
import userEvent from '@testing-library/user-event';
import {TabList} from './TabList';
import {Tab} from './Tab';
Expand Down Expand Up @@ -702,3 +702,187 @@ describe('TabMenu', () => {
expect(reportsItem).not.toHaveAttribute('aria-current');
});
});

describe('TabMenu keyboard navigation (roving tabindex)', () => {
const menuOptions = [
{value: 'analytics', label: 'Analytics'},
{value: 'reports', label: 'Reports'},
];

it('exposes the overflow menu as a single Tab stop (one item tabbable, rest -1)', async () => {
const user = userEvent.setup();
render(
<TabList value="home" onChange={() => {}}>
<Tab value="home" label="Home" />
<TabMenu label="More" options={menuOptions} />
</TabList>,
);

await user.click(screen.getByRole('button', {name: /More/}));

const analytics = screen.getByRole('menuitem', {
name: 'Analytics',
hidden: true,
});
const reports = screen.getByRole('menuitem', {
name: 'Reports',
hidden: true,
});

// Exactly one menu item is in the Tab sequence; arrow keys reach the rest.
expect(analytics).toHaveAttribute('tabindex', '0');
expect(reports).toHaveAttribute('tabindex', '-1');

const tabbable = [analytics, reports].filter(
el => el.getAttribute('tabindex') === '0',
);
expect(tabbable).toHaveLength(1);
});

it('moves focus between items with ArrowDown and ArrowUp', async () => {
const user = userEvent.setup();
render(
<TabList value="home" onChange={() => {}}>
<Tab value="home" label="Home" />
<TabMenu label="More" options={menuOptions} />
</TabList>,
);

await user.click(screen.getByRole('button', {name: /More/}));
const menu = screen.getByRole('menu', {hidden: true});
const analytics = screen.getByRole('menuitem', {
name: 'Analytics',
hidden: true,
});
const reports = screen.getByRole('menuitem', {
name: 'Reports',
hidden: true,
});

fireEvent.keyDown(menu, {key: 'ArrowDown'});
expect(analytics).toHaveFocus();

fireEvent.keyDown(menu, {key: 'ArrowDown'});
expect(reports).toHaveFocus();

fireEvent.keyDown(menu, {key: 'ArrowUp'});
expect(analytics).toHaveFocus();
});

it('moves the roving tab stop with arrow navigation', async () => {
const user = userEvent.setup();
render(
<TabList value="home" onChange={() => {}}>
<Tab value="home" label="Home" />
<TabMenu label="More" options={menuOptions} />
</TabList>,
);

await user.click(screen.getByRole('button', {name: /More/}));
const menu = screen.getByRole('menu', {hidden: true});
const analytics = screen.getByRole('menuitem', {
name: 'Analytics',
hidden: true,
});
const reports = screen.getByRole('menuitem', {
name: 'Reports',
hidden: true,
});

fireEvent.keyDown(menu, {key: 'ArrowDown'}); // focus analytics
fireEvent.keyDown(menu, {key: 'ArrowDown'}); // focus reports

// The tab stop follows focus, so it is still a single stop after moving.
expect(reports).toHaveAttribute('tabindex', '0');
expect(analytics).toHaveAttribute('tabindex', '-1');
});

it('jumps to first and last item with Home and End', async () => {
const user = userEvent.setup();
const options = [
{value: 'analytics', label: 'Analytics'},
{value: 'reports', label: 'Reports'},
{value: 'exports', label: 'Exports'},
];
render(
<TabList value="home" onChange={() => {}}>
<Tab value="home" label="Home" />
<TabMenu label="More" options={options} />
</TabList>,
);

await user.click(screen.getByRole('button', {name: /More/}));
const menu = screen.getByRole('menu', {hidden: true});
const analytics = screen.getByRole('menuitem', {
name: 'Analytics',
hidden: true,
});
const exports = screen.getByRole('menuitem', {
name: 'Exports',
hidden: true,
});

fireEvent.keyDown(menu, {key: 'End'});
expect(exports).toHaveFocus();

fireEvent.keyDown(menu, {key: 'Home'});
expect(analytics).toHaveFocus();
});

it('selects an item with Enter and calls onChange', async () => {
const user = userEvent.setup();
const handleChange = vi.fn();
render(
<TabList value="home" onChange={handleChange}>
<Tab value="home" label="Home" />
<TabMenu label="More" options={menuOptions} />
</TabList>,
);

await user.click(screen.getByRole('button', {name: /More/}));
const analytics = screen.getByRole('menuitem', {
name: 'Analytics',
hidden: true,
});
analytics.focus();
fireEvent.keyDown(analytics, {key: 'Enter'});

expect(handleChange).toHaveBeenCalledWith('analytics');
});

it('closes the menu when Tab is pressed inside it (APG menu-button)', async () => {
const user = userEvent.setup();
render(
<TabList value="home" onChange={() => {}}>
<Tab value="home" label="Home" />
<TabMenu label="More" options={menuOptions} />
</TabList>,
);

await user.click(screen.getByRole('button', {name: /More/}));
const menu = screen.getByRole('menu', {hidden: true});

const hidePopover = vi.mocked(HTMLElement.prototype.hidePopover);
hidePopover.mockClear();
fireEvent.keyDown(menu, {key: 'Tab'});
expect(hidePopover).toHaveBeenCalled();
});

it('closes the menu when Escape is pressed', async () => {
const user = userEvent.setup();
render(
<TabList value="home" onChange={() => {}}>
<Tab value="home" label="Home" />
<TabMenu label="More" options={menuOptions} />
</TabList>,
);

await user.click(screen.getByRole('button', {name: /More/}));
const menu = screen.getByRole('menu', {hidden: true});

const hidePopover = vi.mocked(HTMLElement.prototype.hidePopover);
hidePopover.mockClear();
fireEvent.keyDown(menu, {key: 'Escape'});
expect(hidePopover).toHaveBeenCalled();
});
});
54 changes: 45 additions & 9 deletions packages/core/src/TabList/TabMenu.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
* - /packages/cli/templates/blocks/components/TabList/ (showcase blocks)
*/

import React, {useCallback, useId, type ReactNode} from 'react';
import React, {useCallback, useId, useRef, type ReactNode} from 'react';
import * as stylex from '@stylexjs/stylex';
import {Icon} from '../Icon';
import {renderIconSlot, type IconType} from '../Icon';
Expand Down Expand Up @@ -262,6 +262,7 @@ export function TabMenu({
}: TabMenuProps) {
const tabListCtx = useTabListContext();
const menuId = useId();
const buttonRef = useRef<HTMLButtonElement>(null);

const popover = usePopover({
hasLightDismiss: true,
Expand All @@ -270,20 +271,50 @@ export function TabMenu({
// The popup's own role="menu" is the exposed semantics; a modal dialog
// wrapper would announce an unnamed dialog around the menu.
role: 'none',
// Return focus to the trigger when the menu closes (Escape, Tab, select,
// or light dismiss) so keyboard focus is never dropped to <body>.
onHide: useCallback(() => {
buttonRef.current?.focus();
}, []),
});

const {listRef, handleKeyDown: handleListKeyDown} =
useListFocus<HTMLDivElement>({
onEscape: () => popover.hide(),
});
// The overflow menu is a composite widget: a single roving tab stop with
// arrow-key navigation between items, per the APG menu pattern. The hook owns
// item tabindex — items render tabIndex={-1} and exactly one is promoted to 0.
const {
listRef,
handleKeyDown: handleListKeyDown,
handleFocus,
focusFirst,
} = useListFocus<HTMLDivElement>({
hasRovingTabIndex: true,
onEscape: () => popover.hide(),
});

const handleToggle = useCallback(() => {
if (popover.isOpen) {
popover.hide();
} else {
popover.show();
// Move focus into the menu on open so arrow navigation works and the
// roving tab stop lands on a real item (APG menu-button focus-on-open).
requestAnimationFrame(() => focusFirst());
}
}, [popover]);
}, [popover, focusFirst]);

const handleMenuKeyDown = useCallback(
(e: React.KeyboardEvent) => {
// APG menu-button: Tab leaves the menu rather than walking its items.
// Close and let onHide return focus to the trigger, from which the
// browser's default Tab continues to the next element.
if (e.key === 'Tab') {
popover.hide();
return;
}
handleListKeyDown(e);
},
[handleListKeyDown, popover],
);

const selectedOption = options.find(o => o.value === tabListCtx.value);
const triggerLabel = selectedOption?.label ?? label;
Expand All @@ -299,7 +330,11 @@ export function TabMenu({
[tabListCtx, popover],
);

const setButtonRef = mergeRefs<HTMLButtonElement>(popover.triggerRef, ref);
const setButtonRef = mergeRefs<HTMLButtonElement>(
popover.triggerRef,
buttonRef,
ref,
);

return (
<>
Expand Down Expand Up @@ -357,7 +392,8 @@ export function TabMenu({
id={menuId}
role="menu"
aria-label={label}
onKeyDown={handleListKeyDown}
onKeyDown={handleMenuKeyDown}
onFocus={handleFocus}
{...mergeProps(
themeProps('tab-menu-dropdown'),
stylex.props(styles.dropdown),
Expand All @@ -371,7 +407,7 @@ export function TabMenu({
<div
key={option.value}
role="menuitem"
tabIndex={0}
tabIndex={-1}
aria-current={isSelected ? 'true' : undefined}
onClick={() => handleSelect(option.value)}
onKeyDown={e => {
Expand Down
Loading