diff --git a/.changeset/dropdown-menu-radio-cell.md b/.changeset/dropdown-menu-radio-cell.md new file mode 100644 index 0000000000000..632a9bf3cb4a1 --- /dev/null +++ b/.changeset/dropdown-menu-radio-cell.md @@ -0,0 +1,7 @@ +--- +'@astryxdesign/core': patch +--- + +[feat] DropdownMenu: a `DropdownMenuRadioItem` given children that render is a cell. The children are the tile (a colour swatch, an icon, a numbered slot) instead of the row, with no radio mark, named by `label`; the checked cell carries an accent border, and keyboard focus adds the focus ring. Cells keep the row's role, focus and selection, so the arrow keys and typeahead reach them in DOM order; lay them out with a grid on the group or on a wrapper inside it. `ContextMenuRadioItem` and `BreadcrumbMenuRadioItem` are the same component and take children too. + +@vjeux diff --git a/apps/storybook/stories/DropdownMenu.stories.tsx b/apps/storybook/stories/DropdownMenu.stories.tsx index 93639a6f58308..4d119ec809a3d 100644 --- a/apps/storybook/stories/DropdownMenu.stories.tsx +++ b/apps/storybook/stories/DropdownMenu.stories.tsx @@ -869,6 +869,126 @@ export const LabRadioGroup: Story = { }, }; +// Radio cells: a radio item given children is a cell, laid out by the caller. +const radioCellStyles = stylex.create({ + swatchGrid: { + display: 'grid', + gridTemplateColumns: 'repeat(4, max-content)', + gap: spacingVars['--spacing-1'], + }, + swatch: { + display: 'block', + width: 24, + height: 24, + borderRadius: '50%', + // A tile drawn as a background colour keeps it under forced colors. + forcedColorAdjust: 'none', + }, + swatchColor: (color: string) => ({ + backgroundColor: color, + }), + tileGrid: { + display: 'grid', + gridTemplateColumns: 'repeat(3, 1fr)', + gap: spacingVars['--spacing-1'], + }, + tile: { + display: 'flex', + flexDirection: 'column', + alignItems: 'center', + gap: spacingVars['--spacing-0-5'], + paddingBlock: spacingVars['--spacing-1'], + }, + tileIcon: { + width: 20, + height: 20, + }, +}); + +const RADIO_CELL_SWATCHES = [ + {value: 'red', label: 'Red', color: '#e5484d'}, + {value: 'orange', label: 'Orange', color: '#f76b15'}, + {value: 'green', label: 'Green', color: '#30a46c'}, + {value: 'blue', label: 'Blue', color: '#0090ff'}, + {value: 'purple', label: 'Purple', color: '#8e4ec6'}, + {value: 'pink', label: 'Pink', color: '#d6409f'}, + {value: 'teal', label: 'Teal', color: '#12a594'}, + {value: 'gray', label: 'Gray', color: '#8b8d98'}, +]; + +const RADIO_CELL_TILES = [ + {value: 'pencil', label: 'Pencil', Icon: PencilIcon}, + {value: 'share', label: 'Share', Icon: ShareIcon}, + {value: 'archive', label: 'Archive', Icon: ArchiveBoxIcon}, + {value: 'person', label: 'Person', Icon: UserIcon}, + {value: 'gear', label: 'Gear', Icon: Cog6ToothIcon}, + {value: 'search', label: 'Search', Icon: MagnifyingGlassIcon}, +]; + +export const RadioCells: Story = { + render: function RadioCellsStory() { + const [color, setColor] = useState('blue'); + const [icon, setIcon] = useState('default'); + return ( + + + {RADIO_CELL_SWATCHES.map(swatch => ( + + + ))} + + + + + + Default icon + +
+ {RADIO_CELL_TILES.map(({value, label, Icon}) => ( + + + + + ))} +
+
+
+
+ ); + }, + parameters: { + docs: { + description: { + story: + 'A DropdownMenuRadioItem given children is a cell: the children are the tile, with no radio mark, the label names it, and the checked cell carries an accent border. The caller lays cells out: here the colour group is a grid, and the tab-icon tiles sit in a grid inside their group, after a cell drawn as a row. The arrow keys move through cells in DOM order, as through rows.', + }, + }, + }, +}; + export const LabSelectableSizes: Story = { render: function LabSelectableSizesStory() { const [sm, setSm] = useState('a'); diff --git a/packages/core/src/DropdownMenu/DropdownMenu.doc.mjs b/packages/core/src/DropdownMenu/DropdownMenu.doc.mjs index 949d2c551a941..a5152d1200429 100644 --- a/packages/core/src/DropdownMenu/DropdownMenu.doc.mjs +++ b/packages/core/src/DropdownMenu/DropdownMenu.doc.mjs @@ -49,6 +49,12 @@ const anatomy = [ description: 'Decorative shared radio indicator with an additional menu-owned target.', }, + { + name: 'Radio cell', + required: false, + description: + 'A radio item given children: the caller content as a tile, named by its label, with an accent border when checked.', + }, { name: 'Pointer section heading', required: false, diff --git a/packages/core/src/DropdownMenu/DropdownMenu.spec.md b/packages/core/src/DropdownMenu/DropdownMenu.spec.md index d1091b9a15444..3c687e583ce21 100644 --- a/packages/core/src/DropdownMenu/DropdownMenu.spec.md +++ b/packages/core/src/DropdownMenu/DropdownMenu.spec.md @@ -235,6 +235,7 @@ visibility helper. | Caller-rendered item start content | Presents arbitrary React content directly in an action-row start slot. | Caller-supplied content | Context-dependent | FR1, FR3 | | Checkbox indicator | Draws the decorative checkbox state for a checkbox action row. | `component:Indicator` | Supporting | FR1, FR3 | | Radio indicator | Draws shared radio chrome with the current menu-specific radio target. | Current source and Indicator | Supporting | FR1, FR2, FR3 | +| Radio cell | Draws caller content as a tile; the checked cell has an accent border. | DropdownMenuRadioItem DEC-1 | Context-dependent | RadioItem FR9 | | Pointer section heading | Labels a grouped set of pointer action rows. | Current source and public docs | Supporting | FR1, FR2 | | Pointer divider | Separates groups of pointer action rows. | Current source and public docs | Supporting | FR1, FR2 | | Pointer submenu indicator icon | Identifies a pointer action row that opens a nested flyout. | Current source and public docs | Supporting | FR1, FR2 | @@ -280,6 +281,11 @@ than adding a DropdownMenu-owned heading target. } }, "Radio indicator": {"target": "dropdown-menu-radio"}, + "Radio cell": { + "none": { + "reason": "intentional: The tile is caller content; its checked border reads the accent token and its focus ring the shared focus tokens, so it adds no DropdownMenu target." + } + }, "Pointer section heading": {"target": "dropdown-menu-section-heading"}, "Pointer divider": {"target": "dropdown-menu-divider"}, "Pointer submenu indicator icon": { diff --git a/packages/core/src/DropdownMenu/DropdownMenuRadioCell.test.tsx b/packages/core/src/DropdownMenu/DropdownMenuRadioCell.test.tsx new file mode 100644 index 0000000000000..534b92fa87c29 --- /dev/null +++ b/packages/core/src/DropdownMenu/DropdownMenuRadioCell.test.tsx @@ -0,0 +1,328 @@ +// Copyright (c) Meta Platforms, Inc. and affiliates. + +/** + * @file DropdownMenuRadioCell.test.tsx + * @input Uses vitest, Testing Library, DropdownMenu, DropdownMenuRadioGroup + * and DropdownMenuRadioItem + * @output Tests for a radio item given children: a cell drawn from the + * caller's content, named by its label, that keeps the row's role, focus and + * selection + * @position Testing; validates the cell form of DropdownMenuRadioItem.tsx + * + * SYNC: When DropdownMenuRadioItem.tsx changes its cell form, update these tests. + */ + +import {describe, it, expect, vi, beforeEach} from 'vitest'; +import { + render, + screen, + fireEvent, + waitFor, + within, +} from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; +import * as stylex from '@stylexjs/stylex'; +import {focusOutlineStyles} from '../utils/focusOutline.stylex'; +import {DropdownMenu} from './DropdownMenu'; +import {DropdownMenuItem} from './DropdownMenuItem'; +import {DropdownMenuRadioGroup} from './DropdownMenuRadioGroup'; +import {DropdownMenuRadioItem} from './DropdownMenuRadioItem'; + +beforeEach(() => { + HTMLElement.prototype.showPopover = vi.fn(function (this: HTMLElement) { + this.setAttribute('popover-open', ''); + const event = new Event('toggle', {bubbles: false}); + Object.defineProperty(event, 'newState', {value: 'open'}); + this.dispatchEvent(event); + }); + HTMLElement.prototype.hidePopover = vi.fn(function (this: HTMLElement) { + this.removeAttribute('popover-open'); + const event = new Event('toggle', {bubbles: false}); + Object.defineProperty(event, 'newState', {value: 'closed'}); + this.dispatchEvent(event); + }); + const originalMatches = HTMLElement.prototype.matches; + // eslint-disable-next-line @typescript-eslint/no-explicit-any + (HTMLElement.prototype as any).matches = function ( + selector: string, + ): boolean { + if (selector === ':popover-open') { + return this.hasAttribute('popover-open'); + } + return originalMatches.call(this, selector); + }; +}); + +const COLORS = [ + {value: 'red', label: 'Red'}, + {value: 'green', label: 'Green'}, + {value: 'blue', label: 'Blue'}, +]; + +const ignoreChange = () => {}; + +function ColorMenu({ + value = 'red', + onChange = ignoreChange, +}: { + value?: string; + onChange?: (value: string) => void; +}) { + return ( + + + + {COLORS.map(color => ( + + + + ))} + + + ); +} + +function cell(name: string): HTMLElement { + return screen.getByRole('menuitemradio', {name, hidden: true}); +} + +const RING_CLASSES = stylex + .props(focusOutlineStyles.focusVisible) + .className!.split(' '); + +// The ring's classes on a cell that another, unfocused and unchecked cell +// lacks: the ones the keyboard ring adds. +function ringClassesOn(element: HTMLElement, reference: HTMLElement): string[] { + return RING_CLASSES.filter( + name => + element.classList.contains(name) && !reference.classList.contains(name), + ); +} + +describe('DropdownMenuRadioItem with children (a cell)', () => { + it('draws its children as the cell, named by its label, with no radio mark', async () => { + const user = userEvent.setup(); + render(); + await user.click(screen.getByRole('button', {name: /Color/})); + + const red = cell('Red'); + const green = cell('Green'); + expect(within(red).getByTestId('swatch-red')).toBeInTheDocument(); + expect(red).toHaveAttribute('aria-checked', 'true'); + expect(green).toHaveAttribute('aria-checked', 'false'); + expect(red).toHaveAttribute('tabindex', '-1'); + // No row: no radio mark and no row label slot, only the hidden name. + expect(red.querySelector('.astryx-radio-indicator')).toBeNull(); + expect(red.querySelector('.astryx-dropdown-menu-radio')).toBeNull(); + expect(red).toHaveTextContent(/^Red$/); + // The checked cell is painted apart from the others. + expect(red.className).not.toBe(green.className); + expect(cell('Blue').className).toBe(green.className); + }); + + it('selects on a click and closes the menu, as a row does', async () => { + const user = userEvent.setup(); + const onChange = vi.fn(); + render(); + await user.click(screen.getByRole('button', {name: /Color/})); + + await user.click(cell('Green')); + expect(onChange).toHaveBeenCalledWith('green'); + expect(HTMLElement.prototype.hidePopover).toHaveBeenCalledTimes(1); + }); + + it('is reached by the arrow keys in DOM order and found by typeahead on its label', async () => { + const user = userEvent.setup(); + const onChange = vi.fn(); + render(); + await user.click(screen.getByRole('button', {name: /Color/})); + const menu = screen.getByRole('menu', {hidden: true}); + await waitFor(() => expect(menu).toHaveFocus()); + + fireEvent.keyDown(menu, {key: 'ArrowDown'}); + expect( + screen.getByRole('menuitem', {name: 'Reset', hidden: true}), + ).toHaveFocus(); + fireEvent.keyDown(menu, {key: 'ArrowDown'}); + expect(cell('Red')).toHaveFocus(); + fireEvent.keyDown(menu, {key: 'ArrowDown'}); + expect(cell('Green')).toHaveFocus(); + + fireEvent.keyDown(menu, {key: 'b'}); + expect(cell('Blue')).toHaveFocus(); + fireEvent.keyDown(menu, {key: 'Enter'}); + expect(onChange).toHaveBeenCalledWith('blue'); + }); + + it('reaches cells laid out in a wrapper inside the group', async () => { + const user = userEvent.setup(); + render( + + {}} label="Icon"> + + Default icon + +
+ + + + + + +
+
+
, + ); + await user.click(screen.getByRole('button', {name: /Icon/})); + const menu = screen.getByRole('menu', {hidden: true}); + await waitFor(() => expect(menu).toHaveFocus()); + + fireEvent.keyDown(menu, {key: 'ArrowDown'}); + expect(cell('Default icon')).toHaveFocus(); + fireEvent.keyDown(menu, {key: 'ArrowDown'}); + expect(cell('Cat')).toHaveFocus(); + expect(cell('Cat')).toHaveAttribute('aria-checked', 'true'); + expect( + within(screen.getByTestId('grid')).getAllByRole('menuitemradio', { + hidden: true, + }), + ).toHaveLength(2); + }); + + it('keeps a disabled cell focusable and selects nothing from it', async () => { + const user = userEvent.setup(); + const onChange = vi.fn(); + render( + + + + 1 + + + 2 + + + , + ); + await user.click(screen.getByRole('button', {name: /Slot/})); + + const disabled = cell('Slot 2'); + expect(disabled).toHaveAttribute('aria-disabled', 'true'); + await user.click(disabled); + expect(onChange).not.toHaveBeenCalled(); + expect(HTMLElement.prototype.hidePopover).not.toHaveBeenCalled(); + }); + + it('stays a row when its children render nothing', async () => { + const user = userEvent.setup(); + const showSwatch = false; + render( + + {}} label="Color"> + + {showSwatch && } + + + {''} + + + {showSwatch && } + {showSwatch && } + + + {[].map(id => ( + + ))} + + + , + ); + await user.click(screen.getByRole('button', {name: /Color/})); + // Each is a row, with its radio mark, not an empty cell, also when the + // children are an array of nothing. + for (const name of ['Red', 'Green', 'Blue', 'Gray']) { + expect( + cell(name).querySelector('.astryx-radio-indicator'), + ).not.toBeNull(); + } + }); + + it('is a cell for a fragment or a component that renders nothing, as an element cannot be read before it renders', async () => { + const user = userEvent.setup(); + const showSwatch = false; + function Nothing() { + return null; + } + render( + + {}} label="Color"> + + <> + {showSwatch && } + {showSwatch && } + + + + + + + , + ); + await user.click(screen.getByRole('button', {name: /Color/})); + for (const name of ['Red', 'Green']) { + expect(cell(name).querySelector('.astryx-radio-indicator')).toBeNull(); + } + }); + + it('paints the keyboard ring for focus after keyboard input, also where the mouse then moves it, and none after pointer input', async () => { + const user = userEvent.setup(); + render(); + // A pointer open: the cells the mouse rests on take focus, with no ring. + await user.click(screen.getByRole('button', {name: /Color/})); + const menu = screen.getByRole('menu', {hidden: true}); + await waitFor(() => expect(menu).toHaveFocus()); + fireEvent.pointerMove(cell('Green'), {pointerType: 'mouse'}); + expect(cell('Green')).toHaveFocus(); + expect(ringClassesOn(cell('Green'), cell('Blue'))).toEqual([]); + + // A key takes over: the cell it reaches carries the ring. + await user.keyboard('{ArrowDown}'); + expect(cell('Blue')).toHaveFocus(); + expect( + ringClassesOn(cell('Blue'), cell('Green')).length, + ).toBeGreaterThanOrEqual(3); + + // The last input is still the keyboard: focus the mouse moves keeps it. + fireEvent.pointerMove(cell('Green'), {pointerType: 'mouse'}); + expect(cell('Green')).toHaveFocus(); + expect( + ringClassesOn(cell('Green'), cell('Red')).length, + ).toBeGreaterThanOrEqual(3); + }); + + it('warns in development when a cell is given an icon, a description or end content', async () => { + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); + try { + const user = userEvent.setup(); + render( + + {}} label="Color"> + + + + + , + ); + await user.click(screen.getByRole('button', {name: /Color/})); + expect(warn).toHaveBeenCalledWith( + expect.stringContaining('is a cell, which shows no icon, description'), + ); + expect(cell('Red')).not.toHaveTextContent('Warm'); + } finally { + warn.mockRestore(); + } + }); +}); diff --git a/packages/core/src/DropdownMenu/DropdownMenuRadioItem.doc.mjs b/packages/core/src/DropdownMenu/DropdownMenuRadioItem.doc.mjs index ef2eaa06b619a..f20262e0517ae 100644 --- a/packages/core/src/DropdownMenu/DropdownMenuRadioItem.doc.mjs +++ b/packages/core/src/DropdownMenu/DropdownMenuRadioItem.doc.mjs @@ -60,6 +60,12 @@ export const docs = { description: 'Content to render after the label and description, such as a badge or metadata.', }, + { + name: 'children', + type: 'ReactNode', + description: + "Content drawn as a cell instead of the row: a tile, a colour swatch, an icon, a numbered slot. The cell shows no radio mark; label names it through a visually hidden element, the checked cell carries an accent border (Highlight under forced colors), and keyboard focus adds the focus ring. Cells keep the row role, focus, selection and handlers, so the arrow keys and typeahead reach them in DOM order. Lay them out by styling the group or a wrapper inside it as a grid. Children that render nothing, also an array of nothing, leave the row; an element always makes a cell, even a fragment or a component that returns null. A tile drawn as a background colour needs `forcedColorAdjust: 'none'` to keep it under forced colors. icon, description and endContent are not shown in a cell.", + }, { name: 'ref', type: 'React.Ref', @@ -82,6 +88,8 @@ export const docsZh = { icon: '显示在标签前的图标。', isDisabled: '该单选项是否禁用。', endContent: '在标签和描述之后渲染的内容。', + children: + "以单元格而非行绘制的内容:图块、色板、图标或编号槽位。单元格不显示单选标记;label 通过视觉隐藏的元素为其命名,选中的单元格带有强调色边框(强制颜色模式下为 Highlight),键盘焦点会显示焦点环。单元格保留行的角色、焦点、选择和事件处理,方向键和输入查找按 DOM 顺序到达它们。通过将组或组内的包装元素设为网格来布局。不渲染任何内容的 children(包括只含空内容的数组)保持为行;元素总会生成单元格,即使是片段或返回 null 的组件。以背景色绘制的图块需要 `forcedColorAdjust: 'none'`,才能在强制颜色模式下保留颜色。单元格中不显示 icon、description 和 endContent。", ref: '转发到行根元素(带 role="menuitemradio" 的元素)的 ref。', }, }; @@ -99,6 +107,8 @@ export const docsDense = { isDisabled: 'disabled; aria-disabled, blocks selection, skipped by arrow keys', endContent: 'content after label+description', + children: + 'drawn as a cell (tile, swatch, icon) instead of the row: no radio mark, named by label, accent border when checked, focus ring on keyboard focus; arrows reach cells in DOM order; lay out via group/wrapper grid; non-rendering children (arrays too) stay a row, any element is a cell; background-colour tiles need forcedColorAdjust none', ref: 'forwarded to the row root (the role="menuitemradio" element)', }, }; diff --git a/packages/core/src/DropdownMenu/DropdownMenuRadioItem.spec.md b/packages/core/src/DropdownMenu/DropdownMenuRadioItem.spec.md index 1f8dbff77d38e..6acef3a04f61c 100644 --- a/packages/core/src/DropdownMenu/DropdownMenuRadioItem.spec.md +++ b/packages/core/src/DropdownMenu/DropdownMenuRadioItem.spec.md @@ -13,6 +13,8 @@ review_triggers: [public-api, behavior, accessibility, theming, testing] verified_by: [ packages/core/src/DropdownMenu/DropdownMenuSelectable.test.tsx, + packages/core/src/DropdownMenu/DropdownMenuRadioCell.test.tsx, + packages/core/src/DropdownMenu/__tests__/RadioCell.a11y.chromium.spec.ts, packages/core/src/DropdownMenu/__tests__/Menu.a11y.test.tsx, packages/core/src/RadioList/__tests__/RadioGroup.a11y.test.tsx, packages/core/src/DropdownMenu/__tests__/DropdownMenuRadioItemAudit.a11y.chromium.spec.ts, @@ -40,7 +42,7 @@ system_specs: [spec:AST-020, spec:AST-029] | Area | Contract | | ----------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| Public contract | One `menuitemradio` row inside a DropdownMenuRadioGroup with a required value and label, optional description, icon, end content, disabled state, root inputs, and an explicit React 19 ref. | +| Public contract | One `menuitemradio` row, or a cell given children (proposed), in a DropdownMenuRadioGroup; required value and label; optional description, icon, end content, disabled state, inputs, a ref. | | Behavior | Enabled activation emits the row value through the group, applies the group close policy, then runs the caller click handler; disabled activation does none of these. | | End-user impact | Assistive technology hears one radio option with its checked state; pointer and keyboard selection commit the same value, and a disabled option cannot be chosen. | | Builder impact | Place the row inside a DropdownMenuRadioGroup and give it a unique value and a label; caller handlers run alongside the row's own behavior. | @@ -57,13 +59,17 @@ the pointer presentation of DropdownMenu. The row owns its `menuitemradio` semantics, checked state, decorative radio marker, visible content, disabled paint, and activation order. DropdownMenuRadioGroup owns the controlled value and close policy; DropdownMenu owns focus movement, typeahead, keyboard -activation, and the menu lifecycle. +activation, and the menu lifecycle. Given children that render, the option is +a cell instead of a row: the children are its tile, named by its label, with +no radio marker (DEC-1, pending owner review). ## Compatibility and migration - Released default preserved: yes (`isDisabled=false`). - Compatibility class: no public API change; caller `onClick` and `onPointerMove` compose with the row's own behavior. +- Proposed (DEC-1): optional `children` make the option a cell; omitted, the + row is unchanged. - Controlled behavior: the checked state always derives from the group `value`; the row holds no selection state. - Migration decision: none. @@ -78,8 +84,12 @@ Consumer migration instructions belong in consumer docs and release notes. group value. - Selection order: disabled guard, group `onChange(value)`, optional close, then the caller click handler. -- The decorative shared radio marker, its size, and its inline placement. -- Label, description, icon, and end content composition through Item. +- The decorative shared radio marker, its size, and its inline placement, for + a row. +- Label, description, icon, and end content composition through Item, for a + row. +- The cell form (DEC-1, proposed): caller children as the tile, a visually + hidden label, the checked border, and a focus ring for keyboard focus. - Disabled state exposure and paint, and hover focus for enabled rows. - Root composition of ref, supported host inputs, `className`, `style`, and `xstyle`. @@ -103,6 +113,7 @@ Consumer migration instructions belong in consumer docs and release notes. | Availability | enabled; disabled | Whether the row can take hover focus and commit its value. | Every row | enabled | `component:DropdownMenuRadioItem` | stable | Disabled blocks every activation path. | | Content | label; description; icon; end content | Visible identification of the option, in reading order. | Every row | label | row through Item | stable | `label` is required by the public type. | | Root surface | ref; DOM/data/ARIA; class/style/xstyle; events | Extends the row root while preserving its role and checked state. | Every row | omitted | public component API | stable | `role`, `aria-checked`, and `tabIndex` are not accepted. | +| Presentation | row; cell (children that render) | A row with a marker, or a tile of caller content (a cell). | Every option | row | `component:DropdownMenuRadioItem` | proposed | Non-rendering children leave a row. | ## Behavioral and layout contract @@ -116,9 +127,10 @@ intentional decision. | FR3 | Enabled activation, by pointer or by the parent's synthesized Enter/Space click, MUST call the group `onChange` with the row value, then ask DropdownMenu to close when the group's close policy is on, then call the caller `onClick`. | Released behavior; `architecture:public-component-api/INV7`; focused tests | verify | | FR4 | A disabled row MUST expose `aria-disabled="true"`, render no `tabindex`, take no hover focus, and on any activation, including an assistive-technology click, MUST NOT call `onChange`, close the menu, or call the caller `onClick`. It paints at half opacity with a default cursor. | Current source, focused tests, and browser evidence | verify | | FR5 | Ref, supported host attributes and handlers, `className`, `style`, and `xstyle` MUST reach or compose on the row root. Caller `onPointerMove` MUST run after the row's hover focus, and caller handlers MUST NOT replace the row's own behavior. | `architecture:public-component-api/INV5–INV8`; focused tests | verify | -| FR6 | The marker MUST be the shared radio Indicator, hidden from assistive technology, never the pointer target, sized `sm` in a small menu and `md` otherwise, at the row's inline start on a fine pointer and its inline end on a coarse pointer. | Current source, focused tests, and browser evidence | verify | -| FR7 | The row MUST carry the `dropdown-menu-item` target with the menu size and the marker MUST carry the `dropdown-menu-radio` target with size, checked, and disabled reflections. | `architecture:component-theming-surface/INV4–INV6`; current source/docs | verify | +| FR6 | A row's marker MUST be the shared radio Indicator, hidden from assistive technology, never the pointer target, sized `sm` in a small menu and `md` otherwise, at the row's inline start on a fine pointer and its inline end on a coarse pointer. | Current source, focused tests, and browser evidence | verify | +| FR7 | A row MUST carry the `dropdown-menu-item` target with the menu size and the marker MUST carry the `dropdown-menu-radio` target with size, checked, and disabled reflections. A cell carries neither (DEC-1). | `architecture:component-theming-surface/INV4–INV6`; current source/docs | verify | | FR8 | The row MUST add no key handler or tab stop of its own: enabled rows are `tabindex="-1"` for the parent's roving focus, and keyboard selection arrives as the parent's synthesized click. | `architecture:interaction-modality`; parent keyboard tests | verify | +| FR9 | Given children that render, the option MUST be a cell: one `menuitemradio` element holding them, no marker or Item slots, named by `label` through a visually hidden element; it keeps FR1, FR3–FR5 and FR8. Children that render nothing leave a row; row-only content warns. | Proposed DEC-1; cell tests | proposed (DEC-1) | ### Allowed variation @@ -140,6 +152,7 @@ intentional decision. | Disabled | `aria-disabled=true`, no tab stop, half opacity, and no activation. | Checked or unchecked value. | | Coarse pointer | The marker sits at the inline end of the row. | Direction mirrors the placement. | | RTL / narrow | Content and marker follow logical order with no horizontal page overflow. | Parent menu placement and width. | +| Cell | Children as the tile, named by `label`; accent border when checked. | Keyboard focus adds the ring; layout is the caller's. | ### Transformation and precedence order @@ -153,13 +166,14 @@ intentional decision. ### Performance and resources - **PR1 — Render only.** The row owns no Effect, listener, observer, timer, - request, or local state. + request, or local state. A cell keeps one flag, whether the keyboard moved + focus to it, and activates the shared modality tracking (AR5). ## Accessibility contract - **AR1 — One radio option.** The row is the only announced control; its accessible name comes from its visible content and its state from - `aria-checked`. + `aria-checked`. A cell's name comes from its `label`, not its tile (FR9). - **AR2 — Disabled state.** A disabled row exposes `aria-disabled` and cannot commit a value through any input. - **AR3 — Keyboard ownership.** DropdownMenu owns focus movement and activation; @@ -170,7 +184,15 @@ intentional decision. only as the menu overlay tint with no outline, which measures about 1.1:1 against the menu surface and paints nothing under forced colors. The remediation is a shared row-visual decision with `component:DropdownMenuItem` - (its OQ2); this draft records the gap and does not settle it. + (its OQ2); this draft records the gap and does not settle it. A cell does + not share the gap (AR5). +- **AR5 — A cell's checked state and keyboard focus are visible (proposed, + DEC-1).** A checked cell carries a 2px `--color-accent` border every cell + reserves, transparent when unchecked and Highlight against Canvas under forced + colors. The shared focus ring paints outside it for focus after keyboard + input, also where the mouse then moves it; focus after pointer input paints + none, the browser's ring included (`architecture:interaction-modality` + INV1). ## Design relationships @@ -180,6 +202,7 @@ intentional decision. | Radio marker | Shared radio Indicator at the menu's control size. | Shared radio Indicator | prominent | FR1, FR6 | | Disabled | Half-opacity row with a default cursor. | Current source | supporting | FR4 | | Direction/touch | Logical marker placement that mirrors and moves on touch. | Bidirectionality and current CSS | structural | FR6 | +| Cell | Caller tile; accent border when checked; keyboard ring outside. | Proposed DEC-1 | prominent | FR9, AR5 | No new component-local visual decision is settled by this draft. @@ -206,10 +229,34 @@ No new component-local visual decision is settled by this draft. | FR4, AR2 | Disabled focused test with an assistive-technology click; browser disabled-hover frame | disabled checked/unchecked | Removing the disabled guard makes the focused test fail | `audit:DropdownMenuRadioItem/§1,§4` | | AR4 | Exact-head keyboard-focus and forced-colors frames with rendered contrast | Neutral/Butter light/dark focus, forced colors | Current frames record the gap; a remediation passes only with a perceivable indicator in every theme and under forced colors | `audit:DropdownMenuRadioItem/§1,§5` | | FR6, FR7 | Marker and target tests; exact-head Neutral/Butter/RTL/narrow/size/coarse/focus frames | sm/md/lg, fine/coarse, LTR/RTL, light/dark | A lost target or reflection, a marker that is announced, wrongly sized, or misplaced fails | `audit:DropdownMenuRadioItem/§2,§5,§9,§10` | +| FR9, AR1, AR5 | `DropdownMenuRadioCell.test.tsx`; `RadioCell.a11y.chromium.spec.ts` (AX names, paint, keys, axe) | cell, non-rendering children, pointer/keyboard focus, forced | A marker or row slot in a cell, a cell named by its tile, a lost checked border, or a ring after pointer input fails | `audit:DropdownMenuRadioItem/§1,§4,§5` | ## Decision log -None. This draft records current facts and introduces no product decision. +### DEC-1 (proposed) — A radio item given children is a cell + +**Reference:** `component:DropdownMenuRadioItem/DEC-1` +**Decider:** proposed by `vjeux`, 2026-10-24; pending owner review + +A one-of choice drawn as tiles (colour swatches, icons, numbered slots) is +still a radio set in a menu, so each tile is a `DropdownMenuRadioItem`: given +children that render, the item draws them as a cell instead of its row, with no +radio marker, and names it by its `label` through a visually hidden element, so +the name stays the label when the tile shows text of its own. The selection is +painted on the cell: a border every cell reserves, so checking moves nothing, +and a border rather than a shadow because forced colors keep borders; there it +is Highlight against Canvas. Keyboard focus adds the shared ring outside that +border; focus the menu moves to a cell under the mouse after a pointer open adds +none. Cells keep the row's role, focus, selection, handler order and press +model, so the menu's arrow keys move through them in DOM order and ArrowLeft and +ArrowRight keep their sub-menu meanings. The caller lays cells out, with a grid +on the group or on a wrapper inside it. A cell carries no theming target of its +own; its paint reads the accent and focus tokens (`component:DropdownMenu` +owns the target inventory). +Rejected: a grid variant of `DropdownMenuRadioGroup`, which decides the tiles' +size and columns and still needs caller content for a swatch. +Rejected: exporting the group's context for a custom cell, which leaves every +caller to rebuild the role, focus and paint. ## Open questions diff --git a/packages/core/src/DropdownMenu/DropdownMenuRadioItem.tsx b/packages/core/src/DropdownMenu/DropdownMenuRadioItem.tsx index 57f2125886ed1..f2349a5edea1a 100644 --- a/packages/core/src/DropdownMenu/DropdownMenuRadioItem.tsx +++ b/packages/core/src/DropdownMenu/DropdownMenuRadioItem.tsx @@ -5,7 +5,8 @@ /** * @file DropdownMenuRadioItem.tsx * @input React, stylex, Item + Icon + DropdownMenu context + tokens from core - * @output DropdownMenuRadioItem — a single option in a radio group. + * @output DropdownMenuRadioItem — a single option in a radio group, drawn as a + * row or, given children, as a cell * @position Sub-component; must be used inside a DropdownMenuRadioGroup. * * A menu item representing one option in a single-select group @@ -20,25 +21,61 @@ * same `radio` theming (and any theme replacement) as RadioList. This row keeps * the marker box: its size is derived from the menu's item size and it swaps to * the inline-end of the row on coarse-pointer (touch) devices via CSS `order`. + * + * Given children that render, the item is a cell instead: the caller's content + * is the tile (a swatch, an icon, a numbered slot), with no radio mark and no + * row layout, named by `label` through a visually hidden element. The checked + * cell carries an accent border, and focus after keyboard input the shared + * ring; focus after pointer input paints none. Cells keep the row's role, focus, + * selection and handler order, so the menu's arrow keys, typeahead and press + * model reach them in DOM order; a caller lays them out by styling the group or + * a wrapper as a grid. */ import { useCallback, + useId, + useState, + type FocusEvent, type MouseEvent, type PointerEvent, type ReactNode, + type Ref, } from 'react'; import * as stylex from '@stylexjs/stylex'; import {renderIconSlot, type IconType} from '../Icon'; import {useIndicator} from '../Indicator'; import {Item} from '../Item'; +import {VisuallyHidden} from '../VisuallyHidden'; +import {useDevWarning} from '../hooks/useDevWarning'; import {useDropdownMenuContext} from './DropdownMenuContext'; import {focusMenuItemOnHover} from './menuItemHover'; +import {focusOutlineStyles} from '../utils/focusOutline.stylex'; import {colorVars, spacingVars} from '../theme/tokens.stylex'; -import {composeEventHandlers, mergeProps, themeProps} from '../utils'; +import { + composeEventHandlers, + isRenderable, + mergeProps, + themeProps, +} from '../utils'; +import { + getInteractionModality, + useInteractionModalityTracking, +} from '../utils/interactionModality'; import type {BaseProps} from '../BaseProps'; import {useDropdownMenuRadioGroupContext} from './DropdownMenuContext'; +/** + * Whether children render something React can tell before rendering. An array + * counts only through an item that does, so `{a && }{b && }` with + * both false, or an empty `.map()`, renders nothing. An element always counts: + * a fragment, or a component that returns `null`, is only read when it renders. + */ +function rendersContent(children: ReactNode): boolean { + return Array.isArray(children) + ? children.some(rendersContent) + : isRenderable(children); +} const styles = stylex.create({ root: { width: '100%', @@ -58,6 +95,52 @@ const styles = stylex.create({ opacity: 0.5, cursor: 'default', }, + // A cell: caller content as a tile instead of a row. A 2px border is always + // reserved, so checking a cell recolors it rather than moving its content, + // and it is a border, not a shadow, because forced colors keep borders and + // drop shadows. Positioned so its visually hidden name stays inside it. + cell: { + position: 'relative', + display: 'flex', + alignItems: 'center', + justifyContent: 'center', + boxSizing: 'border-box', + minWidth: 0, + padding: spacingVars['--spacing-0-5'], + borderWidth: 2, + borderStyle: 'solid', + borderColor: { + default: 'transparent', + // A system color, which forced colors leave as written, so an unchecked + // cell's border stays invisible against the menu. + '@media (forced-colors: active)': 'Canvas', + }, + borderRadius: `max(0px, calc(var(--_dropdown-menu-radius, ${spacingVars['--spacing-2']}) - var(--_dropdown-menu-padding, ${spacingVars['--spacing-1']})))`, + color: colorVars['--color-text-primary'], + backgroundColor: { + default: 'transparent', + ':focus': colorVars['--color-overlay-hover'], + }, + cursor: { + default: 'pointer', + ':is([aria-disabled="true"])': 'default', + }, + // The shared keyboard ring sits outside the border that marks the checked + // cell, so a focused checked cell shows both; it is also what marks focus + // under forced colors, which drop the focus overlay. + outlineOffset: 2, + }, + // No focus ring unless the keyboard moved focus here (RadioCell), not even + // the browser's own, which would follow :focus-visible on its own. + cellRingOff: { + outlineStyle: 'none', + }, + cellChecked: { + borderColor: { + default: colorVars['--color-accent'], + '@media (forced-colors: active)': 'Highlight', + }, + }, // Rendered in Item's `marker` slot as a raw flex child. On touch it moves to // the inline-end of the row via `order`. // Layout box for the decorative marker. The painted circle is the radio @@ -114,6 +197,16 @@ export interface DropdownMenuRadioItemProps extends Omit< * metadata. */ endContent?: ReactNode; + /** + * Content drawn as a cell instead of the row: a tile, a colour swatch, an + * icon, a numbered slot. The cell shows no radio mark; `label` names it + * through a visually hidden element, and the checked cell carries an accent + * border. Cells keep the row's role, focus and selection, so the menu's + * arrow keys and typeahead reach them in DOM order; lay them out by styling + * the group or a wrapper inside it (a grid). `icon`, `description` and + * `endContent` are not shown in a cell. + */ + children?: ReactNode; /** Ref forwarded to the row root — the element carrying `role="menuitemradio"`. */ ref?: React.Ref; } @@ -131,6 +224,15 @@ export interface DropdownMenuRadioItemProps extends Omit< * * * + * + * + * + * + * * ``` */ export function DropdownMenuRadioItem({ @@ -140,6 +242,8 @@ export function DropdownMenuRadioItem({ icon, isDisabled = false, endContent, + children, + ref, xstyle, className, style, @@ -178,9 +282,33 @@ export function DropdownMenuRadioItem({ [isDisabled], ); + // Children that render make the option a cell; `false`, `''` and the like + // leave it a row, also inside an array (`rendersContent`). + if (rendersContent(children)) { + return ( + + {children} + + ); + } + return ( ; + +interface RadioCellProps extends Pick< + DropdownMenuRadioItemProps, + 'xstyle' | 'className' | 'style' +> { + hostProps: RadioItemHostProps; + cellRef: Ref | undefined; + label: ReactNode; + isChecked: boolean; + isDisabled: boolean; + /** Whether row-only content (icon, description, end content) was given. */ + hasRowContent: boolean; + onSelect: (event: MouseEvent) => void; + onHover: (event: PointerEvent) => void; + children: ReactNode; +} + +/** + * The cell form of a radio item: its own component, so the row form keeps no + * state or Effect of its own. + */ +function RadioCell({ + hostProps, + cellRef, + label, + isChecked, + isDisabled, + hasRowContent, + onSelect, + onHover, + xstyle, + className, + style, + children, +}: RadioCellProps) { + const labelId = useId(); + // The ring marks focus after keyboard input, also where the mouse then moves + // it. Focus after pointer input (the menu's hover focus after a mouse open) + // paints none, as the menu surfaces gate theirs + // (architecture:interaction-modality INV1). Tracking is + // activated here too, so a cell in any menu reads the last input. + useInteractionModalityTracking(); + const [isKeyboardFocused, setIsKeyboardFocused] = useState(false); + const handleFocus = useCallback((event: FocusEvent) => { + if (event.target === event.currentTarget) { + setIsKeyboardFocused(getInteractionModality() === 'keyboard'); + } + }, []); + useDevWarning( + 'DropdownMenuRadioItem', + 'A radio item with children is a cell, which shows no icon, description ' + + 'or endContent: put them in its children, or drop them.', + hasRowContent, + ); + return ( +
} + role="menuitemradio" + aria-checked={isChecked} + aria-disabled={isDisabled || undefined} + aria-labelledby={labelId} + tabIndex={isDisabled ? undefined : -1} + onFocus={composeEventHandlers(handleFocus, hostProps.onFocus)} + onPointerMove={onHover} + onClick={onSelect} + {...mergeProps( + stylex.props( + styles.cellRingOff, + isKeyboardFocused && focusOutlineStyles.focusVisible, + styles.cell, + isChecked && styles.cellChecked, + isDisabled && styles.disabled, + xstyle, + ), + {className, style}, + )}> + {label} + {children} +
+ ); +} diff --git a/packages/core/src/DropdownMenu/__tests__/RadioCell.a11y.chromium.spec.ts b/packages/core/src/DropdownMenu/__tests__/RadioCell.a11y.chromium.spec.ts new file mode 100644 index 0000000000000..5f404388b0368 --- /dev/null +++ b/packages/core/src/DropdownMenu/__tests__/RadioCell.a11y.chromium.spec.ts @@ -0,0 +1,189 @@ +// Copyright (c) Meta Platforms, Inc. and affiliates. + +/** + * @file RadioCell.a11y.chromium.spec.ts + * @input The `Core/DropdownMenu` RadioCells story in a built Storybook, real + * keyboard and mouse input, forced-colors emulation, and Chromium's own + * accessibility tree read over the DevTools protocol. + * @output Real-engine evidence that radio cells are `menuitemradio` options + * named by their labels, that the checked cell carries an accent border + * (Highlight under forced colors), and that the arrow keys walk them in DOM + * order and Enter picks one. + * @position Run with `pnpm test:a11y-contract`; jsdom has no computed paint, + * no forced colors and no accessibility tree. + */ + +import {expect, test, type Page} from '@playwright/test'; +import {AxeBuilder} from '@axe-core/playwright'; +import { + DEFAULT_STORYBOOK_DIR, + serveStorybook, + type StaticServer, +} from '@astryxdesign/a11y-spec/storybook'; + +let storybook: StaticServer; + +test.beforeAll(async () => { + storybook = await serveStorybook( + process.env.ASTRYX_STORYBOOK_DIR ?? DEFAULT_STORYBOOK_DIR, + ); +}); + +test.afterAll(async () => { + await storybook?.close(); +}); + +const STORY = 'core-dropdownmenu--radio-cells'; + +async function openMenu(page: Page) { + await page.goto(`${storybook.origin}/iframe.html?id=${STORY}&viewMode=story`); + await page.getByRole('button', {name: 'Customize'}).click(); + await expect(page.getByRole('menu')).toBeVisible(); +} + +/** Chromium's own name and checked state for every menuitemradio. */ +async function radioNodes(page: Page) { + const cdp = await page.context().newCDPSession(page); + await cdp.send('Accessibility.enable'); + const {nodes} = await cdp.send('Accessibility.getFullAXTree'); + await cdp.detach(); + return nodes + .filter(node => node.role?.value === 'menuitemradio' && !node.ignored) + .map(node => ({ + name: String(node.name?.value ?? ''), + checked: + node.properties?.find(p => p.name === 'checked')?.value.value === + 'true', + })); +} + +/** A color resolved the way the page resolves it, for comparing paint. */ +async function resolved(page: Page, color: string): Promise { + return page.evaluate(value => { + const probe = document.createElement('span'); + probe.style.color = value; + document.querySelector('[role="menu"]')?.append(probe); + const result = getComputedStyle(probe).color; + probe.remove(); + return result; + }, color); +} + +async function borderColor(page: Page, name: string): Promise { + return page + .getByRole('menuitemradio', {name}) + .evaluate(cell => getComputedStyle(cell).borderTopColor); +} + +test('cells are named options with the checked one painted, walked by the arrow keys in DOM order', async ({ + page, +}) => { + await openMenu(page); + const radios = await radioNodes(page); + expect(radios.map(radio => radio.name)).toEqual([ + 'Red', + 'Orange', + 'Green', + 'Blue', + 'Purple', + 'Pink', + 'Teal', + 'Gray', + ]); + expect(radios.filter(radio => radio.checked).map(r => r.name)).toEqual([ + 'Blue', + ]); + expect(await borderColor(page, 'Blue')).toBe( + await resolved(page, 'var(--color-accent)'), + ); + expect(await borderColor(page, 'Red')).toBe('rgba(0, 0, 0, 0)'); + + // The menu takes focus on a pointer open; the arrow keys walk the cells in + // DOM order, across the grid's rows, and Enter picks one. + await page.keyboard.press('ArrowDown'); + await expect(page.getByRole('menuitemradio', {name: 'Red'})).toBeFocused(); + await page.keyboard.press('ArrowDown'); + await page.keyboard.press('ArrowDown'); + await page.keyboard.press('ArrowDown'); + await page.keyboard.press('ArrowDown'); + await expect(page.getByRole('menuitemradio', {name: 'Purple'})).toBeFocused(); + await page.keyboard.press('Enter'); + await expect(page.getByRole('menu')).toBeHidden(); + + await page.getByRole('button', {name: 'Customize'}).click(); + await expect( + page.getByRole('menuitemradio', {name: 'Purple'}), + ).toHaveAttribute('aria-checked', 'true'); + expect(await borderColor(page, 'Purple')).toBe( + await resolved(page, 'var(--color-accent)'), + ); +}); + +test('after a pointer open, a cell the mouse rests on paints no focus ring, and the keyboard brings it back', async ({ + page, +}) => { + await openMenu(page); + const green = page.getByRole('menuitemradio', {name: 'Green'}); + const box = await green.boundingBox(); + if (box == null) { + throw new Error('the Green cell has no layout box'); + } + await page.mouse.move(box.x + box.width / 2, box.y + box.height / 2, { + steps: 4, + }); + await expect(green).toBeFocused(); + expect( + await green.evaluate(cell => getComputedStyle(cell).outlineStyle), + ).toBe('none'); + + await page.keyboard.press('ArrowDown'); + const blue = page.getByRole('menuitemradio', {name: 'Blue'}); + await expect(blue).toBeFocused(); + expect(await blue.evaluate(cell => getComputedStyle(cell).outlineStyle)).toBe( + 'solid', + ); +}); + +test('under forced colors the checked cell keeps a visible border, keyboard focus an outline, and a swatch its colour', async ({ + page, +}) => { + await page.emulateMedia({forcedColors: 'active'}); + await openMenu(page); + const highlight = await resolved(page, 'Highlight'); + const canvas = await resolved(page, 'Canvas'); + expect(highlight).not.toBe(canvas); + expect(await borderColor(page, 'Blue')).toBe(highlight); + expect(await borderColor(page, 'Red')).toBe(canvas); + // A swatch drawn as a background colour keeps it (the story opts out of + // the forced background), so an unchecked swatch stays visible. + expect( + await page + .getByRole('menuitemradio', {name: 'Red'}) + .evaluate( + cell => + getComputedStyle( + cell.querySelector('[aria-hidden="true"]') as HTMLElement, + ).backgroundColor, + ), + ).toBe('rgb(229, 72, 77)'); // the story's #e5484d, not Canvas + + await page.keyboard.press('ArrowDown'); + const red = page.getByRole('menuitemradio', {name: 'Red'}); + await expect(red).toBeFocused(); + expect(await red.evaluate(cell => getComputedStyle(cell).outlineStyle)).toBe( + 'solid', + ); +}); + +test('axe finds no violation in a menu of cells, including tiles in a wrapper grid inside their group', async ({ + page, +}) => { + await openMenu(page); + const root = await new AxeBuilder({page}).include('[role="menu"]').analyze(); + expect(root.violations.map(v => v.id)).toEqual([]); + + await page.getByRole('menuitem', {name: 'Tab icon'}).click(); + await expect(page.getByRole('menuitemradio', {name: 'Gear'})).toBeVisible(); + const sub = await new AxeBuilder({page}).include('[role="menu"]').analyze(); + expect(sub.violations.map(v => v.id)).toEqual([]); +});