From b96c6f0a8900756af79e7ae59f32c39ddc4a53c6 Mon Sep 17 00:00:00 2001 From: Christopher Chedeau Date: Tue, 20 Oct 2026 13:00:31 -0700 Subject: [PATCH 1/3] fix(ContextMenu): the long press that opens the menu no longer closes it The menu closed on any document mousedown outside it, and a touch's compatibility mousedown (sent as the finger lifts in Chromium, reported mid-hold on iOS Safari) lands on the trigger, so the opening long press dismissed its own menu. Outside dismissal now reads pointer events: a mouse press outside closes at once, a finger's or a pen's tap outside closes it on the lift, and a scroll that starts outside still leaves it open. Some outside presses change: a pen now closes it on its lift rather than at the press, and presses the mousedown listener never saw now close it: a press on a control that cancels its pointerdown, and, on the lift, a finger drag on a touch-action: none surface or a second finger's tap. Proposed FR11 of component:ContextMenu records the rule (another pull request proposes FR10, where focus lands on open), and the layer runtime record names the new listeners. Outside presses are read in the capture phase, so a control that stops the press (a resize handle) still closes the menu. Inside a shadow root, even a closed or nested one, the document sees a press on the menu as a press on a shadow host. Each root from the document down to the menu's own listens in the capture phase and defers a press on the host of the next root down to that root, and a press on such a host itself is judged by that host, so a press on the menu never closes it and a stopped press outside still does. A story renders the menu in a shadow root; the shadow-root spec checks in Chromium that a clicked item is picked, that a press or a tap on the host closes the menu, and that its opening long press does not. --- .changeset/context-menu-long-press-lift.md | 9 + .../MenuPressShadowRootA11y.stories.tsx | 52 +++- docs/architecture/layer-runtime.md | 6 +- .../core/src/ContextMenu/ContextMenu.spec.md | 30 +- .../core/src/ContextMenu/ContextMenu.test.tsx | 278 +++++++++++++++++- packages/core/src/ContextMenu/ContextMenu.tsx | 119 ++++++-- ...ContextMenuLongPress.a11y.chromium.spec.ts | 117 ++++++++ .../MenuPressShadowRoot.a11y.chromium.spec.ts | 101 ++++++- 8 files changed, 669 insertions(+), 43 deletions(-) create mode 100644 .changeset/context-menu-long-press-lift.md create mode 100644 packages/core/src/ContextMenu/__tests__/ContextMenuLongPress.a11y.chromium.spec.ts diff --git a/.changeset/context-menu-long-press-lift.md b/.changeset/context-menu-long-press-lift.md new file mode 100644 index 0000000000000..375cc411b323c --- /dev/null +++ b/.changeset/context-menu-long-press-lift.md @@ -0,0 +1,9 @@ +--- +'@astryxdesign/core': patch +--- + +[fix] A ContextMenu opened by a long press stays open as the finger lifts: the compatibility `mousedown` the browser sends for the touch no longer reads as a press outside. Outside dismissal follows pointer events: a mouse press outside closes the menu at once, a finger's or a pen's tap outside closes it on the lift, and a scroll that starts outside still leaves it open. + +Some outside presses now behave differently. A pen closes the menu on its lift rather than at the press, as the menu press model treats a pen like a finger. Presses the `mousedown` listener never saw now close it too: a press on a control that cancels or stops its `pointerdown` (a NumberInput stepper, a resize handle), and, on the lift, a finger drag on a surface with `touch-action: none` (a Slider) or a second finger's tap. + +@vjeux diff --git a/apps/storybook/stories/MenuPressShadowRootA11y.stories.tsx b/apps/storybook/stories/MenuPressShadowRootA11y.stories.tsx index 8ecc849c683b9..d161e31c1101d 100644 --- a/apps/storybook/stories/MenuPressShadowRootA11y.stories.tsx +++ b/apps/storybook/stories/MenuPressShadowRootA11y.stories.tsx @@ -2,10 +2,12 @@ /** * @file MenuPressShadowRootA11y.stories.tsx - * @input Uses Selector from core, React DOM's createRoot, an open shadow root - * @output The fixture the menu-press shadow-root spec drives in a real + * @input Uses Selector and ContextMenu from core, React DOM's createRoot, an + * open shadow root + * @output The fixtures the menu-press shadow-root spec drives in a real * browser: a Selector rendered inside an open shadow root, with its value - * printed outside it. + * printed outside it, and a ContextMenu inside one whose host leaves a + * margin of its own around it, with the last item picked printed outside. * @position The reproduction path for * `packages/core/src/hooks/__tests__/MenuPressShadowRoot.a11y.chromium.spec.ts`. * @@ -21,6 +23,7 @@ import type {Meta, StoryObj} from '@storybook/react'; import {useEffect, useRef, useState} from 'react'; import {createRoot} from 'react-dom/client'; import {Selector} from '@astryxdesign/core/Selector'; +import {ContextMenu} from '@astryxdesign/core/ContextMenu'; const meta: Meta = { title: 'a11y/Menu press in a shadow root', @@ -71,3 +74,46 @@ export const SelectorInShadowRoot: StoryObj = { ); }, }; + +function ShadowContextMenu({onPick}: {onPick: (label: string) => void}) { + return ( + onPick('Cut')}, + {label: 'Copy', onClick: () => onPick('Copy')}, + ]}> +
+ Right-click inside the shadow root +
+ + ); +} + +export const ContextMenuInShadowRoot: StoryObj = { + render: function ContextMenuInShadowRootStory() { + const hostRef = useRef(null); + const [picked, setPicked] = useState(''); + useEffect(() => { + const host = hostRef.current; + if (host == null) { + return; + } + const shadow = host.shadowRoot ?? host.attachShadow({mode: 'open'}); + const container = document.createElement('div'); + shadow.append(container); + const root = createRoot(container); + root.render(); + return () => { + root.unmount(); + container.remove(); + }; + }, []); + return ( + <> + {/* The host's own padding is a press on the host, outside its tree. */} +
+ {picked} + + ); + }, +}; diff --git a/docs/architecture/layer-runtime.md b/docs/architecture/layer-runtime.md index 7c13e10f55425..c757d8074c9e4 100644 --- a/docs/architecture/layer-runtime.md +++ b/docs/architecture/layer-runtime.md @@ -255,7 +255,11 @@ independent: - auto popovers use native light dismiss; - native dialogs handle backdrop clicks locally; -- ContextMenu uses a local outside `mousedown` listener; +- ContextMenu uses local outside `pointerdown`, `pointerup` and + `pointercancel` listeners: a mouse press closes it at once, a finger's or a + pen's on a lift that was not cancelled. Inside shadow roots its + `pointerdown` listeners also sit on each root and host around the menu, + which alone see where a press landed; - Tooltip and HoverCard use hover/focus loss plus a touch-only outside listener; - sheets own scrim and swipe behavior; and - dropdown submenus encode their root-close relationship locally through diff --git a/packages/core/src/ContextMenu/ContextMenu.spec.md b/packages/core/src/ContextMenu/ContextMenu.spec.md index 00182e292b855..1f51468eabb5c 100644 --- a/packages/core/src/ContextMenu/ContextMenu.spec.md +++ b/packages/core/src/ContextMenu/ContextMenu.spec.md @@ -81,17 +81,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. 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. 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 | +| FR11 | A press outside the Pointer menu surface MUST close it, whatever the pressed element does with the event: a mouse press at once, a finger or pen on a tap (lifted, not cancelled), as `module:DropdownMenu/useMenuPress` groups a pen with a finger, while a scroll that starts outside leaves it open. A touch's compatibility mouse events MUST NOT close it, so the long press that opens the menu never closes it. | `architecture:interaction-modality` INV4; tests | Proposed; pending owner review | ### Allowed variation @@ -190,9 +191,9 @@ anchor is positioning infrastructure rather than consumer anatomy. - `architecture:interaction-modality` owns the shared right-click, keyboard, long-press, pointer, touch, and adaptive-presentation boundaries. - `architecture:layer-runtime` owns the current context-mode `useLayer` host and - cursor-anchor positioning. ContextMenu retains local outside-click handling and - joins shared Escape ordering through `useLayerDismissal`; touch hosting delegates - to BottomSheet. + cursor-anchor positioning. ContextMenu retains local outside-press handling + (FR11) and joins shared Escape ordering through `useLayerDismissal`; touch + hosting delegates to BottomSheet. - `family:overlay-dismissal` owns shared Escape and platform-close ordering. ContextMenu's pointer presentation is a shared-stack adopter; the touch presentation delegates dismissal to BottomSheet, which is also a shared-stack @@ -210,6 +211,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` | +| FR11 | `ContextMenu.test.tsx` outside dismissal suite; `ContextMenuLongPress.a11y.chromium.spec.ts` | long press and lift, tap, pen, scroll, mouse, drag | A long press closed by its own lift or compat `mousedown`, or a scroll that closes, 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 2015dc90c342f..8a70bc0cba332 100644 --- a/packages/core/src/ContextMenu/ContextMenu.test.tsx +++ b/packages/core/src/ContextMenu/ContextMenu.test.tsx @@ -8,7 +8,7 @@ * SYNC: When ContextMenu.tsx changes, update tests to match new behavior */ -import {useRef, useState} from 'react'; +import {useRef, useState, type PointerEvent} from 'react'; import {createPortal} from 'react-dom'; import {describe, it, expect, vi, beforeAll, beforeEach} from 'vitest'; import {render, screen, fireEvent, act, waitFor} from '@testing-library/react'; @@ -369,10 +369,16 @@ describe('ContextMenu', () => { fireEvent.contextMenu(container.querySelector('div div')!); // At the document a press on the menu is retargeted to a shadow host. const item = container.querySelector('[role="menuitem"]')!; - fireEvent.mouseDown(item, {composed: true}); + const mouse = {pointerType: 'mouse', pointerId: 1, composed: true}; + fireEvent.pointerDown(item, mouse); + expect(HTMLElement.prototype.hidePopover).not.toHaveBeenCalled(); + // A finger on the menu does not close it on the lift either. + const finger = {pointerType: 'touch', pointerId: 2, composed: true}; + fireEvent.pointerDown(item, finger); + fireEvent.pointerUp(item, finger); expect(HTMLElement.prototype.hidePopover).not.toHaveBeenCalled(); // A press on the host itself is outside the menu. - fireEvent.mouseDown(host); + fireEvent.pointerDown(host, mouse); expect(HTMLElement.prototype.hidePopover).toHaveBeenCalled(); } finally { host.remove(); @@ -380,6 +386,58 @@ describe('ContextMenu', () => { }, ); + it('closes on a press that a control stops in any shadow root around it, and on a finger lifted on a host that stops it', () => { + const host = document.createElement('div'); + document.body.append(host); + const outer = host.attachShadow({mode: 'closed'}); + // A control in the shadow root around the menu's own stops the press. + const outerHandle = document.createElement('div'); + outerHandle.addEventListener('pointerdown', event => + event.stopPropagation(), + ); + const innerHost = document.createElement('div'); + innerHost.addEventListener('pointerdown', event => event.stopPropagation()); + outer.append(outerHandle, innerHost); + const container = document.createElement('div'); + innerHost.attachShadow({mode: 'closed'}).append(container); + const stop = (event: PointerEvent) => event.stopPropagation(); + const mouse = {pointerType: 'mouse', pointerId: 1, composed: true}; + const finger = {pointerType: 'touch', pointerId: 3, composed: true}; + const close = HTMLElement.prototype.hidePopover; + try { + render( + <> + +
Right-click me
+ +
+ , + {container}, + ); + const trigger = container.querySelector('div div')!; + // A control in the menu's own root stops the press before it bubbles. + fireEvent.contextMenu(trigger); + fireEvent.pointerDown( + container.querySelector('[data-testid="handle"]')!, + mouse, + ); + expect(close).toHaveBeenCalledTimes(1); + // So does the one in the root around it. + fireEvent.contextMenu(trigger); + fireEvent.pointerDown(outerHandle, mouse); + expect(close).toHaveBeenCalledTimes(2); + // A finger on the inner host itself closes it on the lift, though the + // host stops the press. + fireEvent.contextMenu(trigger); + fireEvent.pointerDown(innerHost, finger); + expect(close).toHaveBeenCalledTimes(2); + fireEvent.pointerUp(innerHost, finger); + expect(close).toHaveBeenCalledTimes(3); + } finally { + host.remove(); + } + }); + it('closes the menu when Tab is pressed inside it (APG menu pattern)', () => { render( @@ -603,6 +661,220 @@ describe('ContextMenu', () => { }); }); +describe('ContextMenu outside dismissal', () => { + const finger = (pointerId: number) => ({pointerType: 'touch', pointerId}); + + // Holds a finger on the trigger until the long-press opens the menu. Only + // the long-press delay is faked. + function longPress(trigger: HTMLElement, pointerId: number) { + vi.useFakeTimers({toFake: ['setTimeout', 'clearTimeout']}); + try { + fireEvent.pointerDown(trigger, finger(pointerId)); + fireEvent.touchStart(trigger, {touches: [{clientX: 20, clientY: 20}]}); + act(() => { + vi.advanceTimersByTime(500); + }); + } finally { + vi.useRealTimers(); + } + } + + // What a browser sends after a touch: the lift, then compatibility mouse + // events aimed at the element the finger held. + function lift(target: HTMLElement, pointerId: number) { + fireEvent.pointerUp(target, finger(pointerId)); + fireEvent.touchEnd(target); + fireEvent.mouseDown(target); + fireEvent.mouseUp(target); + fireEvent.click(target); + } + + function renderMenu(onOpenChange: (isOpen: boolean) => void) { + render( + <> + +
Long-press me
+ + + , + ); + return { + trigger: screen.getByTestId('ctx'), + outside: screen.getByRole('button', {name: 'Outside'}), + }; + } + + it('the long press that opened the menu does not close it, mid-hold or as it lifts', () => { + const onOpenChange = vi.fn<(isOpen: boolean) => void>(); + const {trigger} = renderMenu(onOpenChange); + longPress(trigger, 1); + expect(onOpenChange).toHaveBeenLastCalledWith(true); + // iOS: a compatibility mousedown while the finger is still down. + fireEvent.mouseDown(trigger); + expect(onOpenChange).toHaveBeenLastCalledWith(true); + // Chromium: the compatibility events that follow the lift. + lift(trigger, 1); + expect(onOpenChange).toHaveBeenLastCalledWith(true); + expect(onOpenChange).not.toHaveBeenCalledWith(false); + }); + + it('a long press that moves the open menu keeps it open as it lifts', () => { + const onOpenChange = vi.fn<(isOpen: boolean) => void>(); + const {trigger} = renderMenu(onOpenChange); + fireEvent.contextMenu(trigger, {clientX: 50, clientY: 15, detail: 1}); + expect(onOpenChange).toHaveBeenLastCalledWith(true); + longPress(trigger, 2); + lift(trigger, 2); + expect(onOpenChange).not.toHaveBeenCalledWith(false); + }); + + it('a tap outside closes the menu; a scroll that starts outside leaves it open', () => { + const onOpenChange = vi.fn<(isOpen: boolean) => void>(); + const {trigger, outside} = renderMenu(onOpenChange); + fireEvent.contextMenu(trigger, {clientX: 50, clientY: 15, detail: 1}); + // The browser takes this finger for a scroll and cancels it. + fireEvent.pointerDown(outside, finger(3)); + fireEvent.pointerCancel(outside, finger(3)); + expect(onOpenChange).toHaveBeenLastCalledWith(true); + fireEvent.pointerDown(outside, finger(4)); + lift(outside, 4); + expect(onOpenChange).toHaveBeenLastCalledWith(false); + }); + + it('a mouse press outside closes the menu', () => { + const onOpenChange = vi.fn<(isOpen: boolean) => void>(); + const {trigger, outside} = renderMenu(onOpenChange); + fireEvent.contextMenu(trigger, {clientX: 50, clientY: 15, detail: 1}); + fireEvent.pointerDown(outside, {pointerType: 'mouse', pointerId: 1}); + fireEvent.mouseDown(outside); + expect(onOpenChange).toHaveBeenLastCalledWith(false); + }); + + it('a pen closes the menu on a tap outside, as a finger does, and a cancelled pen press leaves it open', () => { + const onOpenChange = vi.fn<(isOpen: boolean) => void>(); + const {trigger, outside} = renderMenu(onOpenChange); + fireEvent.contextMenu(trigger, {clientX: 50, clientY: 15, detail: 1}); + const pen = (pointerId: number) => ({pointerType: 'pen', pointerId}); + // A pen press the browser takes for a scroll: the menu stays open. + fireEvent.pointerDown(outside, pen(7)); + fireEvent.mouseDown(outside); + fireEvent.pointerCancel(outside, pen(7)); + expect(onOpenChange).toHaveBeenLastCalledWith(true); + // A pen tap: the press alone keeps the menu, its lift closes it. + fireEvent.pointerDown(outside, pen(8)); + expect(onOpenChange).toHaveBeenLastCalledWith(true); + fireEvent.pointerUp(outside, pen(8)); + expect(onOpenChange).toHaveBeenLastCalledWith(false); + }); + + it('closes on the presses a mousedown listener missed: a control that cancels its pointerdown, a finger drag on a surface that does not pan, and a second finger, on its lift', () => { + const onOpenChange = vi.fn<(isOpen: boolean) => void>(); + render( + <> + +
Right-click me
+ + +
+ , + ); + const trigger = screen.getByTestId('ctx'); + fireEvent.contextMenu(trigger, {clientX: 50, clientY: 15, detail: 1}); + // The control cancels the press, so no compatibility mousedown follows. + fireEvent.pointerDown(screen.getByRole('button', {name: 'Step'}), { + pointerType: 'mouse', + pointerId: 1, + }); + expect(onOpenChange).toHaveBeenLastCalledWith(false); + fireEvent.contextMenu(trigger, {clientX: 50, clientY: 15, detail: 1}); + expect(onOpenChange).toHaveBeenLastCalledWith(true); + // A drag the browser does not pan: never cancelled, and no compatibility + // mouse events; the lift closes the menu. + const track = screen.getByTestId('track'); + fireEvent.pointerDown(track, finger(9)); + fireEvent.pointerMove(track, finger(9)); + expect(onOpenChange).toHaveBeenLastCalledWith(true); + fireEvent.pointerUp(track, finger(9)); + expect(onOpenChange).toHaveBeenLastCalledWith(false); + // A second finger's tap, which sends no compatibility mouse events. + fireEvent.contextMenu(trigger, {clientX: 50, clientY: 15, detail: 1}); + expect(onOpenChange).toHaveBeenLastCalledWith(true); + fireEvent.pointerDown(track, {...finger(10), isPrimary: false}); + fireEvent.pointerUp(track, {...finger(10), isPrimary: false}); + expect(onOpenChange).toHaveBeenLastCalledWith(false); + }); + + it('closes on a press whose control stops it, as a resize handle does, for a mouse and for a finger', () => { + const onOpenChange = vi.fn<(isOpen: boolean) => void>(); + const stop = (event: PointerEvent) => event.stopPropagation(); + render( + <> + +
Right-click me
+ +
+ , + ); + const trigger = screen.getByTestId('ctx'); + const handle = screen.getByTestId('handle'); + fireEvent.contextMenu(trigger, {clientX: 50, clientY: 15, detail: 1}); + fireEvent.pointerDown(handle, {pointerType: 'mouse', pointerId: 1}); + expect(onOpenChange).toHaveBeenLastCalledWith(false); + fireEvent.contextMenu(trigger, {clientX: 50, clientY: 15, detail: 1}); + expect(onOpenChange).toHaveBeenLastCalledWith(true); + fireEvent.pointerDown(handle, finger(11)); + expect(onOpenChange).toHaveBeenLastCalledWith(true); + fireEvent.pointerUp(handle, finger(11)); + expect(onOpenChange).toHaveBeenLastCalledWith(false); + }); + + it('a tap outside closes the menu even when the host re-renders mid-tap with an inline onOpenChange', () => { + const calls: boolean[] = []; + // A new inline handler on every render gives the menu's close a new + // identity each time, as a host that re-renders often does. + const ui = () => ( + <> + calls.push(isOpen)}> +
Long-press me
+ + + + ); + const {rerender} = render(ui()); + fireEvent.contextMenu(screen.getByTestId('ctx'), { + clientX: 50, + clientY: 15, + detail: 1, + }); + expect(calls.at(-1)).toBe(true); + const outside = screen.getByRole('button', {name: 'Outside'}); + fireEvent.pointerDown(outside, finger(5)); + // The host re-renders between the finger's press and its lift. + rerender(ui()); + lift(outside, 5); + expect(calls.at(-1)).toBe(false); + }); +}); + describe('ContextMenu items', () => { it('renders items with labels', () => { render( diff --git a/packages/core/src/ContextMenu/ContextMenu.tsx b/packages/core/src/ContextMenu/ContextMenu.tsx index 1fc573da53fae..6c4673007936a 100644 --- a/packages/core/src/ContextMenu/ContextMenu.tsx +++ b/packages/core/src/ContextMenu/ContextMenu.tsx @@ -354,6 +354,11 @@ export function ContextMenu({ // Element focused before the menu opened, restored when it closes so focus // does not fall to after Escape or outside-click dismissal. const triggerFocusRef = useRef(null); + // Fingers and pens that went down outside the open menu; each closes it if + // it lifts without the browser cancelling it (a tap, not a scroll). + const outsidePressesRef = useRef(new Set()); + // The latest closeMenu, for the outside-press listeners (see below). + const closeMenuRef = useRef<() => void>(() => {}); const [isOpen, setIsOpen] = useState(false); const [submenuPath, setSubmenuPath] = useState([]); @@ -458,37 +463,112 @@ export function ContextMenu({ ), }); - // Dismiss on any click outside the menu. We use popover="manual" (not - // "auto") because the native light-dismiss treats the mouseup from the - // opening right-click as a dismiss event. Handling it ourselves via - // mousedown avoids that race. + // Dismiss on a press outside the menu. The menu is popover="manual" (not + // "auto") because native light dismiss reads the release of the opening + // right-click as a dismissal, so the press is read here, from pointer + // events: a touch's compatibility mouse events (the mousedown Chromium sends + // as a long press lifts, or one sent while it is still held) + // have no pointer event of their own, so the press that opened the menu + // never closes it. A mouse press outside closes the menu at once; a finger + // or a pen closes it on a tap, lifted rather than cancelled, so a scroll + // that starts outside leaves it open, as before. A pen goes with the finger + // because the menu press model (useMenuPress) groups them. They listen in + // the capture phase, so a control that stops the press (a resize handle) + // still closes the menu. + // + // Inside a shadow root, even a closed or nested one, the document sees a + // press on the menu as a press on a shadow host, and only the menu's own + // root sees where it landed. So each root from the document down to the + // menu's own listens too: capture reaches the outer ones first, and each + // defers a press on the host of the next root down to that root. A press + // on such a host itself reaches no deeper root, so the host judges it, + // whatever the host's own listeners do with it. Each press is judged once. + // + // The listeners read the latest `closeMenu` through a ref, so they are + // installed once per open: a caller's inline `onOpenChange` gives + // `closeMenu` a new identity on every render, and re-installing them would + // forget a press recorded between its down and its lift. + closeMenuRef.current = closeMenu; useEffect(() => { if (!isOpen || usesBottomSheet) { return; } - // Inside a shadow root, even a closed or nested one, the document sees a - // press on the menu as a press on an outer shadow host, so the menu's own - // tree judges its presses first and the document skips those. - const tree = listRef.current?.getRootNode(); - const shadow = tree instanceof ShadowRoot ? tree : null; + const outsidePresses = outsidePressesRef.current; + const pressOutside = (e: PointerEvent) => { + if (e.pointerType === 'touch' || e.pointerType === 'pen') { + outsidePresses.add(e.pointerId); + return; + } + closeMenuRef.current(); + }; + // The menu's own root, each root around it, and the document, each with + // the host of the root below it (none for the menu's own). + const levels: [Document | ShadowRoot, Element | null][] = []; + let tree: Node | undefined = listRef.current?.getRootNode(); + let innerHost: Element | null = null; + while (tree instanceof ShadowRoot) { + levels.push([tree, innerHost]); + innerHost = tree.host; + tree = innerHost.getRootNode(); + } + levels.push([document, innerHost]); const judged = new WeakSet(); - const handleClickOutside = (e: Event) => { - const menu = listRef.current; + const judgePress = (e: Event) => { if (judged.has(e)) { return; } judged.add(e); - if (menu && !menu.contains(e.target as Node)) { - closeMenu(); + const menu = listRef.current; + if (menu != null && !menu.contains(e.target as Node)) { + pressOutside(e as PointerEvent); + } + }; + const pressListeners = levels.map(([root, hostBelow]) => { + const handlePointerDown = (e: Event) => { + if (hostBelow == null || e.target !== hostBelow) { + judgePress(e); + } + }; + return [root, handlePointerDown] as const; + }); + // A press on a host itself, which no deeper root sees. + const handleUnjudgedPointerDown = (e: Event) => { + if (!judged.has(e)) { + judged.add(e); + pressOutside(e as PointerEvent); + } + }; + const handlePointerUp = (e: PointerEvent) => { + if (outsidePresses.delete(e.pointerId)) { + closeMenuRef.current(); } }; - document.addEventListener('mousedown', handleClickOutside); - shadow?.addEventListener('mousedown', handleClickOutside); + const handlePointerCancel = (e: PointerEvent) => { + outsidePresses.delete(e.pointerId); + }; + for (const [, hostBelow] of levels) { + hostBelow?.addEventListener('pointerdown', handleUnjudgedPointerDown); + } + for (const [root, handlePointerDown] of pressListeners) { + root.addEventListener('pointerdown', handlePointerDown, true); + } + document.addEventListener('pointerup', handlePointerUp, true); + document.addEventListener('pointercancel', handlePointerCancel, true); return () => { - document.removeEventListener('mousedown', handleClickOutside); - shadow?.removeEventListener('mousedown', handleClickOutside); + outsidePresses.clear(); + for (const [, hostBelow] of levels) { + hostBelow?.removeEventListener( + 'pointerdown', + handleUnjudgedPointerDown, + ); + } + for (const [root, handlePointerDown] of pressListeners) { + root.removeEventListener('pointerdown', handlePointerDown, true); + } + document.removeEventListener('pointerup', handlePointerUp, true); + document.removeEventListener('pointercancel', handlePointerCancel, true); }; - }, [isOpen, closeMenu, listRef, usesBottomSheet]); + }, [isOpen, listRef, usesBottomSheet]); const listKeyDown = useCallback( (e: React.KeyboardEvent) => { @@ -554,6 +634,9 @@ export function ContextMenu({ document.activeElement instanceof HTMLElement ? document.activeElement : focusEl; + // A finger held on the trigger while the menu was open is now the long + // press that moved it, not a tap outside it. + outsidePressesRef.current.clear(); if (usesBottomSheet) { updateOpenState(true); } else { diff --git a/packages/core/src/ContextMenu/__tests__/ContextMenuLongPress.a11y.chromium.spec.ts b/packages/core/src/ContextMenu/__tests__/ContextMenuLongPress.a11y.chromium.spec.ts new file mode 100644 index 0000000000000..29102ddf41af7 --- /dev/null +++ b/packages/core/src/ContextMenu/__tests__/ContextMenuLongPress.a11y.chromium.spec.ts @@ -0,0 +1,117 @@ +// Copyright (c) Meta Platforms, Inc. and affiliates. + +/** + * @file ContextMenuLongPress.a11y.chromium.spec.ts + * @input The `Core/ContextMenu` Default story in a built Storybook and CDP + * touch input. + * @output Real-engine proof that the long press that opens a context menu + * does not close it as the finger lifts, and that a tap outside still does. + * @position Run with `pnpm test:a11y-contract`; jsdom cannot produce the + * compatibility mouse events a real browser sends after a touch. + */ + +import {expect, test, type CDPSession, 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'; + +async function touchCdp(page: Page): Promise { + const cdp = await page.context().newCDPSession(page); + await cdp.send('Emulation.setTouchEmulationEnabled', { + enabled: true, + maxTouchPoints: 1, + }); + return cdp; +} + +async function triggerCenter(page: Page): Promise<{x: number; y: number}> { + const box = await page.getByText('Right-click this area').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}; +} + +/** + * Resolves once the browser has delivered the `click` that follows a touch + * (the last of its compatibility mouse events) and two frames have run, so + * anything that event closes has closed. + */ +async function armCompatClick(page: Page): Promise { + await page.evaluate(() => { + const w = window as unknown as {__compatClick?: Promise}; + w.__compatClick = new Promise(resolve => { + document.addEventListener( + 'click', + () => + requestAnimationFrame(() => requestAnimationFrame(() => resolve())), + {once: true, capture: true}, + ); + }); + }); +} + +async function compatClickDelivered(page: Page): Promise { + await page.evaluate( + async () => + (window as unknown as {__compatClick?: Promise}).__compatClick, + ); +} + +test('the long press that opens the menu leaves it open as the finger lifts', async ({ + page, +}) => { + await page.goto(`${storybook.origin}/iframe.html?id=${STORY}&viewMode=story`); + const cdp = await touchCdp(page); + const center = await triggerCenter(page); + await cdp.send('Input.dispatchTouchEvent', { + type: 'touchStart', + touchPoints: [{x: center.x, y: center.y, id: 1}], + }); + const menu = page.getByRole('menu'); + await expect(menu).toBeVisible({timeout: 2000}); + await armCompatClick(page); + await cdp.send('Input.dispatchTouchEvent', { + type: 'touchEnd', + touchPoints: [], + }); + await compatClickDelivered(page); + await expect(menu).toBeVisible(); + + // A tap outside still closes it. + await cdp.send('Input.dispatchTouchEvent', { + type: 'touchStart', + touchPoints: [{x: 5, y: 5, id: 2}], + }); + await cdp.send('Input.dispatchTouchEvent', { + type: 'touchEnd', + touchPoints: [], + }); + await expect(menu).toBeHidden(); +}); + +test('a mouse press outside closes a right-click menu', 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'}); + const menu = page.getByRole('menu'); + await expect(menu).toBeVisible(); + await page.mouse.click(5, 5); + await expect(menu).toBeHidden(); +}); diff --git a/packages/core/src/hooks/__tests__/MenuPressShadowRoot.a11y.chromium.spec.ts b/packages/core/src/hooks/__tests__/MenuPressShadowRoot.a11y.chromium.spec.ts index b7eff77e35b7a..6e0f23a0118c9 100644 --- a/packages/core/src/hooks/__tests__/MenuPressShadowRoot.a11y.chromium.spec.ts +++ b/packages/core/src/hooks/__tests__/MenuPressShadowRoot.a11y.chromium.spec.ts @@ -2,15 +2,17 @@ /** * @file MenuPressShadowRoot.a11y.chromium.spec.ts - * @input The `a11y/Menu press in a shadow root` story in a built Storybook and - * real mouse input. + * @input The `a11y/Menu press in a shadow root` stories in a built Storybook, + * real mouse input, and CDP touch input. * @output Real-engine proof that a menu rendered inside a shadow root acts on - * the row under a released pointer (module:DropdownMenu/useMenuPress FR1). + * the row under a released pointer (module:DropdownMenu/useMenuPress FR1), + * and that a ContextMenu there tells a press on its own rows from a press + * outside it, a mouse's at once and a finger's on its lift. * @position Run with `pnpm test:a11y-contract`. Only a real engine resolves a * point inside a shadow root to the shadow host at the document level. */ -import {expect, test} from '@playwright/test'; +import {expect, test, type Page} from '@playwright/test'; import { DEFAULT_STORYBOOK_DIR, serveStorybook, @@ -48,3 +50,94 @@ test('a mouse click on an option inside a shadow root selects it', async ({ await expect(page.getByTestId('selected-value')).toHaveText('Banana'); }); + +const CONTEXT_STORY = + 'a11y-menu-press-in-a-shadow-root--context-menu-in-shadow-root'; + +/** The center of a box, and a point in the host's own padding. */ +async function points(page: Page) { + const trigger = await page + .getByText('Right-click inside the shadow root') + .boundingBox(); + const host = await page.getByTestId('shadow-host').boundingBox(); + if (trigger == null || host == null) { + throw new Error('the trigger or its shadow host has no layout box'); + } + return { + trigger: { + x: trigger.x + trigger.width / 2, + y: trigger.y + trigger.height / 2, + }, + hostPadding: {x: host.x + 8, y: host.y + 8}, + }; +} + +test('a ContextMenu inside a shadow root picks a clicked item, and a press on its host closes it', async ({ + page, +}) => { + await page.goto( + `${storybook.origin}/iframe.html?id=${CONTEXT_STORY}&viewMode=story`, + ); + const {trigger, hostPadding} = await points(page); + const menu = page.getByRole('menu'); + + // At the document, both presses land on the shadow host. + await page.mouse.click(trigger.x, trigger.y, {button: 'right'}); + await expect(menu).toBeVisible(); + const copy = await page.getByRole('menuitem', {name: 'Copy'}).boundingBox(); + if (copy == null) { + throw new Error('the Copy item has no layout box'); + } + await page.mouse.click(copy.x + copy.width / 2, copy.y + copy.height / 2); + await expect(page.getByTestId('picked-item')).toHaveText('Copy'); + await expect(menu).toBeHidden(); + + await page.mouse.click(trigger.x, trigger.y, {button: 'right'}); + await expect(menu).toBeVisible(); + await page.mouse.click(hostPadding.x, hostPadding.y); + await expect(menu).toBeHidden(); + await expect(page.getByTestId('picked-item')).toHaveText('Copy'); +}); + +test('a ContextMenu inside a shadow root stays open as its opening long press lifts, and a tap on its host closes it', async ({ + page, +}) => { + await page.goto( + `${storybook.origin}/iframe.html?id=${CONTEXT_STORY}&viewMode=story`, + ); + const {trigger, hostPadding} = await points(page); + const menu = page.getByRole('menu'); + const cdp = await page.context().newCDPSession(page); + await cdp.send('Emulation.setTouchEmulationEnabled', { + enabled: true, + maxTouchPoints: 1, + }); + + await cdp.send('Input.dispatchTouchEvent', { + type: 'touchStart', + touchPoints: [{x: trigger.x, y: trigger.y, id: 1}], + }); + await expect(menu).toBeVisible({timeout: 2000}); + await cdp.send('Input.dispatchTouchEvent', { + type: 'touchEnd', + touchPoints: [], + }); + // Two frames after the lift, its compatibility mouse events have landed. + await page.evaluate(async () => { + await new Promise(resolve => { + requestAnimationFrame(() => requestAnimationFrame(() => resolve())); + }); + }); + await expect(menu).toBeVisible(); + + await cdp.send('Input.dispatchTouchEvent', { + type: 'touchStart', + touchPoints: [{x: hostPadding.x, y: hostPadding.y, id: 2}], + }); + await expect(menu).toBeVisible(); + await cdp.send('Input.dispatchTouchEvent', { + type: 'touchEnd', + touchPoints: [], + }); + await expect(menu).toBeHidden(); +}); From 904a58d5f9b0b49fb94210860c603114afc23be4 Mon Sep 17 00:00:00 2001 From: Christopher Chedeau Date: Sat, 24 Oct 2026 16:08:03 -0700 Subject: [PATCH 2/3] fix(ContextMenu): the document's lift listeners are attached only while an outside finger or pen press is live The open menu held document pointerup and pointercancel listeners for its whole open lifetime, though they only matter while a finger's or a pen's outside press is in flight. They now come on with the first such press and off once none is left (a lift, a cancel, or the menu closing), so an open menu with no press in flight holds none (architecture:react-component-runtime INV5); they move onto the shared press-to-release tracker once one exists. The pointerdown listeners that judge a press stay for the open menu, since they are what sees a press begin. layer-runtime.md says so. New case: an open menu holds no document lift listener until a finger presses outside, and drops it with that lift; it fails on the previous head. Every outside-dismissal case still passes. --- docs/architecture/layer-runtime.md | 5 +- .../core/src/ContextMenu/ContextMenu.test.tsx | 49 ++++++++++++++++++ packages/core/src/ContextMenu/ContextMenu.tsx | 50 ++++++++++++++----- 3 files changed, 90 insertions(+), 14 deletions(-) diff --git a/docs/architecture/layer-runtime.md b/docs/architecture/layer-runtime.md index c757d8074c9e4..a32b100e7f468 100644 --- a/docs/architecture/layer-runtime.md +++ b/docs/architecture/layer-runtime.md @@ -257,8 +257,9 @@ independent: - native dialogs handle backdrop clicks locally; - ContextMenu uses local outside `pointerdown`, `pointerup` and `pointercancel` listeners: a mouse press closes it at once, a finger's or a - pen's on a lift that was not cancelled. Inside shadow roots its - `pointerdown` listeners also sit on each root and host around the menu, + pen's on a lift that was not cancelled; the `pointerup` and `pointercancel` + listeners are attached only while such a press is live. Inside shadow roots + its `pointerdown` listeners also sit on each root and host around the menu, which alone see where a press landed; - Tooltip and HoverCard use hover/focus loss plus a touch-only outside listener; - sheets own scrim and swipe behavior; and diff --git a/packages/core/src/ContextMenu/ContextMenu.test.tsx b/packages/core/src/ContextMenu/ContextMenu.test.tsx index 8a70bc0cba332..330a8142b17e0 100644 --- a/packages/core/src/ContextMenu/ContextMenu.test.tsx +++ b/packages/core/src/ContextMenu/ContextMenu.test.tsx @@ -731,6 +731,55 @@ describe('ContextMenu outside dismissal', () => { expect(onOpenChange).not.toHaveBeenCalledWith(false); }); + it('an open menu holds no document lift listener until a finger presses outside, and drops it with that lift', () => { + const live = new Set(); + const isLift = (type: string) => + type === 'pointerup' || type === 'pointercancel'; + const addSpy = vi + .spyOn(document, 'addEventListener') + .mockImplementation(function (this: Document, type, listener, options) { + if (listener != null && isLift(type)) { + live.add(listener); + } + EventTarget.prototype.addEventListener.call( + this, + type, + listener, + options, + ); + }); + const removeSpy = vi + .spyOn(document, 'removeEventListener') + .mockImplementation(function (this: Document, type, listener, options) { + if (listener != null && isLift(type)) { + live.delete(listener); + } + EventTarget.prototype.removeEventListener.call( + this, + type, + listener, + options, + ); + }); + try { + const onOpenChange = vi.fn<(isOpen: boolean) => void>(); + const {trigger, outside} = renderMenu(onOpenChange); + const atRest = new Set(live); + fireEvent.contextMenu(trigger, {clientX: 50, clientY: 15, detail: 1}); + expect(onOpenChange).toHaveBeenLastCalledWith(true); + // Open, with no press in flight: nothing follows a lift. + expect(live).toEqual(atRest); + fireEvent.pointerDown(outside, finger(5)); + expect(live.size).toBeGreaterThan(atRest.size); + lift(outside, 5); + expect(onOpenChange).toHaveBeenLastCalledWith(false); + expect(live).toEqual(atRest); + } finally { + addSpy.mockRestore(); + removeSpy.mockRestore(); + } + }); + it('a tap outside closes the menu; a scroll that starts outside leaves it open', () => { const onOpenChange = vi.fn<(isOpen: boolean) => void>(); const {trigger, outside} = renderMenu(onOpenChange); diff --git a/packages/core/src/ContextMenu/ContextMenu.tsx b/packages/core/src/ContextMenu/ContextMenu.tsx index 6c4673007936a..2dc0af0b1d06d 100644 --- a/packages/core/src/ContextMenu/ContextMenu.tsx +++ b/packages/core/src/ContextMenu/ContextMenu.tsx @@ -494,9 +494,45 @@ export function ContextMenu({ return; } const outsidePresses = outsidePressesRef.current; + // A finger's or a pen's lift is followed only while such a press is live: + // the document's pointerup and pointercancel listeners come on with the + // first one and off once none is left (`architecture:react-component-runtime` + // INV5), so an open menu with no press in flight holds none. + let isFollowingLifts = false; + const stopFollowingLifts = () => { + if (isFollowingLifts) { + isFollowingLifts = false; + document.removeEventListener('pointerup', handlePointerUp, true); + document.removeEventListener( + 'pointercancel', + handlePointerCancel, + true, + ); + } + }; + function handlePointerUp(e: PointerEvent) { + const wasOutside = outsidePresses.delete(e.pointerId); + if (outsidePresses.size === 0) { + stopFollowingLifts(); + } + if (wasOutside) { + closeMenuRef.current(); + } + } + function handlePointerCancel(e: PointerEvent) { + outsidePresses.delete(e.pointerId); + if (outsidePresses.size === 0) { + stopFollowingLifts(); + } + } const pressOutside = (e: PointerEvent) => { if (e.pointerType === 'touch' || e.pointerType === 'pen') { outsidePresses.add(e.pointerId); + if (!isFollowingLifts) { + isFollowingLifts = true; + document.addEventListener('pointerup', handlePointerUp, true); + document.addEventListener('pointercancel', handlePointerCancel, true); + } return; } closeMenuRef.current(); @@ -538,24 +574,16 @@ export function ContextMenu({ pressOutside(e as PointerEvent); } }; - const handlePointerUp = (e: PointerEvent) => { - if (outsidePresses.delete(e.pointerId)) { - closeMenuRef.current(); - } - }; - const handlePointerCancel = (e: PointerEvent) => { - outsidePresses.delete(e.pointerId); - }; for (const [, hostBelow] of levels) { hostBelow?.addEventListener('pointerdown', handleUnjudgedPointerDown); } for (const [root, handlePointerDown] of pressListeners) { root.addEventListener('pointerdown', handlePointerDown, true); } - document.addEventListener('pointerup', handlePointerUp, true); - document.addEventListener('pointercancel', handlePointerCancel, true); return () => { outsidePresses.clear(); + document.removeEventListener('pointerup', handlePointerUp, true); + document.removeEventListener('pointercancel', handlePointerCancel, true); for (const [, hostBelow] of levels) { hostBelow?.removeEventListener( 'pointerdown', @@ -565,8 +593,6 @@ export function ContextMenu({ for (const [root, handlePointerDown] of pressListeners) { root.removeEventListener('pointerdown', handlePointerDown, true); } - document.removeEventListener('pointerup', handlePointerUp, true); - document.removeEventListener('pointercancel', handlePointerCancel, true); }; }, [isOpen, listRef, usesBottomSheet]); From 163af165c8021e1f5278b73b3cd0851bfb43b4f9 Mon Sep 17 00:00:00 2001 From: Christopher Chedeau Date: Sat, 24 Oct 2026 21:16:43 -0700 Subject: [PATCH 3/3] fix(ContextMenu): narrow the change to the long-press lift; other outside presses keep their outcomes Per the owner's requested change, the pull request keeps only the long-press-lift fix. Outside dismissal is main's outside mousedown listener again, with one addition: the compatibility mouse events of the finger press that opened the menu (the long press, or a touch contextmenu where Chromium sends one) are skipped, identified by the shared gesture counter (Layer/gestureCounter), which advances on every pointerdown and keydown; any later mousedown outside closes the menu as on main. A mouse or keyboard open records no gesture, so nothing changes for them. The five other outcome changes are dropped: a pen closes at its press again, and presses that send no mousedown (a control that cancels or stops its pointerdown, such as a NumberInput stepper or a resize handle, a finger drag on a surface that does not pan, a second finger) leave the menu open again. The document pointerdown, pointerup and pointercancel listeners and the nested-root levels are gone, so the open menu adds no listener beyond main's. FR11 states only the long-press rule and points finger and pen outside presses to spec:AST-094; layer-runtime.md and the changeset say the same. Tests: the long press (mid-hold and lift, then a later press closes), a touch contextmenu, and a long press that moves the open menu fail on main; a mouse or pen press closing at the press, and the presses that send no mousedown leaving the menu open, fail on the previous head. The long-press and shadow-root Chromium cases fail on main and pass here. --- .changeset/context-menu-long-press-lift.md | 4 +- docs/architecture/layer-runtime.md | 8 +- .../core/src/ContextMenu/ContextMenu.spec.md | 10 +- .../core/src/ContextMenu/ContextMenu.test.tsx | 479 ++++++------------ packages/core/src/ContextMenu/ContextMenu.tsx | 166 ++---- 5 files changed, 202 insertions(+), 465 deletions(-) diff --git a/.changeset/context-menu-long-press-lift.md b/.changeset/context-menu-long-press-lift.md index 375cc411b323c..789babdf8fbfd 100644 --- a/.changeset/context-menu-long-press-lift.md +++ b/.changeset/context-menu-long-press-lift.md @@ -2,8 +2,6 @@ '@astryxdesign/core': patch --- -[fix] A ContextMenu opened by a long press stays open as the finger lifts: the compatibility `mousedown` the browser sends for the touch no longer reads as a press outside. Outside dismissal follows pointer events: a mouse press outside closes the menu at once, a finger's or a pen's tap outside closes it on the lift, and a scroll that starts outside still leaves it open. - -Some outside presses now behave differently. A pen closes the menu on its lift rather than at the press, as the menu press model treats a pen like a finger. Presses the `mousedown` listener never saw now close it too: a press on a control that cancels or stops its `pointerdown` (a NumberInput stepper, a resize handle), and, on the lift, a finger drag on a surface with `touch-action: none` (a Slider) or a second finger's tap. +[fix] A ContextMenu opened by a finger's long press stays open as the finger lifts: the compatibility `mousedown` the browser sends for that press (while it is held on iOS, as it lifts in Chromium) no longer reads as a press outside. Every other press outside the menu closes it, or leaves it open, as before. @vjeux diff --git a/docs/architecture/layer-runtime.md b/docs/architecture/layer-runtime.md index a32b100e7f468..34814b9e0cd98 100644 --- a/docs/architecture/layer-runtime.md +++ b/docs/architecture/layer-runtime.md @@ -255,12 +255,8 @@ independent: - auto popovers use native light dismiss; - native dialogs handle backdrop clicks locally; -- ContextMenu uses local outside `pointerdown`, `pointerup` and - `pointercancel` listeners: a mouse press closes it at once, a finger's or a - pen's on a lift that was not cancelled; the `pointerup` and `pointercancel` - listeners are attached only while such a press is live. Inside shadow roots - its `pointerdown` listeners also sit on each root and host around the menu, - which alone see where a press landed; +- ContextMenu uses a local outside `mousedown` listener, which skips the + compatibility mouse events of the finger press that opened the menu; - Tooltip and HoverCard use hover/focus loss plus a touch-only outside listener; - sheets own scrim and swipe behavior; and - dropdown submenus encode their root-close relationship locally through diff --git a/packages/core/src/ContextMenu/ContextMenu.spec.md b/packages/core/src/ContextMenu/ContextMenu.spec.md index 1f51468eabb5c..8f94443421c9e 100644 --- a/packages/core/src/ContextMenu/ContextMenu.spec.md +++ b/packages/core/src/ContextMenu/ContextMenu.spec.md @@ -92,7 +92,7 @@ Consumer props, content modes, and presentation policy remain documented in | 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 | -| FR11 | A press outside the Pointer menu surface MUST close it, whatever the pressed element does with the event: a mouse press at once, a finger or pen on a tap (lifted, not cancelled), as `module:DropdownMenu/useMenuPress` groups a pen with a finger, while a scroll that starts outside leaves it open. A touch's compatibility mouse events MUST NOT close it, so the long press that opens the menu never closes it. | `architecture:interaction-modality` INV4; tests | Proposed; pending owner review | +| FR11 | The browser's compatibility mouse events for the finger press that opened the Pointer menu surface (a long press, or a touch `contextmenu`) MUST NOT close it, mid-hold or as the finger lifts; every later outside `mousedown` closes it as before. Finger and pen outside presses follow `spec:AST-094`. | `architecture:interaction-modality` INV4; tests | Proposed; pending owner review | ### Allowed variation @@ -191,9 +191,9 @@ anchor is positioning infrastructure rather than consumer anatomy. - `architecture:interaction-modality` owns the shared right-click, keyboard, long-press, pointer, touch, and adaptive-presentation boundaries. - `architecture:layer-runtime` owns the current context-mode `useLayer` host and - cursor-anchor positioning. ContextMenu retains local outside-press handling - (FR11) and joins shared Escape ordering through `useLayerDismissal`; touch - hosting delegates to BottomSheet. + cursor-anchor positioning. ContextMenu retains local outside-click handling and + joins shared Escape ordering through `useLayerDismissal`; touch hosting delegates + to BottomSheet. - `family:overlay-dismissal` owns shared Escape and platform-close ordering. ContextMenu's pointer presentation is a shared-stack adopter; the touch presentation delegates dismissal to BottomSheet, which is also a shared-stack @@ -211,7 +211,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` | -| FR11 | `ContextMenu.test.tsx` outside dismissal suite; `ContextMenuLongPress.a11y.chromium.spec.ts` | long press and lift, tap, pen, scroll, mouse, drag | A long press closed by its own lift or compat `mousedown`, or a scroll that closes, fails. | `audit:ContextMenu/behavior` | +| FR11 | `ContextMenu.test.tsx` outside dismissal suite; `ContextMenuLongPress.a11y.chromium.spec.ts` | long press: mid-hold, lift; touch contextmenu | A long press closed by its own compatibility `mousedown`, or a changed outside outcome, 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 330a8142b17e0..df3384d4d98c6 100644 --- a/packages/core/src/ContextMenu/ContextMenu.test.tsx +++ b/packages/core/src/ContextMenu/ContextMenu.test.tsx @@ -8,10 +8,18 @@ * SYNC: When ContextMenu.tsx changes, update tests to match new behavior */ -import {useRef, useState, type PointerEvent} from 'react'; +import {useRef, useState} from 'react'; import {createPortal} from 'react-dom'; import {describe, it, expect, vi, beforeAll, beforeEach} from 'vitest'; -import {render, screen, fireEvent, act, waitFor} from '@testing-library/react'; +import { + render, + screen, + fireEvent, + act, + waitFor, + cleanup, + createEvent, +} from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import {readFileSync} from 'node:fs'; import {ContextMenu} from './ContextMenu'; @@ -369,16 +377,10 @@ describe('ContextMenu', () => { fireEvent.contextMenu(container.querySelector('div div')!); // At the document a press on the menu is retargeted to a shadow host. const item = container.querySelector('[role="menuitem"]')!; - const mouse = {pointerType: 'mouse', pointerId: 1, composed: true}; - fireEvent.pointerDown(item, mouse); - expect(HTMLElement.prototype.hidePopover).not.toHaveBeenCalled(); - // A finger on the menu does not close it on the lift either. - const finger = {pointerType: 'touch', pointerId: 2, composed: true}; - fireEvent.pointerDown(item, finger); - fireEvent.pointerUp(item, finger); + fireEvent.mouseDown(item, {composed: true}); expect(HTMLElement.prototype.hidePopover).not.toHaveBeenCalled(); // A press on the host itself is outside the menu. - fireEvent.pointerDown(host, mouse); + fireEvent.mouseDown(host); expect(HTMLElement.prototype.hidePopover).toHaveBeenCalled(); } finally { host.remove(); @@ -386,58 +388,6 @@ describe('ContextMenu', () => { }, ); - it('closes on a press that a control stops in any shadow root around it, and on a finger lifted on a host that stops it', () => { - const host = document.createElement('div'); - document.body.append(host); - const outer = host.attachShadow({mode: 'closed'}); - // A control in the shadow root around the menu's own stops the press. - const outerHandle = document.createElement('div'); - outerHandle.addEventListener('pointerdown', event => - event.stopPropagation(), - ); - const innerHost = document.createElement('div'); - innerHost.addEventListener('pointerdown', event => event.stopPropagation()); - outer.append(outerHandle, innerHost); - const container = document.createElement('div'); - innerHost.attachShadow({mode: 'closed'}).append(container); - const stop = (event: PointerEvent) => event.stopPropagation(); - const mouse = {pointerType: 'mouse', pointerId: 1, composed: true}; - const finger = {pointerType: 'touch', pointerId: 3, composed: true}; - const close = HTMLElement.prototype.hidePopover; - try { - render( - <> - -
Right-click me
- -
- , - {container}, - ); - const trigger = container.querySelector('div div')!; - // A control in the menu's own root stops the press before it bubbles. - fireEvent.contextMenu(trigger); - fireEvent.pointerDown( - container.querySelector('[data-testid="handle"]')!, - mouse, - ); - expect(close).toHaveBeenCalledTimes(1); - // So does the one in the root around it. - fireEvent.contextMenu(trigger); - fireEvent.pointerDown(outerHandle, mouse); - expect(close).toHaveBeenCalledTimes(2); - // A finger on the inner host itself closes it on the lift, though the - // host stops the press. - fireEvent.contextMenu(trigger); - fireEvent.pointerDown(innerHost, finger); - expect(close).toHaveBeenCalledTimes(2); - fireEvent.pointerUp(innerHost, finger); - expect(close).toHaveBeenCalledTimes(3); - } finally { - host.remove(); - } - }); - it('closes the menu when Tab is pressed inside it (APG menu pattern)', () => { render( @@ -661,269 +611,6 @@ describe('ContextMenu', () => { }); }); -describe('ContextMenu outside dismissal', () => { - const finger = (pointerId: number) => ({pointerType: 'touch', pointerId}); - - // Holds a finger on the trigger until the long-press opens the menu. Only - // the long-press delay is faked. - function longPress(trigger: HTMLElement, pointerId: number) { - vi.useFakeTimers({toFake: ['setTimeout', 'clearTimeout']}); - try { - fireEvent.pointerDown(trigger, finger(pointerId)); - fireEvent.touchStart(trigger, {touches: [{clientX: 20, clientY: 20}]}); - act(() => { - vi.advanceTimersByTime(500); - }); - } finally { - vi.useRealTimers(); - } - } - - // What a browser sends after a touch: the lift, then compatibility mouse - // events aimed at the element the finger held. - function lift(target: HTMLElement, pointerId: number) { - fireEvent.pointerUp(target, finger(pointerId)); - fireEvent.touchEnd(target); - fireEvent.mouseDown(target); - fireEvent.mouseUp(target); - fireEvent.click(target); - } - - function renderMenu(onOpenChange: (isOpen: boolean) => void) { - render( - <> - -
Long-press me
- - - , - ); - return { - trigger: screen.getByTestId('ctx'), - outside: screen.getByRole('button', {name: 'Outside'}), - }; - } - - it('the long press that opened the menu does not close it, mid-hold or as it lifts', () => { - const onOpenChange = vi.fn<(isOpen: boolean) => void>(); - const {trigger} = renderMenu(onOpenChange); - longPress(trigger, 1); - expect(onOpenChange).toHaveBeenLastCalledWith(true); - // iOS: a compatibility mousedown while the finger is still down. - fireEvent.mouseDown(trigger); - expect(onOpenChange).toHaveBeenLastCalledWith(true); - // Chromium: the compatibility events that follow the lift. - lift(trigger, 1); - expect(onOpenChange).toHaveBeenLastCalledWith(true); - expect(onOpenChange).not.toHaveBeenCalledWith(false); - }); - - it('a long press that moves the open menu keeps it open as it lifts', () => { - const onOpenChange = vi.fn<(isOpen: boolean) => void>(); - const {trigger} = renderMenu(onOpenChange); - fireEvent.contextMenu(trigger, {clientX: 50, clientY: 15, detail: 1}); - expect(onOpenChange).toHaveBeenLastCalledWith(true); - longPress(trigger, 2); - lift(trigger, 2); - expect(onOpenChange).not.toHaveBeenCalledWith(false); - }); - - it('an open menu holds no document lift listener until a finger presses outside, and drops it with that lift', () => { - const live = new Set(); - const isLift = (type: string) => - type === 'pointerup' || type === 'pointercancel'; - const addSpy = vi - .spyOn(document, 'addEventListener') - .mockImplementation(function (this: Document, type, listener, options) { - if (listener != null && isLift(type)) { - live.add(listener); - } - EventTarget.prototype.addEventListener.call( - this, - type, - listener, - options, - ); - }); - const removeSpy = vi - .spyOn(document, 'removeEventListener') - .mockImplementation(function (this: Document, type, listener, options) { - if (listener != null && isLift(type)) { - live.delete(listener); - } - EventTarget.prototype.removeEventListener.call( - this, - type, - listener, - options, - ); - }); - try { - const onOpenChange = vi.fn<(isOpen: boolean) => void>(); - const {trigger, outside} = renderMenu(onOpenChange); - const atRest = new Set(live); - fireEvent.contextMenu(trigger, {clientX: 50, clientY: 15, detail: 1}); - expect(onOpenChange).toHaveBeenLastCalledWith(true); - // Open, with no press in flight: nothing follows a lift. - expect(live).toEqual(atRest); - fireEvent.pointerDown(outside, finger(5)); - expect(live.size).toBeGreaterThan(atRest.size); - lift(outside, 5); - expect(onOpenChange).toHaveBeenLastCalledWith(false); - expect(live).toEqual(atRest); - } finally { - addSpy.mockRestore(); - removeSpy.mockRestore(); - } - }); - - it('a tap outside closes the menu; a scroll that starts outside leaves it open', () => { - const onOpenChange = vi.fn<(isOpen: boolean) => void>(); - const {trigger, outside} = renderMenu(onOpenChange); - fireEvent.contextMenu(trigger, {clientX: 50, clientY: 15, detail: 1}); - // The browser takes this finger for a scroll and cancels it. - fireEvent.pointerDown(outside, finger(3)); - fireEvent.pointerCancel(outside, finger(3)); - expect(onOpenChange).toHaveBeenLastCalledWith(true); - fireEvent.pointerDown(outside, finger(4)); - lift(outside, 4); - expect(onOpenChange).toHaveBeenLastCalledWith(false); - }); - - it('a mouse press outside closes the menu', () => { - const onOpenChange = vi.fn<(isOpen: boolean) => void>(); - const {trigger, outside} = renderMenu(onOpenChange); - fireEvent.contextMenu(trigger, {clientX: 50, clientY: 15, detail: 1}); - fireEvent.pointerDown(outside, {pointerType: 'mouse', pointerId: 1}); - fireEvent.mouseDown(outside); - expect(onOpenChange).toHaveBeenLastCalledWith(false); - }); - - it('a pen closes the menu on a tap outside, as a finger does, and a cancelled pen press leaves it open', () => { - const onOpenChange = vi.fn<(isOpen: boolean) => void>(); - const {trigger, outside} = renderMenu(onOpenChange); - fireEvent.contextMenu(trigger, {clientX: 50, clientY: 15, detail: 1}); - const pen = (pointerId: number) => ({pointerType: 'pen', pointerId}); - // A pen press the browser takes for a scroll: the menu stays open. - fireEvent.pointerDown(outside, pen(7)); - fireEvent.mouseDown(outside); - fireEvent.pointerCancel(outside, pen(7)); - expect(onOpenChange).toHaveBeenLastCalledWith(true); - // A pen tap: the press alone keeps the menu, its lift closes it. - fireEvent.pointerDown(outside, pen(8)); - expect(onOpenChange).toHaveBeenLastCalledWith(true); - fireEvent.pointerUp(outside, pen(8)); - expect(onOpenChange).toHaveBeenLastCalledWith(false); - }); - - it('closes on the presses a mousedown listener missed: a control that cancels its pointerdown, a finger drag on a surface that does not pan, and a second finger, on its lift', () => { - const onOpenChange = vi.fn<(isOpen: boolean) => void>(); - render( - <> - -
Right-click me
- - -
- , - ); - const trigger = screen.getByTestId('ctx'); - fireEvent.contextMenu(trigger, {clientX: 50, clientY: 15, detail: 1}); - // The control cancels the press, so no compatibility mousedown follows. - fireEvent.pointerDown(screen.getByRole('button', {name: 'Step'}), { - pointerType: 'mouse', - pointerId: 1, - }); - expect(onOpenChange).toHaveBeenLastCalledWith(false); - fireEvent.contextMenu(trigger, {clientX: 50, clientY: 15, detail: 1}); - expect(onOpenChange).toHaveBeenLastCalledWith(true); - // A drag the browser does not pan: never cancelled, and no compatibility - // mouse events; the lift closes the menu. - const track = screen.getByTestId('track'); - fireEvent.pointerDown(track, finger(9)); - fireEvent.pointerMove(track, finger(9)); - expect(onOpenChange).toHaveBeenLastCalledWith(true); - fireEvent.pointerUp(track, finger(9)); - expect(onOpenChange).toHaveBeenLastCalledWith(false); - // A second finger's tap, which sends no compatibility mouse events. - fireEvent.contextMenu(trigger, {clientX: 50, clientY: 15, detail: 1}); - expect(onOpenChange).toHaveBeenLastCalledWith(true); - fireEvent.pointerDown(track, {...finger(10), isPrimary: false}); - fireEvent.pointerUp(track, {...finger(10), isPrimary: false}); - expect(onOpenChange).toHaveBeenLastCalledWith(false); - }); - - it('closes on a press whose control stops it, as a resize handle does, for a mouse and for a finger', () => { - const onOpenChange = vi.fn<(isOpen: boolean) => void>(); - const stop = (event: PointerEvent) => event.stopPropagation(); - render( - <> - -
Right-click me
- -
- , - ); - const trigger = screen.getByTestId('ctx'); - const handle = screen.getByTestId('handle'); - fireEvent.contextMenu(trigger, {clientX: 50, clientY: 15, detail: 1}); - fireEvent.pointerDown(handle, {pointerType: 'mouse', pointerId: 1}); - expect(onOpenChange).toHaveBeenLastCalledWith(false); - fireEvent.contextMenu(trigger, {clientX: 50, clientY: 15, detail: 1}); - expect(onOpenChange).toHaveBeenLastCalledWith(true); - fireEvent.pointerDown(handle, finger(11)); - expect(onOpenChange).toHaveBeenLastCalledWith(true); - fireEvent.pointerUp(handle, finger(11)); - expect(onOpenChange).toHaveBeenLastCalledWith(false); - }); - - it('a tap outside closes the menu even when the host re-renders mid-tap with an inline onOpenChange', () => { - const calls: boolean[] = []; - // A new inline handler on every render gives the menu's close a new - // identity each time, as a host that re-renders often does. - const ui = () => ( - <> - calls.push(isOpen)}> -
Long-press me
- - - - ); - const {rerender} = render(ui()); - fireEvent.contextMenu(screen.getByTestId('ctx'), { - clientX: 50, - clientY: 15, - detail: 1, - }); - expect(calls.at(-1)).toBe(true); - const outside = screen.getByRole('button', {name: 'Outside'}); - fireEvent.pointerDown(outside, finger(5)); - // The host re-renders between the finger's press and its lift. - rerender(ui()); - lift(outside, 5); - expect(calls.at(-1)).toBe(false); - }); -}); - describe('ContextMenu items', () => { it('renders items with labels', () => { render( @@ -1723,3 +1410,145 @@ describe('ContextMenu inline trigger (triggerAs)', () => { expect(screen.getByTestId('area').tagName).toBe('DIV'); }); }); + +describe('ContextMenu outside dismissal', () => { + const finger = (pointerId: number) => ({pointerType: 'touch', pointerId}); + + // Holds a finger on the trigger until the long-press opens the menu. Only + // the long-press delay is faked. + function longPress(trigger: HTMLElement, pointerId: number) { + vi.useFakeTimers({toFake: ['setTimeout', 'clearTimeout']}); + try { + fireEvent.pointerDown(trigger, finger(pointerId)); + fireEvent.touchStart(trigger, {touches: [{clientX: 20, clientY: 20}]}); + act(() => { + vi.advanceTimersByTime(500); + }); + } finally { + vi.useRealTimers(); + } + } + + // What a browser sends after a touch: the lift, then compatibility mouse + // events aimed at the element the finger held. + function lift(target: HTMLElement, pointerId: number) { + fireEvent.pointerUp(target, finger(pointerId)); + fireEvent.touchEnd(target); + fireEvent.mouseDown(target); + fireEvent.mouseUp(target); + fireEvent.click(target); + } + + function renderMenu(onOpenChange: (isOpen: boolean) => void) { + render( + <> + +
Long-press me
+ + + , + ); + return { + trigger: screen.getByTestId('ctx'), + outside: screen.getByRole('button', {name: 'Outside'}), + }; + } + + it('the long press that opened the menu does not close it, mid-hold or as it lifts; the next press outside does', () => { + const onOpenChange = vi.fn<(isOpen: boolean) => void>(); + const {trigger, outside} = renderMenu(onOpenChange); + longPress(trigger, 1); + expect(onOpenChange).toHaveBeenLastCalledWith(true); + // iOS: a compatibility mousedown while the finger is still down. + fireEvent.mouseDown(trigger); + expect(onOpenChange).toHaveBeenLastCalledWith(true); + // Chromium: the compatibility events that follow the lift. + lift(trigger, 1); + expect(onOpenChange).not.toHaveBeenCalledWith(false); + // A new press is a new gesture: a mouse press outside closes the menu. + fireEvent.pointerDown(outside, {pointerType: 'mouse', pointerId: 9}); + fireEvent.mouseDown(outside); + expect(onOpenChange).toHaveBeenLastCalledWith(false); + }); + + it('a finger long press that opens the menu through a pointer contextmenu (Chromium) does not close it as it lifts', () => { + const onOpenChange = vi.fn<(isOpen: boolean) => void>(); + const {trigger, outside} = renderMenu(onOpenChange); + fireEvent.pointerDown(trigger, finger(8)); + const contextmenu = createEvent.contextMenu(trigger, { + clientX: 20, + clientY: 15, + detail: 1, + }); + Object.defineProperty(contextmenu, 'pointerType', {value: 'touch'}); + fireEvent(trigger, contextmenu); + expect(onOpenChange).toHaveBeenLastCalledWith(true); + lift(trigger, 8); + expect(onOpenChange).not.toHaveBeenCalledWith(false); + // A right-click is not a finger: its gesture's next mousedown, outside, + // closes the menu. + fireEvent.pointerDown(trigger, {pointerType: 'mouse', pointerId: 1}); + fireEvent.contextMenu(trigger, {clientX: 20, clientY: 15, detail: 1}); + fireEvent.mouseDown(outside); + expect(onOpenChange).toHaveBeenLastCalledWith(false); + }); + + it('a long press that moves the open menu keeps it open as it lifts', () => { + const onOpenChange = vi.fn<(isOpen: boolean) => void>(); + const {trigger} = renderMenu(onOpenChange); + fireEvent.contextMenu(trigger, {clientX: 50, clientY: 15, detail: 1}); + expect(onOpenChange).toHaveBeenLastCalledWith(true); + longPress(trigger, 2); + lift(trigger, 2); + expect(onOpenChange).not.toHaveBeenCalledWith(false); + }); + + it('a tap outside closes the menu through its compatibility mousedown; a scroll that starts outside leaves it open', () => { + const onOpenChange = vi.fn<(isOpen: boolean) => void>(); + const {trigger, outside} = renderMenu(onOpenChange); + fireEvent.contextMenu(trigger, {clientX: 50, clientY: 15, detail: 1}); + // The browser takes this finger for a scroll: no compatibility events. + fireEvent.pointerDown(outside, finger(3)); + fireEvent.pointerCancel(outside, finger(3)); + expect(onOpenChange).toHaveBeenLastCalledWith(true); + fireEvent.pointerDown(outside, finger(4)); + lift(outside, 4); + expect(onOpenChange).toHaveBeenLastCalledWith(false); + }); + + it('a mouse press or a pen press outside closes the menu at the press', () => { + for (const pointerType of ['mouse', 'pen']) { + const onOpenChange = vi.fn<(isOpen: boolean) => void>(); + const {trigger, outside} = renderMenu(onOpenChange); + fireEvent.contextMenu(trigger, {clientX: 50, clientY: 15, detail: 1}); + fireEvent.pointerDown(outside, {pointerType, pointerId: 7}); + fireEvent.mouseDown(outside); + expect(onOpenChange).toHaveBeenLastCalledWith(false); + cleanup(); + } + }); + + it('presses that send no mousedown leave the menu open, as before: a cancelled pointerdown, a finger drag, a second finger', () => { + const onOpenChange = vi.fn<(isOpen: boolean) => void>(); + const {trigger, outside} = renderMenu(onOpenChange); + fireEvent.contextMenu(trigger, {clientX: 50, clientY: 15, detail: 1}); + // A control that cancels its pointerdown (a NumberInput stepper, a + // resize handle): the browser sends no mousedown. + fireEvent.pointerDown(outside, {pointerType: 'mouse', pointerId: 1}); + fireEvent.pointerUp(outside, {pointerType: 'mouse', pointerId: 1}); + // A finger drag on a surface that does not pan (a Slider), lifted. + fireEvent.pointerDown(outside, finger(5)); + fireEvent.pointerMove(outside, finger(5)); + fireEvent.pointerUp(outside, finger(5)); + // A second finger's tap. + fireEvent.pointerDown(outside, finger(6)); + fireEvent.pointerDown(outside, finger(7)); + fireEvent.pointerUp(outside, finger(7)); + fireEvent.pointerCancel(outside, finger(6)); + expect(onOpenChange).toHaveBeenLastCalledWith(true); + expect(onOpenChange).not.toHaveBeenCalledWith(false); + }); +}); diff --git a/packages/core/src/ContextMenu/ContextMenu.tsx b/packages/core/src/ContextMenu/ContextMenu.tsx index 2dc0af0b1d06d..2e257bb130568 100644 --- a/packages/core/src/ContextMenu/ContextMenu.tsx +++ b/packages/core/src/ContextMenu/ContextMenu.tsx @@ -52,6 +52,7 @@ import {Icon} from '../Icon'; import {useLayer} from '../Layer/useLayer'; import {LayerDepthProvider} from '../Layer/LayerDepthContext'; import {useLayerDismissal} from '../Layer/useLayerDismissal'; +import {currentGesture} from '../Layer/gestureCounter'; import {MenuBottomSheetActionList} from '../DropdownMenu/MenuBottomSheetActionList'; import {renderDropdownItems} from '../DropdownMenu/renderDropdownItems'; import { @@ -354,11 +355,11 @@ export function ContextMenu({ // Element focused before the menu opened, restored when it closes so focus // does not fall to after Escape or outside-click dismissal. const triggerFocusRef = useRef(null); - // Fingers and pens that went down outside the open menu; each closes it if - // it lifts without the browser cancelling it (a tap, not a scroll). - const outsidePressesRef = useRef(new Set()); - // The latest closeMenu, for the outside-press listeners (see below). - const closeMenuRef = useRef<() => void>(() => {}); + // The gesture of the finger press that opened the menu (`Layer/ + // gestureCounter`), null when a mouse or the keyboard opened it: the + // browser's compatibility mouse events for that same press are not presses + // outside. + const openingGestureRef = useRef(null); const [isOpen, setIsOpen] = useState(false); const [submenuPath, setSubmenuPath] = useState([]); @@ -463,138 +464,43 @@ export function ContextMenu({ ), }); - // Dismiss on a press outside the menu. The menu is popover="manual" (not - // "auto") because native light dismiss reads the release of the opening - // right-click as a dismissal, so the press is read here, from pointer - // events: a touch's compatibility mouse events (the mousedown Chromium sends - // as a long press lifts, or one sent while it is still held) - // have no pointer event of their own, so the press that opened the menu - // never closes it. A mouse press outside closes the menu at once; a finger - // or a pen closes it on a tap, lifted rather than cancelled, so a scroll - // that starts outside leaves it open, as before. A pen goes with the finger - // because the menu press model (useMenuPress) groups them. They listen in - // the capture phase, so a control that stops the press (a resize handle) - // still closes the menu. - // - // Inside a shadow root, even a closed or nested one, the document sees a - // press on the menu as a press on a shadow host, and only the menu's own - // root sees where it landed. So each root from the document down to the - // menu's own listens too: capture reaches the outer ones first, and each - // defers a press on the host of the next root down to that root. A press - // on such a host itself reaches no deeper root, so the host judges it, - // whatever the host's own listeners do with it. Each press is judged once. - // - // The listeners read the latest `closeMenu` through a ref, so they are - // installed once per open: a caller's inline `onOpenChange` gives - // `closeMenu` a new identity on every render, and re-installing them would - // forget a press recorded between its down and its lift. - closeMenuRef.current = closeMenu; + // Dismiss on any click outside the menu. We use popover="manual" (not + // "auto") because the native light-dismiss treats the mouseup from the + // opening right-click as a dismiss event. Handling it ourselves via + // mousedown avoids that race. A touch's compatibility mousedown for the + // press that opened the menu (sent mid-hold by iOS, or as a long press + // lifts by Chromium) belongs to that press's gesture, so it is not read as + // a press outside; any later press is a new gesture and closes as before. useEffect(() => { if (!isOpen || usesBottomSheet) { return; } - const outsidePresses = outsidePressesRef.current; - // A finger's or a pen's lift is followed only while such a press is live: - // the document's pointerup and pointercancel listeners come on with the - // first one and off once none is left (`architecture:react-component-runtime` - // INV5), so an open menu with no press in flight holds none. - let isFollowingLifts = false; - const stopFollowingLifts = () => { - if (isFollowingLifts) { - isFollowingLifts = false; - document.removeEventListener('pointerup', handlePointerUp, true); - document.removeEventListener( - 'pointercancel', - handlePointerCancel, - true, - ); - } - }; - function handlePointerUp(e: PointerEvent) { - const wasOutside = outsidePresses.delete(e.pointerId); - if (outsidePresses.size === 0) { - stopFollowingLifts(); - } - if (wasOutside) { - closeMenuRef.current(); - } - } - function handlePointerCancel(e: PointerEvent) { - outsidePresses.delete(e.pointerId); - if (outsidePresses.size === 0) { - stopFollowingLifts(); - } - } - const pressOutside = (e: PointerEvent) => { - if (e.pointerType === 'touch' || e.pointerType === 'pen') { - outsidePresses.add(e.pointerId); - if (!isFollowingLifts) { - isFollowingLifts = true; - document.addEventListener('pointerup', handlePointerUp, true); - document.addEventListener('pointercancel', handlePointerCancel, true); - } - return; - } - closeMenuRef.current(); - }; - // The menu's own root, each root around it, and the document, each with - // the host of the root below it (none for the menu's own). - const levels: [Document | ShadowRoot, Element | null][] = []; - let tree: Node | undefined = listRef.current?.getRootNode(); - let innerHost: Element | null = null; - while (tree instanceof ShadowRoot) { - levels.push([tree, innerHost]); - innerHost = tree.host; - tree = innerHost.getRootNode(); - } - levels.push([document, innerHost]); + // Inside a shadow root, even a closed or nested one, the document sees a + // press on the menu as a press on an outer shadow host, so the menu's own + // tree judges its presses first and the document skips those. + const tree = listRef.current?.getRootNode(); + const shadow = tree instanceof ShadowRoot ? tree : null; const judged = new WeakSet(); - const judgePress = (e: Event) => { + const handleClickOutside = (e: Event) => { + const menu = listRef.current; if (judged.has(e)) { return; } judged.add(e); - const menu = listRef.current; - if (menu != null && !menu.contains(e.target as Node)) { - pressOutside(e as PointerEvent); + if (currentGesture() === openingGestureRef.current) { + return; } - }; - const pressListeners = levels.map(([root, hostBelow]) => { - const handlePointerDown = (e: Event) => { - if (hostBelow == null || e.target !== hostBelow) { - judgePress(e); - } - }; - return [root, handlePointerDown] as const; - }); - // A press on a host itself, which no deeper root sees. - const handleUnjudgedPointerDown = (e: Event) => { - if (!judged.has(e)) { - judged.add(e); - pressOutside(e as PointerEvent); + if (menu && !menu.contains(e.target as Node)) { + closeMenu(); } }; - for (const [, hostBelow] of levels) { - hostBelow?.addEventListener('pointerdown', handleUnjudgedPointerDown); - } - for (const [root, handlePointerDown] of pressListeners) { - root.addEventListener('pointerdown', handlePointerDown, true); - } + document.addEventListener('mousedown', handleClickOutside); + shadow?.addEventListener('mousedown', handleClickOutside); return () => { - outsidePresses.clear(); - document.removeEventListener('pointerup', handlePointerUp, true); - document.removeEventListener('pointercancel', handlePointerCancel, true); - for (const [, hostBelow] of levels) { - hostBelow?.removeEventListener( - 'pointerdown', - handleUnjudgedPointerDown, - ); - } - for (const [root, handlePointerDown] of pressListeners) { - root.removeEventListener('pointerdown', handlePointerDown, true); - } + document.removeEventListener('mousedown', handleClickOutside); + shadow?.removeEventListener('mousedown', handleClickOutside); }; - }, [isOpen, listRef, usesBottomSheet]); + }, [isOpen, closeMenu, listRef, usesBottomSheet]); const listKeyDown = useCallback( (e: React.KeyboardEvent) => { @@ -660,9 +566,6 @@ export function ContextMenu({ document.activeElement instanceof HTMLElement ? document.activeElement : focusEl; - // A finger held on the trigger while the menu was open is now the long - // press that moved it, not a tap outside it. - outsidePressesRef.current.clear(); if (usesBottomSheet) { updateOpenState(true); } else { @@ -698,6 +601,14 @@ export function ContextMenu({ const localX = isKeyboardInvoked || !rect ? 0 : e.clientX - rect.left; const localY = isKeyboardInvoked || !rect ? (rect?.height ?? 0) : e.clientY - rect.top; + // A finger's long press reaches here as a pointer `contextmenu` where + // the browser sends one (Chromium); its compatibility mousedown then + // belongs to the opening press. A mouse or the keyboard opens without + // one, so every later mousedown is a press outside. + openingGestureRef.current = + (e.nativeEvent as Partial).pointerType === 'touch' + ? currentGesture() + : null; openAtLocalPoint(localX, localY, e.currentTarget as HTMLElement); }, [isDisabled, openAtLocalPoint, getAnchorBaseRect], @@ -719,6 +630,9 @@ export function ContextMenu({ onLongPress: useCallback( (point: {x: number; y: number}) => { const rect = getAnchorBaseRect(); + // The finger is still down: the compatibility mousedown iOS sends + // mid-hold, or Chromium as it lifts, belongs to this press. + openingGestureRef.current = currentGesture(); openAtLocalPoint( rect ? point.x - rect.left : point.x, rect ? point.y - rect.top : point.y,