From 0964147cefd1b1a5ba44f1f0e442ae6625206441 Mon Sep 17 00:00:00 2001 From: Cindy Zhang Date: Sun, 11 Oct 2026 05:33:35 -0700 Subject: [PATCH 1/4] [Component Audit] Harden CommandPaletteItem activation --- .changeset/command-palette-item-disabled.md | 7 + apps/storybook/rtl-audit/targets.json | 5 + .../stories/CommandPaletteItem.stories.tsx | 46 ++++ .../src/CommandPalette/CommandPalette.spec.md | 6 +- .../CommandPalette/CommandPaletteItem.doc.mjs | 95 ++++++-- .../CommandPalette/CommandPaletteItem.spec.md | 207 ++++++++++++++++++ .../CommandPaletteItem.test.tsx | 175 ++++++++++++++- .../src/CommandPalette/CommandPaletteItem.tsx | 22 +- 8 files changed, 527 insertions(+), 36 deletions(-) create mode 100644 .changeset/command-palette-item-disabled.md create mode 100644 apps/storybook/stories/CommandPaletteItem.stories.tsx create mode 100644 packages/core/src/CommandPalette/CommandPaletteItem.spec.md diff --git a/.changeset/command-palette-item-disabled.md b/.changeset/command-palette-item-disabled.md new file mode 100644 index 0000000000000..cd40db2fb4d7b --- /dev/null +++ b/.changeset/command-palette-item-disabled.md @@ -0,0 +1,7 @@ +--- +'@astryxdesign/core': patch +--- + +[fix] Keep standalone command-palette item IDs on the rendered option and suppress consumer click callbacks when an item is disabled. + +@cixzhang diff --git a/apps/storybook/rtl-audit/targets.json b/apps/storybook/rtl-audit/targets.json index 72f551ad390d8..fa9baf26316e2 100644 --- a/apps/storybook/rtl-audit/targets.json +++ b/apps/storybook/rtl-audit/targets.json @@ -520,6 +520,11 @@ "next": ".astryx-avatar-group-overflow" } }, + { + "component": "CommandPaletteItem", + "storyId": "core-commandpaletteitem--states", + "dims": [] + }, { "component": "CommandPaletteFooter", "storyId": "core-commandpalettefooter--default", diff --git a/apps/storybook/stories/CommandPaletteItem.stories.tsx b/apps/storybook/stories/CommandPaletteItem.stories.tsx new file mode 100644 index 0000000000000..862b06c540d59 --- /dev/null +++ b/apps/storybook/stories/CommandPaletteItem.stories.tsx @@ -0,0 +1,46 @@ +// Copyright (c) Meta Platforms, Inc. and affiliates. + +import type {Meta, StoryObj} from '@storybook/react'; +import { + CommandPaletteItem, + CommandPaletteList, +} from '@astryxdesign/core/CommandPalette'; +import {Kbd} from '@astryxdesign/core/Kbd'; + +const meta: Meta = { + title: 'Core/CommandPaletteItem', + component: CommandPaletteItem, + tags: ['autodocs', 'visual-theme-matrix'], + parameters: { + layout: 'centered', + }, +}; + +export default meta; +type Story = StoryObj; + +/** Default, highlighted, selected, and disabled states in the supported listbox composition. */ +export const States: Story = { + render: () => ( + + {}}> + Open file + ⌘O + + {}}> + Search files + + {}}> + Settings + + {}}> + Archived command + + + ), +}; diff --git a/packages/core/src/CommandPalette/CommandPalette.spec.md b/packages/core/src/CommandPalette/CommandPalette.spec.md index 810cc9b1ac7b5..a4f30739400ab 100644 --- a/packages/core/src/CommandPalette/CommandPalette.spec.md +++ b/packages/core/src/CommandPalette/CommandPalette.spec.md @@ -57,8 +57,8 @@ Consumer migration instructions belong in consumer docs and release notes. **Owns** -- Search orchestration and the current Input, List, Item, Group, Group heading, - Empty, and Footer parts and targets. +- Search orchestration and composition of the current Input, List, Item, Group, + Group heading, Empty, and Footer parts and targets. - The native Query field rendered inside the default Input. **Does not own / non-goals** @@ -67,6 +67,8 @@ Consumer migration instructions belong in consumer docs and release notes. badges — owned by their respective components. - Caller-provided replacement Input, Footer, item content, or trailing input content — owned by the product callsite. +- The Item option's passthrough, pointer, disabled, selected, highlighted, and + standalone-scroll semantics — owned by `component:CommandPaletteItem`. - A `command-palette` root target — no such current target exists, and this factual backfill does not invent one. - Shared layer hosting, positioning, or lifecycle rules beyond the current diff --git a/packages/core/src/CommandPalette/CommandPaletteItem.doc.mjs b/packages/core/src/CommandPalette/CommandPaletteItem.doc.mjs index 35152c02b93a8..1819cfb868bd5 100644 --- a/packages/core/src/CommandPalette/CommandPaletteItem.doc.mjs +++ b/packages/core/src/CommandPalette/CommandPaletteItem.doc.mjs @@ -7,47 +7,93 @@ export const docs = { subComponentOf: 'CommandPalette', displayName: 'Command Palette Item', isHiddenFromOverview: true, - description: 'A selectable item. Accepts arbitrary children for full rendering control. Registers with context for keyboard navigation when inside CommandPalette.', + description: + 'One option in a command palette result list. CommandPalette context supplies selection, highlight, identity, and close behavior.', + usage: { + description: + 'Use CommandPaletteItem for one non-interactive result row inside CommandPaletteList or CommandPaletteGroup. CommandPalette owns the composite keyboard model; standalone items expose only their explicit visual state and pointer activation.', + bestPractices: [ + { + guidance: true, + description: + 'Keep each value unique within the result list so highlight, active-descendant identity, and selection resolve to the same item.', + }, + { + guidance: true, + description: + 'Use non-interactive child content such as icons, descriptions, and shortcut hints; activate the option itself instead of nesting buttons or links.', + }, + { + guidance: false, + description: + 'Treat isHighlighted as selection. Highlight is the active keyboard result; isSelected represents the chosen value.', + }, + ], + anatomy: [ + { + name: 'Item', + required: true, + description: + 'The option root that carries selection, disabled, item identity, interaction, and theme state.', + }, + { + name: 'Content', + required: true, + description: + 'Caller-provided non-interactive label and supporting presentation.', + }, + ], + }, + theming: { + targets: [{className: 'astryx-command-palette-item'}], + }, props: [ { name: 'value', type: 'string', - description: 'Unique value for identification and selection.', + description: + 'Unique value used for item identity, highlight lookup, and selection.', required: true, }, { name: 'children', type: 'ReactNode', - description: 'Item content: render icons, descriptions, keyboard shortcuts, etc.', + description: + 'Non-interactive item content such as a label, icon, description, or keyboard shortcut.', required: true, }, { name: 'onSelect', type: '(value: string) => void', - description: 'Called when this item is selected via click or Enter.', + description: + 'Called after this item is activated by pointer. Keyboard selection in CommandPalette reports through CommandPalette onValueChange.', }, { name: 'isHighlighted', type: 'boolean', - description: 'Whether this item has keyboard focus. Derived from context when inside CommandPalette.', + description: + 'Whether this item is the active keyboard result. Derived from context when omitted inside CommandPalette.', default: 'false', }, { name: 'isSelected', type: 'boolean', - description: 'Whether this item is selected in picker mode.', + description: + 'Whether this item represents the selected value in picker mode.', default: 'false', }, { name: 'isDisabled', type: 'boolean', - description: 'Whether the item is non-interactive.', + description: + 'Whether the item is unavailable and suppresses pointer activation.', default: 'false', }, { name: 'xstyle', type: 'StyleXStyles', - description: 'StyleX styles for layout customization. Must be a stylex.create() value.', + description: + 'StyleX styles for layout customization. Must be a stylex.create() value.', }, ], playground: { @@ -62,14 +108,17 @@ export const docsZh = { name: 'CommandPaletteItem', isHiddenFromOverview: true, displayName: 'Command Palette Item', - description: '可选择的条目。接受任意子元素以实现完整的渲染控制。', + description: + '命令面板结果列表中的一个选项。CommandPalette 上下文提供选择、高亮、标识和关闭行为。', propDescriptions: { - value: '用于标识和选择的唯一值。', - children: '条目内容——渲染图标、描述、键盘快捷键等。', - onSelect: '通过点击或 Enter 选择此条目时调用。', - isHighlighted: '此条目是否具有键盘焦点。在 CommandPalette 内从上下文派生。', - isSelected: '此条目在选择器模式下是否被选中。', - isDisabled: '条目是否为非交互状态。', + value: '用于条目标识、高亮查找和选择的唯一值。', + children: '非交互式条目内容，例如标签、图标、描述或键盘快捷键。', + onSelect: + '通过指针激活此条目后调用。CommandPalette 中的键盘选择通过根组件的 onValueChange 报告。', + isHighlighted: + '此条目是否为当前键盘结果。在 CommandPalette 内省略时从上下文派生。', + isSelected: '此条目是否代表选择器模式中的已选值。', + isDisabled: '条目是否不可用并禁止指针激活。', xstyle: 'StyleX 布局自定义样式。必须是 stylex.create() 的值。', }, }; @@ -78,14 +127,16 @@ export const docsDense = { name: 'CommandPaletteItem', isHiddenFromOverview: true, displayName: 'Command Palette Item', - description: 'selectable item; arbitrary children; registers w/ context for kbd nav', + description: + 'one result option; context owns selection, highlight, identity, and close', propDescriptions: { - value: 'unique id for selection', - children: 'item content: icons, descriptions, shortcuts', - onSelect: 'called on click or Enter', - isHighlighted: 'keyboard focus; derived from context inside palette', - isSelected: 'selected in picker mode', - isDisabled: 'non-interactive', + value: 'unique value for identity, highlight lookup, and selection', + children: 'non-interactive label and supporting content', + onSelect: + 'called after pointer activation; palette keyboard selection uses root onValueChange', + isHighlighted: 'active keyboard result; derived from context when omitted', + isSelected: 'selected value in picker mode', + isDisabled: 'unavailable; suppresses pointer activation', xstyle: 'StyleX layout styles; must be stylex.create() value', }, }; diff --git a/packages/core/src/CommandPalette/CommandPaletteItem.spec.md b/packages/core/src/CommandPalette/CommandPaletteItem.spec.md new file mode 100644 index 0000000000000..c28d001e9f107 --- /dev/null +++ b/packages/core/src/CommandPalette/CommandPaletteItem.spec.md @@ -0,0 +1,207 @@ +--- +schema_version: 3 +template_version: 7 +kind: component +id: component:CommandPaletteItem +authority: draft +archive_reason: null +superseded_by: null +approved_by: null +approved_at: null +owners: [cixzhang] +review_triggers: [behavior, accessibility, public-api, compatibility, theming] +verified_by: + [ + packages/core/src/CommandPalette/CommandPaletteItem.test.tsx, + packages/core/src/CommandPalette/CommandPalette.test.tsx, + packages/core/src/theme/themingTargets.test.ts, + apps/storybook/stories/CommandPaletteItem.stories.tsx, + apps/storybook/rtl-audit/targets.json, + scripts/check-knowledge.mjs, + ] +modules: [] +families: [] +design_specs: [] +architecture: + [ + architecture:component-test-sufficiency, + architecture:component-theming-surface, + architecture:interaction-modality, + architecture:public-component-api, + ] +contributing: [] +system_specs: [spec:AST-017, spec:AST-020] +--- + +# CommandPaletteItem component contract + +## Contract at a glance + +| Area | Contract | +| ----------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| Public contract | One result option with context-backed identity, highlight, selection, and close behavior; explicit standalone visual state and pointer activation remain available. | +| Behavior | Consumer click runs first; cancellation stops selection; successful activation reports locally, updates context, and closes; disabled items run no click callbacks. | +| End-user impact | Disabled commands cannot activate, and keyboard focus, selected value, and option identity remain distinct and consistently exposed. | +| Builder impact | Values stay unique; children stay non-interactive; CommandPalette owns composite keyboard navigation and reports keyboard selection through root `onValueChange`. | +| Compatibility/readiness | Victim-free `[fix]` restores promised disabled and native-passthrough behavior; the standalone keyboard contract remains unsettled. | +| Review checks | Reject disabled activation, dropped standalone DOM identity, highlight-as-selection ARIA, nested interactive children, or a second context scroll owner. | +| Governing rules | `architecture:public-component-api/INV5`; `architecture:interaction-modality`; `spec:AST-017/FR1,FR3,FR5`; `spec:AST-020/FR1,FR2`. | + +This table is a review projection; the body below is authoritative. + +## Intent + +CommandPaletteItem presents one command result as an option. CommandPalette +context supplies composite state and keyboard ownership; the item owns its option +semantics, visual states, pointer activation, native passthrough boundary, and +standalone scroll behavior. + +## Compatibility and migration + +- Released default preserved: `yes` +- Compatibility class: victim-free contract restoration under + `spec:AST-017/FR1,FR3,FR5`; `isDisabled` already promises a non-interactive + item and the native `BaseProps` seam already promises standalone DOM props +- Controlled/uncontrolled behavior: explicit item state still precedes context + state, then the existing false default +- Migration decision: none for these corrections; OQ1 holds any change to the + standalone keyboard contract + +Consumer migration instructions belong in consumer docs and release notes. + +## Ownership boundary + +**Owns** + +- The option root, item value marker, selected and disabled ARIA state, and + `command-palette-item` theme target. +- Explicit → CommandPalette context → false precedence for highlighted and + selected visual state. +- Pointer activation order, disabled suppression, and context selection/close. +- Standalone highlighted-item scroll behavior and the inline-preview initial + scroll suppression. + +**Does not own / non-goals** + +- CommandPalette's listbox, query-field active descendant, arrow-key navigation, + Enter handling, search results, or root `onValueChange` callback. +- Grouping, list layout, or caller-provided non-interactive item content. +- A standalone composite listbox keyboard model; its direction remains OQ1. +- Interactive descendants. Buttons and links are sibling actions or separate + commands, not content inside an option. + +## Public concepts + +| Concept | Closed values or states | Meaning | Availability | Default | Owner | Stability | Invalid-value behavior | +| ------------- | --------------------------------------------------- | ------------------------------------------------- | ------------------------ | ------- | ------------------------------ | --------- | --------------------------------------------------------- | +| Highlight | explicit; context index; false | Active keyboard result, painted but not selected. | All uses | false | `component:CommandPaletteItem` | Stable | Unknown context value leaves every item unhighlighted. | +| Selection | explicit; context value; false | Chosen value exposed through `aria-selected`. | All uses | false | `component:CommandPaletteItem` | Stable | Duplicate values resolve ambiguously; values stay unique. | +| Availability | enabled; disabled | Whether pointer activation is available. | All uses | enabled | `component:CommandPaletteItem` | Stable | Disabled suppresses consumer and component callbacks. | +| Item identity | context-generated id; standalone caller-provided id | DOM identity used by active-descendant or caller. | Contextual or standalone | absent | component or caller | Stable | Context identity wins only for a registered item. | + +## Behavioral and layout contract + +| ID | Candidate invariant | Basis | Draft review state | +| --- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | ------------------------------------------------------------ | ------------------------------------------ | +| FR1 | Native props and `ref` reach the option root. CommandPalette context owns registered-item id, role, selected state, disabled state, and value marker; standalone caller id persists. | Released source and `architecture:public-component-api/INV5` | Verified by focused DOM tests | +| FR2 | Pointer activation runs consumer `onClick`, then stops when prevented; otherwise it calls item `onSelect`, then context selection and close. Disabled items run none of those paths. | Released callbacks and disabled promise | Corrected and mutation-resistant | +| FR3 | Explicit `isHighlighted` and `isSelected` precede context-derived values, then false. Highlight paints active state only; selection alone drives `aria-selected`. | Released source, docs, and listbox semantics | Verified by focused DOM tests | +| FR4 | Context owns highlighted-item scrolling. Outside context, a highlighted item scrolls nearest, except an initially highlighted item inside an inline Dialog does not scroll. | Released source and regression history | Verified by focused Effect tests | +| FR5 | Children provide the option's label and supporting presentation, without nested interactive descendants. | Option semantics and `spec:AST-020/FR1,FR2` | Documented; exact-head a11y remains a gate | + +### Allowed variation + +- **AV1 — Content.** Labels, icons, descriptions, and shortcut hints may vary + while remaining non-interactive. +- **AV2 — Visual state.** Highlight, selection, disabled state, and their valid + combinations may vary independently. +- **AV3 — Layout.** Consumer `xstyle`, class, and style may adjust the single + root without replacing its option semantics. + +### Representative states + +| State | Required invariant | Allowed variation | +| -------------------- | ------------------------------------------------------------------------- | --------------------------------- | +| Enabled standalone | Caller id and native props persist; pointer activation reports locally. | Content and visual state. | +| Disabled standalone | `aria-disabled` is true and no click callback fires. | Content. | +| Context highlighted | Context item id matches active descendant and highlight is not selection. | Item index and content. | +| Context selected | Matching value exposes `aria-selected=true`. | Highlight may be same or another. | +| Consumer-cancelled | Consumer click runs; local and context selection/close do not. | Consumer side effect. | +| Inline initial state | Initial standalone highlight does not scroll the surrounding preview. | Selected value and content. | + +### Transformation and precedence order + +- **ORD1 — Visual state.** Explicit item prop → CommandPalette context → false. +- **ORD2 — Pointer activation.** Disabled guard → consumer click → prevented + check → item `onSelect` → context select → context close. +- **ORD3 — Identity.** Registered context id → standalone caller id → absent. + +### Performance and resources + +- **PR1 — Lookup.** Context matching remains bounded by the current flat result + list and performs no listener, observer, or per-item registry work. +- **PR2 — Scroll ownership.** One owner scrolls a highlighted contextual item; + standalone items schedule no work when not highlighted. + +## Accessibility contract + +- **AR1 — Option semantics.** The root exposes `role=option`; its content or + caller label provides the accessible name, and `aria-selected` reports chosen + value rather than visual highlight. +- **AR2 — Composite ownership.** Inside CommandPalette, the query field retains + focus and points to the context-generated item id; CommandPalette owns arrow + navigation and Enter selection under its adopted listbox/combobox model. +- **AR3 — Disabled state.** Disabled items expose `aria-disabled=true`, do not + paint interaction overlays, and suppress every pointer activation callback. +- **AR4 — Content.** Item children contain no nested button, link, or other + interactive descendant whose semantics would be lost inside an option. +- **AR5 — Standalone gap.** A standalone option has no owning composite keyboard + model. OQ1 must settle that public contract before runtime semantics change. + +## Design relationships + +| Anatomy | Design requirement | Representation authority | Hierarchy role | Component contract | +| ------- | ------------------------------------------------------------------------------ | ------------------------------ | -------------- | ------------------ | +| Item | Paints enabled, highlighted, selected, disabled, and press states on one root. | Current source and public docs | Prominent | FR1–FR4, AR1–AR3 | +| Content | Supplies the option name and non-interactive supporting presentation. | Product callsite | Supporting | FR5, AR1, AR4 | + +## Family and system relationships + +- `component:CommandPalette` owns composite search, listbox, active-descendant, + keyboard, and close orchestration; this component binds one option to it. +- `architecture:interaction-modality` owns shared pointer and keyboard behavior. +- `architecture:public-component-api` owns native passthrough and released API + compatibility. +- `architecture:component-theming-surface` owns the aggregate Item anatomy map + and stable `command-palette-item` target. +- `spec:AST-017` classifies the victim-free corrections and holds OQ1 against + silent released-contract changes. +- `spec:AST-020` supplies accessibility evidence and adopted-pattern boundaries. + +## Verification map + +| Contract | Verification | Representative states | Mutation or failure expectation | Audit section | +| ----------------- | -------------------------------------------------------------------------------------- | ------------------------------------------------- | ----------------------------------------------------------------------------------------------------------- | ----------------------------------- | +| FR1, FR3, AR1–AR2 | `CommandPaletteItem.test.tsx`, `CommandPalette.test.tsx` | standalone; context highlighted/selected | Dropped id/ref/native prop, clobberable role/state, or highlight-as-selection fails focused DOM assertions. | `audit:CommandPaletteItem/a11y` | +| FR2, AR3 | `CommandPaletteItem.test.tsx` | enabled; disabled; consumer-cancelled; contextual | Disabled consumer activation, ignored cancellation, missing local report, selection, or close fails tests. | `audit:CommandPaletteItem/behavior` | +| FR4 | `CommandPaletteItem.test.tsx` | standalone; inline initial; changed highlight | Lost nearest scrolling, duplicate contextual scrolling, or initial inline scroll fails Effect assertions. | `audit:CommandPaletteItem/behavior` | +| FR5, AR4 | public docs, dedicated Storybook states, exact-head a11y/browser evidence | label; shortcut; disabled | Interactive descendant guidance regresses or supported composition fails exact-head accessibility evidence. | `audit:CommandPaletteItem/a11y` | +| Theme/RTL | focused class assertion, `themingTargets.test.ts`, Storybook fixture, RTL verified N/A | all visual states | Target loss fails inventory/DOM tests; any new directional behavior requires a non-empty RTL dimension. | `audit:CommandPaletteItem/theming` | + +## Decision log + +No component-local API, keyboard, or visual decision is settled in this draft. +Objective corrections preserve the current public shape. + +## Open questions + +- **OQ1 — What standalone keyboard contract should CommandPaletteItem expose?** + (`human-api`) Current standalone use exposes an option and pointer activation + but no owning listbox keyboard model. Decide whether a public composite owner + must supply that model, the item should gain a different standalone role and + interaction contract, or standalone activation should enter a migration path. + +## Content boundary + +This file does not duplicate consumer examples, audit scores, implementation +steps, aggregate CommandPalette search/list behavior, or shared system rules. diff --git a/packages/core/src/CommandPalette/CommandPaletteItem.test.tsx b/packages/core/src/CommandPalette/CommandPaletteItem.test.tsx index 74e897aadba6e..c02dd04aab1b5 100644 --- a/packages/core/src/CommandPalette/CommandPaletteItem.test.tsx +++ b/packages/core/src/CommandPalette/CommandPaletteItem.test.tsx @@ -6,9 +6,12 @@ * @output Unit tests for CommandPaletteItem */ +import {createRef, type ReactElement} from 'react'; import {afterEach, describe, it, expect, vi} from 'vitest'; import {render, screen, fireEvent, waitFor} from '@testing-library/react'; import {Dialog} from '../Dialog/Dialog'; +import {CommandPaletteContext} from './CommandPaletteContext'; +import type {CommandPaletteContextValue} from './CommandPaletteContext'; import {CommandPaletteItem} from './CommandPaletteItem'; const scrollIntoViewDescriptor = Object.getOwnPropertyDescriptor( @@ -38,11 +41,45 @@ afterEach(() => { } }); +function makeContext( + overrides: Partial = {}, +): CommandPaletteContextValue { + return { + search: '', + setSearch: vi.fn(), + value: '', + setValue: vi.fn(), + listId: 'command-list', + highlightedIndex: -1, + setHighlightedIndex: vi.fn(), + getItemId: index => `command-item-${index}`, + selectableItems: [{value: 'test', label: 'Item'}], + searchResults: [], + selectItem: vi.fn(), + onKeyDown: vi.fn(), + onClose: vi.fn(), + isOpen: true, + isBusy: false, + ...overrides, + }; +} + +function renderWithContext( + item: ReactElement, + overrides: Partial = {}, +) { + const context = makeContext(overrides); + return { + context, + ...render( + {item}, + ), + }; +} + describe('CommandPaletteItem', () => { it('renders children', () => { - render( - Test Item, - ); + render(Test Item); expect(screen.getByText('Test Item')).toBeInTheDocument(); }); @@ -62,6 +99,36 @@ describe('CommandPaletteItem', () => { expect(handleSelect).toHaveBeenCalledWith('test'); }); + it('coordinates selection and close through CommandPalette context', () => { + const handleSelect = vi.fn(); + const {context} = renderWithContext( + + Item + , + ); + + fireEvent.click(screen.getByRole('option')); + + expect(handleSelect).toHaveBeenCalledWith('test'); + expect(context.selectItem).toHaveBeenCalledWith('test'); + expect(context.onClose).toHaveBeenCalledTimes(1); + }); + + it('lets consumer click cancellation stop contextual selection and close', () => { + const {context} = renderWithContext( + event.preventDefault()}> + Item + , + ); + + fireEvent.click(screen.getByRole('option')); + + expect(context.selectItem).not.toHaveBeenCalled(); + expect(context.onClose).not.toHaveBeenCalled(); + }); + it('does not call onSelect when disabled', () => { const handleSelect = vi.fn(); render( @@ -73,6 +140,71 @@ describe('CommandPaletteItem', () => { expect(handleSelect).not.toHaveBeenCalled(); }); + it('does not call consumer click or selection handlers when disabled', () => { + const handleClick = vi.fn(); + const handleSelect = vi.fn(); + render( + + Item + , + ); + + fireEvent.click(screen.getByRole('option')); + + expect(handleClick).not.toHaveBeenCalled(); + expect(handleSelect).not.toHaveBeenCalled(); + }); + + it('lets a consumer cancel selection from onClick', () => { + const handleSelect = vi.fn(); + render( + event.preventDefault()} + onSelect={handleSelect}> + Item + , + ); + + fireEvent.click(screen.getByRole('option')); + + expect(handleSelect).not.toHaveBeenCalled(); + }); + + it('derives selected and highlighted state plus item identity from context', () => { + renderWithContext( + + Item + , + {value: 'test', highlightedIndex: 0}, + ); + + const item = screen.getByRole('option'); + expect(item).toHaveAttribute('id', 'command-item-0'); + expect(item).toHaveAttribute('aria-selected', 'true'); + expect(item.className).toContain('itemHighlighted'); + }); + + it('protects owned option semantics from native passthrough props', () => { + render( + + Item + , + ); + + const item = screen.getByRole('option'); + expect(item).toHaveAttribute('aria-selected', 'false'); + expect(item).not.toHaveAttribute('aria-disabled'); + }); + it('sets aria-disabled when disabled', () => { render( @@ -157,10 +289,43 @@ describe('CommandPaletteItem', () => { }); }); - it('sets data-value attribute', () => { + it('forwards ref and standalone DOM attributes to the option', () => { + const ref = createRef(); + render( + + Item + , + ); + + const item = screen.getByRole('option', {name: 'Open settings'}); + expect(ref.current).toBe(item); + expect(item).toHaveAttribute('id', 'standalone-item'); + expect(item).toHaveAttribute('data-kind', 'command'); + }); + + it('carries the stable theme target and consumer styling on its root', () => { render( - Item, + + Item + , ); + + const item = screen.getByRole('option'); + expect(item).toHaveClass('astryx-command-palette-item'); + expect(item).toHaveClass('consumer-item'); + expect(item).toHaveStyle({marginBlockStart: '8px'}); + }); + + it('sets data-value attribute', () => { + render(Item); expect(screen.getByRole('option')).toHaveAttribute( 'data-value', 'my-value', diff --git a/packages/core/src/CommandPalette/CommandPaletteItem.tsx b/packages/core/src/CommandPalette/CommandPaletteItem.tsx index 0a1fa645b488f..28589efb74bbf 100644 --- a/packages/core/src/CommandPalette/CommandPaletteItem.tsx +++ b/packages/core/src/CommandPalette/CommandPaletteItem.tsx @@ -74,7 +74,11 @@ export interface CommandPaletteItemProps extends Omit< ref?: React.Ref; /** Unique value for identification and selection. */ value: string; - /** Called when this item is selected (via click or Enter). */ + /** + * Called after this item is activated by pointer. + * Keyboard selection inside CommandPalette reports through the root + * `onValueChange` callback. + */ onSelect?: (value: string) => void; /** * Whether this item is visually highlighted (keyboard focus). @@ -92,7 +96,7 @@ export interface CommandPaletteItemProps extends Omit< * @default false */ isDisabled?: boolean; - /** Item content. Fully custom — render icons, descriptions, shortcuts, etc. */ + /** Non-interactive item content: labels, icons, descriptions, shortcuts, etc. */ children: ReactNode; } @@ -100,9 +104,10 @@ export interface CommandPaletteItemProps extends Omit< * A selectable item in the command palette. * Accepts arbitrary children for full rendering control. * - * When used inside CommandPalette, registers with context for - * keyboard navigation and selection. Can also be used - * standalone with explicit isHighlighted/isSelected props. + * When used inside CommandPalette, context supplies identity, highlight, + * selection, and close behavior. Standalone use supports explicit visual + * state and pointer activation but does not provide a composite listbox + * keyboard model. * * @compositionHint Place inside CommandPaletteList or CommandPaletteGroup. * @@ -124,6 +129,7 @@ export function CommandPaletteItem({ xstyle, className, style, + id: idProp, onClick: onClickProp, onMouseEnter: onMouseEnterProp, ...props @@ -188,13 +194,15 @@ export function CommandPaletteItem({
= 0 ? ctx.getItemId(itemIndex) : undefined} + id={ctx && itemIndex >= 0 ? ctx.getItemId(itemIndex) : idProp} role="option" aria-selected={isSelected} aria-disabled={isDisabled || undefined} data-value={value} {...pressable} - onClick={composeEventHandlers(onClickProp, handleClick)} + onClick={ + isDisabled ? undefined : composeEventHandlers(onClickProp, handleClick) + } onMouseEnter={onMouseEnterProp} {...mergeProps( themeProps('command-palette-item'), From 710f81155b132be0edde5d3813ebe2c13eeb37e3 Mon Sep 17 00:00:00 2001 From: Cindy Zhang Date: Sun, 11 Oct 2026 05:34:47 -0700 Subject: [PATCH 2/4] Document valid command item content --- .../src/CommandPalette/CommandPalette.doc.mjs | 117 ++++++++++++++---- .../src/CommandPalette/CommandPalette.tsx | 4 +- .../CommandPalette/CommandPaletteItem.spec.md | 9 +- 3 files changed, 104 insertions(+), 26 deletions(-) diff --git a/packages/core/src/CommandPalette/CommandPalette.doc.mjs b/packages/core/src/CommandPalette/CommandPalette.doc.mjs index 36328f3bef95b..927d771fafd63 100644 --- a/packages/core/src/CommandPalette/CommandPalette.doc.mjs +++ b/packages/core/src/CommandPalette/CommandPalette.doc.mjs @@ -88,7 +88,8 @@ export const docs = { 'dialog', 'navigation', ], - description: 'Root component. Manages open state, search, keyboard navigation, and composition slots.', + description: + 'Root component. Manages open state, search, keyboard navigation, and composition slots.', // Intentionally a contained isInline preview, not playground.overlay: the // component stays visible on load and knobs stay live, whereas a real // showModal() overlay makes the page inert — see ComponentPlaygroundConfig.overlay @@ -116,13 +117,15 @@ export const docs = { { name: 'searchSource', type: 'SearchSource', - description: 'Search source providing items via search(query) and bootstrap(). Use createStaticSource for static lists.', + description: + 'Search source providing items via search(query) and bootstrap(). Use createStaticSource for static lists.', required: true, }, { name: 'input', type: 'ReactNode', - description: 'Input slot. Defaults to CommandPaletteInput with standard behavior.', + description: + 'Input slot. Defaults to CommandPaletteInput with standard behavior.', default: '', slotElements: [ { @@ -137,7 +140,8 @@ export const docs = { { name: 'footer', type: 'ReactNode', - description: 'Footer slot. Defaults to CommandPaletteFooter showing keyboard hints.', + description: + 'Footer slot. Defaults to CommandPaletteFooter showing keyboard hints.', default: '', slotElements: [ { @@ -152,7 +156,8 @@ export const docs = { { name: 'renderItem', type: '(item: T, isSelected: boolean) => ReactNode', - description: 'Per-item render function. Auto-grouping by auxiliaryData.group is preserved. When omitted, renders label text.', + description: + 'Per-item render function for non-interactive presentation content such as labels, icons, descriptions, and shortcut hints. Do not return buttons or links inside the option. Auto-grouping by auxiliaryData.group is preserved. When omitted, renders label text.', }, { name: 'emptySearchText', @@ -163,7 +168,8 @@ export const docs = { { name: 'emptyBootstrapText', type: 'ReactNode', - description: 'Content shown when there is no search query and bootstrap() returns nothing.', + description: + 'Content shown when there is no search query and bootstrap() returns nothing.', default: "'Type to search'", }, { @@ -197,7 +203,8 @@ export const docs = { { name: 'isInline', type: 'boolean', - description: 'Renders command palette content inline without modal behavior. Automatically disables input auto-focus and initial highlighted-item auto-scroll. For documentation previews and showcases only.', + description: + 'Renders command palette content inline without modal behavior. Automatically disables input auto-focus and initial highlighted-item auto-scroll. For documentation previews and showcases only.', default: 'false', }, ], @@ -222,12 +229,34 @@ export const docs = { }, usage: { anatomy, - description: 'CommandPalette is a searchable dialog for quick access to commands, navigation, and actions. Use it as a keyboard-driven launcher powered by SearchSource for filtering and selection.', + description: + 'CommandPalette is a searchable dialog for quick access to commands, navigation, and actions. Use it as a keyboard-driven launcher powered by SearchSource for filtering and selection.', bestPractices: [ - { guidance: true, description: 'Provide a searchSource with bootstrap results so users see useful options before typing.' }, - { guidance: true, description: 'Use auxiliaryData.group on items to automatically organize results into labeled sections.' }, - { guidance: false, description: 'Use CommandPalette for simple dropdowns or menus; use Menu or Selector for inline selections.' }, - { guidance: false, description: 'Add too many groups or items; curate results to keep the palette fast and scannable.' }, + { + guidance: true, + description: + 'Provide a searchSource with bootstrap results so users see useful options before typing.', + }, + { + guidance: true, + description: + 'Use auxiliaryData.group on items to automatically organize results into labeled sections.', + }, + { + guidance: true, + description: + 'Keep renderItem content non-interactive; each rendered row is one option, so buttons and links cannot be nested inside it.', + }, + { + guidance: false, + description: + 'Use CommandPalette for simple dropdowns or menus; use Menu or Selector for inline selections.', + }, + { + guidance: false, + description: + 'Add too many groups or items; curate results to keep the palette fast and scannable.', + }, ], }, }; @@ -236,12 +265,34 @@ export const docs = { export const docsZh = { usage: { anatomy, - description: 'CommandPalette is a searchable dialog for quick access to commands, navigation, and actions. Use it as a keyboard-driven launcher powered by SearchSource for filtering and selection.', + description: + 'CommandPalette is a searchable dialog for quick access to commands, navigation, and actions. Use it as a keyboard-driven launcher powered by SearchSource for filtering and selection.', bestPractices: [ - { guidance: true, description: 'Provide a searchSource with bootstrap results so users see useful options before typing.' }, - { guidance: true, description: 'Use auxiliaryData.group on items to automatically organize results into labeled sections.' }, - { guidance: false, description: 'Use CommandPalette for simple dropdowns or menus; use Menu or Selector for inline selections.' }, - { guidance: false, description: 'Add too many groups or items; curate results to keep the palette fast and scannable.' }, + { + guidance: true, + description: + 'Provide a searchSource with bootstrap results so users see useful options before typing.', + }, + { + guidance: true, + description: + 'Use auxiliaryData.group on items to automatically organize results into labeled sections.', + }, + { + guidance: true, + description: + 'Keep renderItem content non-interactive; each rendered row is one option, so buttons and links cannot be nested inside it.', + }, + { + guidance: false, + description: + 'Use CommandPalette for simple dropdowns or menus; use Menu or Selector for inline selections.', + }, + { + guidance: false, + description: + 'Add too many groups or items; curate results to keep the palette fast and scannable.', + }, ], }, }; @@ -252,12 +303,34 @@ export const docsDense = { 'searchSource-driven command palette dialog; filtering, keyboard nav, grouping, selection; same SearchSource interface as Typeahead', usage: { anatomy, - description: 'CommandPalette is a searchable dialog for quick access to commands, navigation, and actions. Use it as a keyboard-driven launcher powered by SearchSource for filtering and selection.', + description: + 'CommandPalette is a searchable dialog for quick access to commands, navigation, and actions. Use it as a keyboard-driven launcher powered by SearchSource for filtering and selection.', bestPractices: [ - { guidance: true, description: 'Provide a searchSource with bootstrap results so users see useful options before typing.' }, - { guidance: true, description: 'Use auxiliaryData.group on items to automatically organize results into labeled sections.' }, - { guidance: false, description: 'Use CommandPalette for simple dropdowns or menus; use Menu or Selector for inline selections.' }, - { guidance: false, description: 'Add too many groups or items; curate results to keep the palette fast and scannable.' }, + { + guidance: true, + description: + 'Provide a searchSource with bootstrap results so users see useful options before typing.', + }, + { + guidance: true, + description: + 'Use auxiliaryData.group on items to automatically organize results into labeled sections.', + }, + { + guidance: true, + description: + 'Keep renderItem content non-interactive; each rendered row is one option, so buttons and links cannot be nested inside it.', + }, + { + guidance: false, + description: + 'Use CommandPalette for simple dropdowns or menus; use Menu or Selector for inline selections.', + }, + { + guidance: false, + description: + 'Add too many groups or items; curate results to keep the palette fast and scannable.', + }, ], }, }; diff --git a/packages/core/src/CommandPalette/CommandPalette.tsx b/packages/core/src/CommandPalette/CommandPalette.tsx index d2468af8a5bf4..9053af0386b92 100644 --- a/packages/core/src/CommandPalette/CommandPalette.tsx +++ b/packages/core/src/CommandPalette/CommandPalette.tsx @@ -78,7 +78,9 @@ export interface CommandPaletteProps< /** * Per-item render function. Receives the item and whether it is currently selected. - * Auto-grouping by `auxiliaryData.group` is preserved. + * Return non-interactive presentation content; buttons and links cannot be + * nested inside the rendered option. Auto-grouping by `auxiliaryData.group` + * is preserved. * When omitted, renders each item's `label` text. */ renderItem?: (item: T, isSelected: boolean) => ReactNode; diff --git a/packages/core/src/CommandPalette/CommandPaletteItem.spec.md b/packages/core/src/CommandPalette/CommandPaletteItem.spec.md index c28d001e9f107..f65d6f1a429ed 100644 --- a/packages/core/src/CommandPalette/CommandPaletteItem.spec.md +++ b/packages/core/src/CommandPalette/CommandPaletteItem.spec.md @@ -30,7 +30,7 @@ architecture: architecture:public-component-api, ] contributing: [] -system_specs: [spec:AST-017, spec:AST-020] +system_specs: [spec:AST-017, spec:AST-020, spec:AST-058] --- # CommandPaletteItem component contract @@ -45,7 +45,7 @@ system_specs: [spec:AST-017, spec:AST-020] | Builder impact | Values stay unique; children stay non-interactive; CommandPalette owns composite keyboard navigation and reports keyboard selection through root `onValueChange`. | | Compatibility/readiness | Victim-free `[fix]` restores promised disabled and native-passthrough behavior; the standalone keyboard contract remains unsettled. | | Review checks | Reject disabled activation, dropped standalone DOM identity, highlight-as-selection ARIA, nested interactive children, or a second context scroll owner. | -| Governing rules | `architecture:public-component-api/INV5`; `architecture:interaction-modality`; `spec:AST-017/FR1,FR3,FR5`; `spec:AST-020/FR1,FR2`. | +| Governing rules | `architecture:public-component-api/INV5,INV7`; `architecture:interaction-modality`; `spec:AST-017/FR1,FR3,FR5`; `spec:AST-020/FR1,FR2`; `spec:AST-058/FR9,AR1`. | This table is a review projection; the body below is authoritative. @@ -154,7 +154,8 @@ Consumer migration instructions belong in consumer docs and release notes. - **AR3 — Disabled state.** Disabled items expose `aria-disabled=true`, do not paint interaction overlays, and suppress every pointer activation callback. - **AR4 — Content.** Item children contain no nested button, link, or other - interactive descendant whose semantics would be lost inside an option. + interactive descendant whose semantics would be lost inside an option, + following `spec:AST-058/AR1,FR9`. - **AR5 — Standalone gap.** A standalone option has no owning composite keyboard model. OQ1 must settle that public contract before runtime semantics change. @@ -177,6 +178,8 @@ Consumer migration instructions belong in consumer docs and release notes. - `spec:AST-017` classifies the victim-free corrections and holds OQ1 against silent released-contract changes. - `spec:AST-020` supplies accessibility evidence and adopted-pattern boundaries. +- `spec:AST-058` forbids interactive controls inside an option until a host + adopts its grid contract. ## Verification map From c50060451e7c28fbb05587a6e15c36cdbaafce65 Mon Sep 17 00:00:00 2001 From: Cindy Zhang Date: Sun, 11 Oct 2026 06:08:43 -0700 Subject: [PATCH 3/4] Close CommandPaletteItem audit evidence gaps Co-authored-by: korkt-kim --- .../rtl-audit/verified-not-applicable.json | 9 +++ .../stories/CommandPaletteItem.stories.tsx | 2 +- .../CommandPalette/CommandPaletteItem.spec.md | 57 ++++++++++++------- .../CommandPaletteItem.test.tsx | 29 +++++++++- .../src/CommandPalette/CommandPaletteItem.tsx | 5 +- 5 files changed, 72 insertions(+), 30 deletions(-) diff --git a/apps/storybook/rtl-audit/verified-not-applicable.json b/apps/storybook/rtl-audit/verified-not-applicable.json index 74bdb3ecd2916..2d7f00da26692 100644 --- a/apps/storybook/rtl-audit/verified-not-applicable.json +++ b/apps/storybook/rtl-audit/verified-not-applicable.json @@ -319,6 +319,15 @@ "packages/core/src/Divider/Divider.tsx": "sha256:f302b5c9eecde40b486c09d49c71cb7dab158e2fcedcb38c0145671c45d9cf8d" } }, + { + "component": "core/CommandPaletteItem", + "reason": "CommandPaletteItem owns one inline flex option row with logical inline padding, logical start text alignment, and direction-neutral highlight, selection, disabled, and press paint. Its supported content is non-interactive text, icons, descriptions, and shortcut hints; it owns no directional glyph, side placement, reordering, horizontal scroll, drag, overlay, or horizontal keyboard behavior. The dedicated state fixture exercises every component-owned visual state without adding an RTL-specific dimension.", + "evidence": { + "apps/storybook/stories/CommandPaletteItem.stories.tsx": "sha256:9d5c2229f89da3ac53347078560b9ad57cfff7b20a3c71ad3fab2c338161dbf1", + "packages/core/src/CommandPalette/CommandPaletteItem.tsx": "sha256:aee0fcaa7b9829e04317f1cab9708c045a1da0d9180e490d64c6a576c39d8f42", + "packages/core/src/CommandPalette/index.ts": "sha256:ba38845bbde6d73172df2c0791ca513c9f2b7baab17440e066e783ff6cfc7558" + } + }, { "component": "core/CommandPaletteGroup", "reason": "CommandPaletteGroup owns a vertically stacked listbox group with logical inline padding and block spacing. Its visible heading and item content follow normal document direction; it renders no directional glyph, physical inline positioning, reordered controls, horizontal scrolling or dragging, side-placed overlay, or horizontal keyboard behavior. The dedicated fixtures exercise the supported composition and public heading theme target without adding a component-owned RTL dimension.", diff --git a/apps/storybook/stories/CommandPaletteItem.stories.tsx b/apps/storybook/stories/CommandPaletteItem.stories.tsx index 862b06c540d59..c3bbc46ce146c 100644 --- a/apps/storybook/stories/CommandPaletteItem.stories.tsx +++ b/apps/storybook/stories/CommandPaletteItem.stories.tsx @@ -27,7 +27,7 @@ export const States: Story = { style={{inlineSize: 'min(22rem, calc(100vw - 2rem))'}}> {}}> Open file - ⌘O + { expect(handleSelect).not.toHaveBeenCalled(); }); - it('derives selected and highlighted state plus item identity from context', () => { + it('derives selected state and item identity from context', () => { renderWithContext( Item , - {value: 'test', highlightedIndex: 0}, + {value: 'test'}, ); const item = screen.getByRole('option'); expect(item).toHaveAttribute('id', 'command-item-0'); expect(item).toHaveAttribute('aria-selected', 'true'); - expect(item.className).toContain('itemHighlighted'); + }); + + it('paints context-derived highlight when the item is not selected', () => { + renderWithContext( + Item, + {highlightedIndex: 0}, + ); + + expect(getComputedStyle(screen.getByRole('option')).backgroundColor).toBe( + 'var(--color-overlay-hover)', + ); + }); + + it('lets explicit visual state override context-derived state', () => { + renderWithContext( + + Item + , + {value: 'test', highlightedIndex: 0}, + ); + + const item = screen.getByRole('option'); + expect(item).toHaveAttribute('aria-selected', 'false'); + expect(getComputedStyle(item).backgroundColor).toBe('rgba(0, 0, 0, 0)'); }); it('protects owned option semantics from native passthrough props', () => { diff --git a/packages/core/src/CommandPalette/CommandPaletteItem.tsx b/packages/core/src/CommandPalette/CommandPaletteItem.tsx index 28589efb74bbf..344f861437f4c 100644 --- a/packages/core/src/CommandPalette/CommandPaletteItem.tsx +++ b/packages/core/src/CommandPalette/CommandPaletteItem.tsx @@ -180,15 +180,12 @@ export function CommandPaletteItem({ }, [ctx, isHighlighted, isInlineDialog]); const handleClick = useCallback(() => { - if (isDisabled) { - return; - } onSelect?.(value); if (ctx) { ctx.selectItem(value); ctx.onClose(); } - }, [isDisabled, value, onSelect, ctx]); + }, [value, onSelect, ctx]); return (
Date: Sun, 11 Oct 2026 06:19:04 -0700 Subject: [PATCH 4/4] Finalize CommandPaletteItem audit record --- .changeset/command-palette-item-disabled.md | 2 +- .../src/CommandPalette/CommandPaletteItem.spec.md | 12 ++++++------ 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/.changeset/command-palette-item-disabled.md b/.changeset/command-palette-item-disabled.md index cd40db2fb4d7b..62b2fc25f5b59 100644 --- a/.changeset/command-palette-item-disabled.md +++ b/.changeset/command-palette-item-disabled.md @@ -4,4 +4,4 @@ [fix] Keep standalone command-palette item IDs on the rendered option and suppress consumer click callbacks when an item is disabled. -@cixzhang +@cixzhang @korkt-kim diff --git a/packages/core/src/CommandPalette/CommandPaletteItem.spec.md b/packages/core/src/CommandPalette/CommandPaletteItem.spec.md index 0596fb1a15977..45e435538b581 100644 --- a/packages/core/src/CommandPalette/CommandPaletteItem.spec.md +++ b/packages/core/src/CommandPalette/CommandPaletteItem.spec.md @@ -66,11 +66,11 @@ standalone scroll behavior. state, then the existing false default - Migration decision: none for these corrections; OQ1 holds any change to the standalone keyboard contract -- Documentation correction: latest stable promised Enter through item - `onSelect`, but no public runtime path invokes it — CommandPalette mints rows - without item callbacks and standalone items have no keyboard owner. Docs now - describe current pointer/item and root-keyboard callbacks; OQ1 owns any new - keyboard path. +- Documentation compatibility: item `onSelect` reports pointer activation only. + No public runtime path delivers Enter to it — CommandPalette mints rows without + item callbacks and standalone items have no keyboard owner — so describing + keyboard selection through root `onValueChange` leaves no released consumer + behind (`spec:AST-017/FR1,FR5`). OQ1 owns any standalone keyboard path. Consumer migration instructions belong in consumer docs and release notes. @@ -109,7 +109,7 @@ Consumer migration instructions belong in consumer docs and release notes. | ID | Candidate invariant | Basis | Draft review state | | --- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | ------------------------------------------------------------ | ------------------------------------------------------------------------- | | FR1 | Native props and `ref` reach the option root. CommandPalette context owns registered-item id, role, selected state, disabled state, and value marker; standalone caller id persists. | Released source and `architecture:public-component-api/INV5` | Verified by focused DOM tests | -| FR2 | Pointer activation runs consumer `onClick`, then stops when prevented; otherwise it calls item `onSelect`, then context selection and close. Disabled items run none of those paths. | Released callbacks and disabled promise | Corrected and mutation-resistant | +| FR2 | Pointer activation runs consumer `onClick`, then stops when prevented; otherwise it calls item `onSelect`, then context selection and close. Disabled items run none of those paths. | Released callbacks and disabled promise | Verified; mutation-resistant | | FR3 | Explicit `isHighlighted` and `isSelected` precede context-derived values, then false. Highlight paints active state only; selection alone drives `aria-selected`. | Released source, docs, and listbox semantics | State semantics tested; precedence tested; combined paint gap remains OQ2 | | FR4 | Context owns highlighted-item scrolling. Outside context, a highlighted item scrolls nearest, except an initially highlighted item inside an inline Dialog does not scroll. | Released source and regression history | Verified by focused Effect tests | | FR5 | Children provide the option's label and supporting presentation, without nested interactive descendants. | Option semantics and `spec:AST-020/FR1,FR2` | Documented; exact-head a11y remains a gate |