diff --git a/.changeset/context-menu-pointer-open-focus.md b/.changeset/context-menu-pointer-open-focus.md new file mode 100644 index 0000000000000..29e0264dec946 --- /dev/null +++ b/.changeset/context-menu-pointer-open-focus.md @@ -0,0 +1,7 @@ +--- +'@astryxdesign/core': patch +--- + +[fix] A ContextMenu opened by a right-click or a long press no longer highlights its first row: focus goes to the menu itself, so the highlight starts where the pointer goes, as a DropdownMenu opened from its trigger already does. A keyboard-invoked open (Shift+F10, the Menu key) still lands on the first row, detected from the shared input modality being keyboard (a key was the last input, or there has been none yet), since browsers report those keys at the focused element rather than at (0, 0). + +@vjeux diff --git a/packages/core/src/ContextMenu/ContextMenu.spec.md b/packages/core/src/ContextMenu/ContextMenu.spec.md index 9ad771e40db7f..3c236584bcdcc 100644 --- a/packages/core/src/ContextMenu/ContextMenu.spec.md +++ b/packages/core/src/ContextMenu/ContextMenu.spec.md @@ -85,17 +85,18 @@ Consumer props, content modes, and presentation policy remain documented in ## Behavioral and layout contract -| ID | Candidate invariant | Basis | Draft review state | -| --- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ---------------------------------------------------------------------------- | --------------------------------------------------- | -| FR1 | Every render contains a caller-provided Trigger area. Pointer presentation opens a Pointer menu surface at the current local cursor anchor; touch presentation opens a Touch sheet frame containing a Touch menu surface and, for data mode, a Touch action list and Touch action rows. | Current source, docs, and tests | Current behavior; verified | -| FR2 | The current `context-menu` target remains on the painted Pointer menu surface and alternative Touch menu surface, not on the Trigger area or cursor anchor. | Current source, docs, and tests | Current inventory; verified | -| FR3 | Pointer action rows retain DropdownMenu ownership. The touch frame retains BottomSheet ownership, while data-driven touch lists and rows retain List ownership. | Current source and owner docs | Current delegation; verified | -| FR4 | Compound `menuContent` remains a caller-supplied pointer-menu interior and may also render inside the touch frame; the data-driven touch path instead converts the same item data to List and ListItem presentation. | Current source and tests | Current branches; verified | -| FR5 | The Pointer menu surface carries `data-astryx-menu-press` and follows `module:DropdownMenu/useMenuPress` FR1–FR7: the row under the release acts, the highlight follows a held pointer, a mouse released outside closes and a finger leaves the menu open, the stray click never acts, and `touch-action` follows overflow. Under a finger the press follows `spec:AST-094` FR26–FR28 and FR30, and the menu's grow, dismissal, long-press lift and outside press follow FR31–FR33 and FR35 (pending implementation); a mouse press outside is unchanged. Right-click, Shift+F10, long-press invocation and keyboard navigation keep their existing behavior. | `module:DropdownMenu/useMenuPress`; `ContextMenu.test.tsx` press model suite | Current module projection; verified in jsdom | -| FR6 | With `triggerAs="span"` the Trigger area MUST be an inline element that owns a box, so the cursor anchor measures from the trigger's own rect on every invocation path. Right-click, Shift+F10, long-press, and `context-menu` target placement all use that box. | `component:ContextMenu/DEC-1` | Accepted; verified in jsdom | -| FR7 | Sub-menu rows follow `component:DropdownMenu` FR12 inside the pointer menu: ContextMenu mounts the same drill-in view stack, named after its `label`, so a sub-menu drills in on a phone here too. | `component:DropdownMenu/DEC-2` | Proposed; verified in jsdom; owner decision pending | -| FR8 | In the pointer menu ArrowDown on the last enabled row wraps to the first and ArrowUp on the first to the last; PageDown and PageUp page as in DropdownMenu FR6; a held key's auto-repeat does not activate; typeahead matches the row's label element alone. | Current source and tests; shared `useListFocus` and `useTypeahead` suites | Proposed; verified in jsdom; owner decision pending | -| FR9 | While the Pointer menu surface is open, it participates in `family:overlay-dismissal` through the shared stack and provides descendant depth. One Escape press affects only the top-most present nested surface; ContextMenu closes only when it is that surface. | `family:overlay-dismissal` FR1–FR6; `ContextMenu.test.tsx` nested-layer case | Current family projection; verified in jsdom | +| ID | Candidate invariant | Basis | Draft review state | +| ---- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | ---------------------------------------------------------------------------- | --------------------------------------------------- | +| FR1 | Every render contains a caller-provided Trigger area. Pointer presentation opens a Pointer menu surface at the current local cursor anchor; touch presentation opens a Touch sheet frame containing a Touch menu surface and, for data mode, a Touch action list and Touch action rows. | Current source, docs, and tests | Current behavior; verified | +| FR2 | The current `context-menu` target remains on the painted Pointer menu surface and alternative Touch menu surface, not on the Trigger area or cursor anchor. | Current source, docs, and tests | Current inventory; verified | +| FR3 | Pointer action rows retain DropdownMenu ownership. The touch frame retains BottomSheet ownership, while data-driven touch lists and rows retain List ownership. | Current source and owner docs | Current delegation; verified | +| FR4 | Compound `menuContent` remains a caller-supplied pointer-menu interior and may also render inside the touch frame; the data-driven touch path instead converts the same item data to List and ListItem presentation. | Current source and tests | Current branches; verified | +| FR5 | The Pointer menu surface carries `data-astryx-menu-press` and follows `module:DropdownMenu/useMenuPress` FR1–FR7: the row under the release acts, the highlight follows a held pointer, a mouse released outside closes and a finger leaves the menu open, the stray click never acts, and `touch-action` follows overflow. Under a finger the press follows `spec:AST-094` FR26–FR28 and FR30, and the menu's grow, dismissal, long-press lift and outside press follow FR31–FR33 and FR35 (pending implementation); a mouse press outside is unchanged. Invocation and keyboard navigation keep their behavior; focus on open follows proposed FR10. | `module:DropdownMenu/useMenuPress`; `ContextMenu.test.tsx` press model suite | Current module projection; verified in jsdom | +| FR6 | With `triggerAs="span"` the Trigger area MUST be an inline element that owns a box, so the cursor anchor measures from the trigger's own rect on every invocation path. Right-click, Shift+F10, long-press, and `context-menu` target placement all use that box. | `component:ContextMenu/DEC-1` | Accepted; verified in jsdom | +| FR7 | Sub-menu rows follow `component:DropdownMenu` FR12 inside the pointer menu: ContextMenu mounts the same drill-in view stack, named after its `label`, so a sub-menu drills in on a phone here too. | `component:DropdownMenu/DEC-2` | Proposed; verified in jsdom; owner decision pending | +| FR8 | In the pointer menu ArrowDown on the last enabled row wraps to the first and ArrowUp on the first to the last; PageDown and PageUp page as in DropdownMenu FR6; a held key's auto-repeat does not activate; typeahead matches the row's label element alone. | Current source and tests; shared `useListFocus` and `useTypeahead` suites | Proposed; verified in jsdom; owner decision pending | +| FR9 | While the Pointer menu surface is open, it participates in `family:overlay-dismissal` through the shared stack and provides descendant depth. One Escape press affects only the top-most present nested surface; ContextMenu closes only when it is that surface. | `family:overlay-dismissal` FR1–FR6; `ContextMenu.test.tsx` nested-layer case | Current family projection; verified in jsdom | +| FR10 | A right-click or long press MUST open the Pointer menu surface with the menu focused (`tabIndex={-1}`, no outline) and no row highlighted; a keyboard open (Shift+F10, Menu key: the input modality is keyboard, as before any input, or an event at (0, 0) with no detail) MUST focus the first enabled row, or with none keep focus on the invoking element. Any other open, as an assistive technology's, is a pointer open. | DropdownMenu trigger opens; `module:DropdownMenu/useMenuPress` AR1; tests | Proposed; pending owner review | ### Allowed variation @@ -214,6 +215,7 @@ anchor is positioning infrastructure rather than consumer anatomy. | FR6 | `ContextMenu.test.tsx` keyboard case; shared `useListFocus` and `useTypeahead` suites | wrap at both ends | Clamping arrows fail. | `audit:ContextMenu/behavior` | | Theming anatomy map | `scripts/check-knowledge.mjs` | Canonical anatomy and current target | Missing, extra, prefixed, stale, or unclassified mappings fail repository validation. | `audit:ContextMenu/theming` | | FR6, AR1 | `ContextMenu.test.tsx` "inline trigger (triggerAs)" suite | `span` in a paragraph, default `div` | A block trigger inside prose, or a lost invocation path, fails. | `audit:ContextMenu/behavior` | +| FR10 | `ContextMenu.test.tsx` "open focus" suite; `__tests__/ContextMenuOpenFocus.a11y.chromium.spec.ts` | right-click, long press, real Shift+F10 | A pointer open that highlights a row, or a keyboard open that does not, fails. | `audit:ContextMenu/behavior` | ## Decision log diff --git a/packages/core/src/ContextMenu/ContextMenu.test.tsx b/packages/core/src/ContextMenu/ContextMenu.test.tsx index d3d00b8417338..8afb9b4cd113a 100644 --- a/packages/core/src/ContextMenu/ContextMenu.test.tsx +++ b/packages/core/src/ContextMenu/ContextMenu.test.tsx @@ -37,6 +37,7 @@ import {Divider} from '../Divider'; import {useLayerDismissal} from '../Layer/useLayerDismissal'; import {warmMenuBottomSheet} from '../DropdownMenu/__tests__/warmMenuBottomSheet'; import {COMPACT_TOUCH_PRESENTATION_QUERY} from '../hooks/useAdaptivePresentation'; +import {__resetInteractionModalityForTest} from '../utils/interactionModality'; // The touch sheet is loaded lazily; resolve its modules once up front, so // ContextMenu's own lazy sheet renders within the cases' waitFor. @@ -1351,6 +1352,142 @@ describe('ContextMenu press model', () => { }); }); +describe('ContextMenu open focus', () => { + const ITEMS = [{label: 'Cut'}, {label: 'Copy'}, {label: 'Paste'}]; + const menu = () => screen.getByRole('menu', {hidden: true}); + const item = (name: string) => + screen.getByRole('menuitem', {name, hidden: true}); + // A right-click is a secondary-button pointerdown, then a contextmenu at + // the pointer's position. + function rightClick(target: Element) { + fireEvent.pointerDown(target, { + pointerType: 'mouse', + pointerId: 1, + button: 2, + }); + fireEvent.contextMenu(target, {clientX: 50, clientY: 15, detail: 1}); + } + + beforeEach(() => { + __resetInteractionModalityForTest(); + }); + + it('a right-click opens with the menu focused and no row highlighted', async () => { + render( + +
Right-click me
+ , + ); + rightClick(screen.getByText('Right-click me')); + await waitFor(() => expect(menu()).toHaveFocus()); + expect(document.activeElement?.getAttribute('role')).toBe('menu'); + }); + + it('a long press opens with the menu focused and no row highlighted', async () => { + // Only the long-press delay is faked; the open's focus frame stays real. + vi.useFakeTimers({toFake: ['setTimeout', 'clearTimeout']}); + try { + render( + +
Long-press me
+ , + ); + fireEvent.touchStart(screen.getByTestId('ctx'), { + touches: [{clientX: 20, clientY: 20}], + }); + act(() => { + vi.advanceTimersByTime(500); + }); + } finally { + vi.useRealTimers(); + } + await waitFor(() => expect(menu()).toHaveFocus()); + expect(document.activeElement?.getAttribute('role')).toBe('menu'); + }); + + it('after a pointer open, ArrowDown reaches the first row', async () => { + render( + +
Right-click me
+ , + ); + rightClick(screen.getByText('Right-click me')); + await waitFor(() => expect(menu()).toHaveFocus()); + fireEvent.keyDown(menu(), {key: 'ArrowDown'}); + expect(item('Cut')).toHaveFocus(); + }); + + it('a keyboard-invoked open still focuses the first row', async () => { + render( + + + , + ); + fireEvent.contextMenu(screen.getByRole('button', {name: 'Target'}), { + clientX: 0, + clientY: 0, + detail: 0, + }); + await waitFor(() => expect(item('Cut')).toHaveFocus()); + }); + + it('an open before any input on the page, away from (0, 0), is a keyboard open: the input modality starts as keyboard', async () => { + render( + + + , + ); + fireEvent.contextMenu(screen.getByRole('button', {name: 'Target'}), { + clientX: 40, + clientY: 12, + detail: 0, + }); + await waitFor(() => expect(item('Cut')).toHaveFocus()); + }); + + it('a keyboard open with every row disabled leaves focus, and its ring, on the invoking element', async () => { + render( + + + , + ); + const target = screen.getByRole('button', {name: 'Target'}); + act(() => target.focus()); + fireEvent.keyDown(target, {key: 'F10', shiftKey: true}); + fireEvent.contextMenu(target, {clientX: 30, clientY: 12, detail: 0}); + await waitFor(() => + expect(screen.getByRole('menu', {hidden: true})).toBeInTheDocument(), + ); + // Let the open's focus frame pass. + await act(async () => { + await new Promise(resolve => { + requestAnimationFrame(() => resolve()); + }); + }); + expect(target).toHaveFocus(); + expect(menu()).not.toHaveFocus(); + }); + + it('Shift+F10 reported at the focused element still focuses the first row', async () => { + render( + + + , + ); + const target = screen.getByRole('button', {name: 'Target'}); + // The page was last used with a pointer, then the key: real browsers + // report the contextmenu at the focused element, with no detail. + fireEvent.pointerDown(document.body, {pointerType: 'mouse', pointerId: 1}); + fireEvent.keyDown(target, {key: 'F10', shiftKey: true}); + fireEvent.contextMenu(target, {clientX: 30, clientY: 12, detail: 0}); + await waitFor(() => expect(item('Cut')).toHaveFocus()); + }); +}); + describe('ContextMenuGroup', () => { it('carries the group semantics through the alias, not just the component', async () => { // Three public surfaces alias this component at once. An alias that diff --git a/packages/core/src/ContextMenu/ContextMenu.tsx b/packages/core/src/ContextMenu/ContextMenu.tsx index 322a6c7c85059..a5ef3d48e01a8 100644 --- a/packages/core/src/ContextMenu/ContextMenu.tsx +++ b/packages/core/src/ContextMenu/ContextMenu.tsx @@ -73,6 +73,10 @@ import {useListFocus} from '../hooks/useListFocus'; import {useMenuPress} from '../hooks/useMenuPress'; import {useTypeahead} from '../hooks/useTypeahead'; import {useLongPress} from '../hooks/useLongPress'; +import { + getInteractionModality, + useInteractionModalityTracking, +} from '../utils/interactionModality'; import {layerAnimations} from '../Layer/layerAnimations.stylex'; import { colorVars, @@ -148,6 +152,8 @@ const styles = stylex.create({ transitionTimingFunction: easeVars['--ease-standard'], userSelect: 'none', WebkitTouchCallout: 'none', + // A pointer open focuses the menu itself; the rows carry the highlight. + outline: 'none', }, // Scroll ownership by the browser's own signal; see DropdownMenu. touchPinchZoom: { @@ -374,6 +380,9 @@ export function ContextMenu({ // outside. const openingGestureRef = useRef(null); + // Records how the user last interacted, so a keyboard-invoked open can be + // told from a right-click (see handleContextMenu). + useInteractionModalityTracking(); const [isOpen, setIsOpen] = useState(false); const [submenuPath, setSubmenuPath] = useState([]); const currentSubmenu = submenuPath.at(-1); @@ -563,7 +572,12 @@ export function ContextMenu({ // what makes the menu context-relative: it scrolls with the content and the // browser auto-flips it against the viewport edges via CSS anchor positioning. const openAtLocalPoint = useCallback( - (localX: number, localY: number, focusEl: HTMLElement | null) => { + ( + localX: number, + localY: number, + focusEl: HTMLElement | null, + modality: 'keyboard' | 'pointer', + ) => { const rect = triggerRef.current?.getBoundingClientRect(); const x = rect ? keepAnchorInside(localX, rect.width) : localX; const y = rect ? keepAnchorInside(localY, rect.height) : localY; @@ -583,10 +597,23 @@ export function ContextMenu({ updateOpenState(true); } else { layer.show(); - requestAnimationFrame(() => focusFirst()); + // A keyboard-invoked open (Shift+F10, the Menu key) lands on the + // first enabled row per the APG menu pattern; with no row that can + // take focus, focus stays, with its ring, on the element that invoked + // the menu. A right-click or a long press focuses the menu itself + // (tabIndex={-1}), so no row is lit as if pre-selected; arrows, + // typeahead and Escape still reach the menu's key handler. + // DropdownMenu opens from its trigger the same way. + requestAnimationFrame(() => { + if (modality === 'keyboard') { + focusFirst(); + return; + } + listRef.current?.focus(); + }); } }, - [layer, focusFirst, updateOpenState, usesBottomSheet], + [layer, focusFirst, listRef, updateOpenState, usesBottomSheet], ); // The box the cursor anchor's offsets are measured from: the trigger, which @@ -622,7 +649,21 @@ export function ContextMenu({ (e.nativeEvent as Partial).pointerType === 'touch' ? currentGesture() : null; - openAtLocalPoint(localX, localY, e.currentTarget as HTMLElement); + // Where focus lands follows how the menu was asked for. Real browsers + // report Shift+F10 and the Menu key at the focused element, not at + // (0, 0), so the shared last-input modality is the signal: a key press + // before this event makes it a keyboard open; a right-click's own + // pointerdown makes it a pointer open. + const modality = + isKeyboardInvoked || getInteractionModality() === 'keyboard' + ? 'keyboard' + : 'pointer'; + openAtLocalPoint( + localX, + localY, + e.currentTarget as HTMLElement, + modality, + ); }, [isDisabled, openAtLocalPoint, getAnchorBaseRect], ); @@ -650,6 +691,7 @@ export function ContextMenu({ rect ? point.x - rect.left : point.x, rect ? point.y - rect.top : point.y, triggerRef.current, + 'pointer', ); }, [openAtLocalPoint, getAnchorBaseRect], diff --git a/packages/core/src/ContextMenu/__tests__/ContextMenuOpenFocus.a11y.chromium.spec.ts b/packages/core/src/ContextMenu/__tests__/ContextMenuOpenFocus.a11y.chromium.spec.ts new file mode 100644 index 0000000000000..341086cb0bdb2 --- /dev/null +++ b/packages/core/src/ContextMenu/__tests__/ContextMenuOpenFocus.a11y.chromium.spec.ts @@ -0,0 +1,100 @@ +// Copyright (c) Meta Platforms, Inc. and affiliates. + +/** + * @file ContextMenuOpenFocus.a11y.chromium.spec.ts + * @input The `Core/ContextMenu` Default story in a built Storybook, a real + * mouse right-click, and a CDP touch long press. + * @output Real-engine proof that a pointer-opened context menu focuses the + * menu itself and highlights no row, and that Shift+F10, which Chromium + * reports at the focused element rather than at (0, 0), still lands on the + * first row. + * @position Run with `pnpm test:a11y-contract`; jsdom cannot produce a real + * right-click or a held finger. + */ + +import {expect, test, type Page} from '@playwright/test'; +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-contextmenu--default'; +const TRIGGER_TEXT = 'Right-click this area'; + +async function focusedRole(page: Page): Promise { + return page.evaluate( + () => document.activeElement?.getAttribute('role') ?? null, + ); +} + +async function triggerCenter(page: Page): Promise<{x: number; y: number}> { + const box = await page.getByText(TRIGGER_TEXT).boundingBox(); + if (box == null) { + throw new Error('the trigger area has no rendered bounds'); + } + return {x: box.x + box.width / 2, y: box.y + box.height / 2}; +} + +test('a right-click opens with the menu focused and no row highlighted', async ({ + page, +}) => { + await page.goto(`${storybook.origin}/iframe.html?id=${STORY}&viewMode=story`); + const center = await triggerCenter(page); + await page.mouse.click(center.x, center.y, {button: 'right'}); + await expect(page.getByRole('menu')).toBeVisible(); + await expect.poll(async () => focusedRole(page)).toBe('menu'); + // Arrows still work from the menu: the first press lands on the first row. + await page.keyboard.press('ArrowDown'); + await expect(page.getByRole('menuitem', {name: 'Cut'})).toBeFocused(); +}); + +test('a long press opens with the menu focused and no row highlighted', async ({ + page, +}) => { + await page.goto(`${storybook.origin}/iframe.html?id=${STORY}&viewMode=story`); + const cdp = await page.context().newCDPSession(page); + await cdp.send('Emulation.setTouchEmulationEnabled', { + enabled: true, + maxTouchPoints: 1, + }); + const center = await triggerCenter(page); + await cdp.send('Input.dispatchTouchEvent', { + type: 'touchStart', + touchPoints: [{x: center.x, y: center.y, id: 1}], + }); + // Read while the finger is still down: the open's focus is what is + // measured here, not what the lift does afterwards. + await expect(page.getByRole('menu')).toBeVisible({timeout: 2000}); + await expect.poll(async () => focusedRole(page)).toBe('menu'); + await cdp.send('Input.dispatchTouchEvent', { + type: 'touchEnd', + touchPoints: [], + }); +}); + +test('Shift+F10 on a focused reference opens with the first row focused', async ({ + page, +}) => { + await page.goto( + `${storybook.origin}/iframe.html?id=core-contextmenu--inline-trigger&viewMode=story`, + ); + // The page was last used with a pointer; the key press decides the open. + await page.mouse.click(5, 5); + await page.getByRole('link', {name: 'T123'}).focus(); + await page.keyboard.press('Shift+F10'); + await expect(page.getByRole('menu')).toBeVisible(); + await expect(page.getByRole('menuitem', {name: 'Open'})).toBeFocused(); +});