From bf66afb6f9058036bf0aa05060aa0012902d464d Mon Sep 17 00:00:00 2001 From: Baraa Hamodi Date: Thu, 9 Jul 2026 00:22:47 -0700 Subject: [PATCH] fix(core): use roving tabindex in TabList overflow menu --- .changeset/tabmenu-roving.md | 6 + packages/core/src/TabList/TabList.test.tsx | 186 ++++++++++++++++++++- packages/core/src/TabList/TabMenu.tsx | 54 +++++- 3 files changed, 236 insertions(+), 10 deletions(-) create mode 100644 .changeset/tabmenu-roving.md diff --git a/.changeset/tabmenu-roving.md b/.changeset/tabmenu-roving.md new file mode 100644 index 000000000000..f07a41d78a0c --- /dev/null +++ b/.changeset/tabmenu-roving.md @@ -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 diff --git a/packages/core/src/TabList/TabList.test.tsx b/packages/core/src/TabList/TabList.test.tsx index b061dda0f481..d26e3b9b39fc 100644 --- a/packages/core/src/TabList/TabList.test.tsx +++ b/packages/core/src/TabList/TabList.test.tsx @@ -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'; @@ -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( + {}}> + + + , + ); + + 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( + {}}> + + + , + ); + + 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( + {}}> + + + , + ); + + 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( + {}}> + + + , + ); + + 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( + + + + , + ); + + 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( + {}}> + + + , + ); + + 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( + {}}> + + + , + ); + + 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(); + }); +}); diff --git a/packages/core/src/TabList/TabMenu.tsx b/packages/core/src/TabList/TabMenu.tsx index 9232037934a4..41fec707722e 100644 --- a/packages/core/src/TabList/TabMenu.tsx +++ b/packages/core/src/TabList/TabMenu.tsx @@ -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'; @@ -262,6 +262,7 @@ export function TabMenu({ }: TabMenuProps) { const tabListCtx = useTabListContext(); const menuId = useId(); + const buttonRef = useRef(null); const popover = usePopover({ hasLightDismiss: true, @@ -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 . + onHide: useCallback(() => { + buttonRef.current?.focus(); + }, []), }); - const {listRef, handleKeyDown: handleListKeyDown} = - useListFocus({ - 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({ + 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; @@ -299,7 +330,11 @@ export function TabMenu({ [tabListCtx, popover], ); - const setButtonRef = mergeRefs(popover.triggerRef, ref); + const setButtonRef = mergeRefs( + popover.triggerRef, + buttonRef, + ref, + ); return ( <> @@ -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), @@ -371,7 +407,7 @@ export function TabMenu({
handleSelect(option.value)} onKeyDown={e => {