Repository navigation
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
meta-cla
Bot
added
the
CLA Signed
|
AI review status for this pull request.
|
PR Analysis Report📚 Storybook Preview🧪 Sandbox PreviewModified ComponentsContextMenu (@astryxdesign/core) · View in Storybook
Bundle Size Summary
Accessibility AuditStatus: No accessibility violations detected. Visual Regression46 of 494 shot(s) changed. A change here is a question, not a failure: check whether the after is the
and 26 more. Generated by PR Enrichment workflow | Storybook | Sandbox | View full report |
![astracat-bot[bot]](https://avatars.githubusercontent.com/in/4796231?s=60&v=4){kind=small}
There was a problem hiding this comment.
This fix looks ready to approve. Moving outside dismissal from mousedown to pointer events stops a touch's compatibility mouse events from closing the menu its own long press opened, while keeping the existing outcomes that a mouse press outside closes at once, a finger tap outside closes, and a scroll starting outside does not.
Evidence checked in review:
- In real Chromium with synthesized touch on the Default story, the previous behavior closed the menu right after the finger lifted, this change leaves it open through the compatibility click, and a later tap outside still closes it.
- The full ContextMenu unit file passes, including the new long-press, tap/scroll, and mouse outside-dismissal cases, and the core typecheck is clean.
One small non-blocking follow-up: the layer-runtime dismissal inventory still describes ContextMenu as using a local outside mousedown listener, so that one word could be refreshed to match the new pointer-press mechanism whenever convenient.
WebKit was not run here, so the iOS timing rests on the removal of the mousedown listener itself rather than an iOS run.
Thank you for the focused restoration and the real-engine regression coverage.
[Automated review]
cixzhang
left a comment
cixzhang
left a comment
There was a problem hiding this comment.
CodeBunny approved at this head; CI green. Approved.
Head branch was pushed to by a user without write access
67b9966 to
b0f51c5
Compare
There was a problem hiding this comment.
Thanks for the focused fix: in real Chromium the long-pressed menu now stays open through the lift, and taps, scrolls, and mouse or pen presses outside behave as before. One regression needs fixing first: the outside-dismissal effect depends on closeMenu, whose identity follows onOpenChange, and its cleanup clears outsideTouchesRef, so when a caller passes an inline onOpenChange (as LazyTableContextMenu does) and re-renders between a finger's pointerdown and pointerup, the tap outside no longer closes the menu. Please keep the recorded touches across re-renders (for example, read closeMenu through a ref so the effect depends only on isOpen and usesBottomSheet) and add a test with an inline onOpenChange and a re-render mid-tap.
[Automated review]
| document.addEventListener('pointercancel', handlePointerCancel); | ||
| return () => { | ||
| document.removeEventListener('mousedown', handleClickOutside); | ||
| outsideTouches.clear(); |
There was a problem hiding this comment.
This cleanup runs whenever closeMenu changes identity, not only when the menu closes, and closeMenu follows the caller's onOpenChange through updateOpenState. With an inline onOpenChange (as LazyTableContextMenu passes), a re-render between a finger's pointerdown and pointerup (state set on touchstart, a timer) clears the finger recorded here, so the tap outside no longer closes the menu; before this change, the tap's compatibility mousedown closed it. Reading closeMenu through a ref, so the effect depends only on isOpen and usesBottomSheet, would make this cleanup a true close. A regression case with an inline onOpenChange and a re-render mid-tap would lock it in.
There was a problem hiding this comment.
Fixed in 7e78c3f. The listeners now read the latest closeMenu through a ref, and the effect depends only on isOpen, usesBottomSheet and the stable menu ref, so its cleanup runs only when the menu closes. A new case re-renders with a new inline onOpenChange between a finger's press and lift outside, and checks that the tap still closes the menu. It failed on the previous head and passes now.
There was a problem hiding this comment.
The fix itself checks out: in real Chromium a long-pressed ContextMenu now stays open through the finger's lift, and taps, scrolls, and mouse or pen presses outside still behave as before. Approval now waits on a maintainer decision, because the new FR9 adds a normative outside-press rule to the draft ContextMenu spec that no approved spec states yet, so the component owner needs to approve or adjust it, including whether a pen press should close the menu at once like a mouse, since the menu press model's current spec treats a pen like a finger. Separately, the changeset describes the iOS mid-hold case as fixed without an iOS Safari run; please either add that evidence or limit the note to the Chromium lift the tests prove.
[Automated review]
| '@astryxdesign/core': patch | ||
| --- | ||
|
|
||
| [fix] A ContextMenu opened by a long press stays open: the compatibility `mousedown` a browser sends for the touch (mid-hold on iOS, as the finger lifts in Chromium) no longer reads as a press outside. Outside dismissal follows pointer events: a mouse or pen press outside closes the menu at once, and a finger's tap outside closes it, while a scroll that starts outside still leaves it open. |
There was a problem hiding this comment.
This states the iOS mid-hold case as fixed, but only Chromium was run, and the browser-support spec does not let Chromium stand in for an iOS Safari claim. Please either attach an iOS Safari long-press check or keep this note to the Chromium lift that the tests prove.
There was a problem hiding this comment.
Limited in a065c3c: the changeset now describes the Chromium lift the tests prove (the compatibility mousedown sent as the finger lifts) and makes no iOS Safari claim. The PR body keeps the iOS report as unverified.
| | 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. Every invocation path (right-click, Shift+F10, long-press) and the `context-menu` target's placement are unchanged. | `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 | | ||
| | 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. A key pressed in a control the menu hosts beside its rows stays with it, as in `component:DropdownMenu` FR12. | Proposed in this change; `ContextMenu.test.tsx` keyboard case; shared `useListFocus` and `useTypeahead` suites | Proposed; verified in jsdom, pending owner review | | ||
| | FR9 | A press outside the Pointer menu surface MUST close it: a mouse or pen press at once, a finger on a tap (lifted, not cancelled), while a scroll that starts outside leaves it open. The touch's compatibility mouse events (a `mousedown` sent mid-hold or as the finger lifts) MUST NOT close it, so the long press that opens the menu never closes it. | `architecture:interaction-modality` INV4; tests | Proposed; pending owner review | |
There was a problem hiding this comment.
FR9 is a new normative rule in a draft record, so it needs the component owner's approval before this can be approved. One point for that decision: FR9 closes the menu at once for a pen, like a mouse, while the menu press model's current spec groups a pen with a finger.
There was a problem hiding this comment.
Adjusted in a065c3c: a pen now goes with the finger, as module:DropdownMenu/useMenuPress groups them. A pen press outside closes the menu when it lifts uncancelled, a pen press the browser takes for a scroll leaves it open, and a mouse still closes it at once. FR9 says so, and a new unit case covers the pen (it fails on main, where a pen closes the menu at once). FR9 itself is the component owner's to approve.
7e78c3f to
e647055
Compare
There was a problem hiding this comment.
The long-press fix itself checks out: in real Chromium the menu now stays open as the finger lifts (it closes on main), and mouse presses, finger taps and drags outside behave as before. Approval waits on a maintainer decision, because FR9 adds a normative outside-press rule to the draft ContextMenu spec that no approved spec states yet; the component owner needs to approve or adjust it, including whether a pen press outside should wait for its lift (as FR9 now says) or keep closing the menu at press as it does today. Two small documentation fixes are also needed:
docs/architecture/layer-runtime.mdstill says ContextMenu uses a local outsidemousedownlistener.- The Changeset and description should note that a press on a control that cancels its
pointerdownnow closes the menu, which it did not before.
[Automated review]
a065c3c to
a14fd1f
Compare
There was a problem hiding this comment.
The long-press fix checks out: in real Chromium the menu now stays open as the finger lifts (it closes on main), while mouse presses, finger taps and pans outside end as before. Approval waits on the ContextMenu owner approving or adjusting the new FR9 outside-press rule, which also makes a pen press close the menu on its lift rather than at press, and makes presses on controls that cancel pointerdown and drags on touch-action: none surfaces close it. Please also:
- Name those two new dismissals in the Changeset and description, or guard them if the owner keeps
main's outcome. - Update
docs/architecture/layer-runtime.md, which still says ContextMenu uses an outsidemousedownlistener. - Rebase onto
main, where the draft ContextMenu record already uses FR9 for shared-stack Escape, and give the new rule the next free number.
[Automated review]
a14fd1f to
34397ae
Compare
There was a problem hiding this comment.
The long-press fix checks out: in real Chromium the menu now stays open as the finger lifts (it closes on main), while mouse presses, finger taps and pans outside end as before. Approval waits on the ContextMenu owner approving or adjusting the new FR10 outside-press rule, which also makes a pen press close the menu on its lift rather than at press, and makes presses on controls that cancel pointerdown (such as NumberInput's steppers) and drags on touch-action: none surfaces (such as Slider) close it. Please also:
- Name those two new dismissals in the Changeset and description, or guard them if the owner keeps
main's outcome. - Update
docs/architecture/layer-runtime.md, which still says ContextMenu uses an outsidemousedownlistener.
[Automated review]
| | 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 press outside the Pointer menu surface MUST close it: 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 | |
There was a problem hiding this comment.
FR10 is a new normative rule in a draft record, so the ContextMenu owner needs to approve it before this can be approved. Beyond the pen now waiting for its lift (on main a pen press outside closes the menu at press), its “A press outside … MUST close it” and “(lifted, not cancelled)” also make a press on a control that cancels pointerdown, a finger drag that starts on a touch-action: none surface and a second finger's tap outside close the menu, which main does not do.
There was a problem hiding this comment.
Agreed. As of 915edf9, FR10 says a press outside closes the menu "whatever the pressed element does with the event". The second finger's tap is named in the changeset and description alongside the cancelled press and the unpanned drag, and a case covers all three. The owner decision on FR10 stays open. docs/architecture/layer-runtime.md now lists the pointer listeners instead of the mousedown one.
| '@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. A pen now waits for its lift, as the menu press model treats a pen like a finger. |
There was a problem hiding this comment.
This names the pen change but not the two other new dismissals: in Chromium a mouse press on a control that cancels its pointerdown (NumberInput's steppers, Slider) and a finger drag on a touch-action: none surface now close the menu, where main leaves it open. Please name them here and in the description, or guard them if the owner keeps main's outcomes.
There was a problem hiding this comment.
Named in 915edf9. The changeset, the description and the commit message now list the presses that changed:
- A pen closes the menu on its lift rather than at the press.
- A press on a control that cancels its
pointerdown(a NumberInput stepper) now closes it. - So does a finger drag on a
touch-action: nonesurface (a Slider), on the lift. - So does a second finger's tap, on the lift.
A new case pins the last three, and it fails on main. I kept these outcomes rather than guarding them: a press outside is a press outside, whatever the pressed element does with the event. FR10 now says so, for the owner to accept or adjust.
34397ae to
53fa96a
Compare
There was a problem hiding this comment.
The long-press fix checks out: in real Chromium the menu now stays open as the finger lifts (it closes on main), while mouse presses, finger taps and pans outside end as before. Approval waits on the ContextMenu owner approving or adjusting the new draft FR10 outside-press rule, which also makes a pen press close the menu on its lift rather than at press, and makes presses on controls that cancel pointerdown (such as NumberInput's steppers) and drags on touch-action: none surfaces (such as Slider) close it. Please also:
- Name those two new dismissals in the Changeset, the description and the commit message (which still says a pen closes the menu at once and names FR9), or guard them if the owner keeps
main's outcome. - Update
docs/architecture/layer-runtime.md, which still says ContextMenu uses an outsidemousedownlistener.
[Automated review]
53fa96a to
e98dbc2
Compare
github-actions
Bot
added
the
needs:spec-owner-review
e98dbc2 to
915edf9
Compare
There was a problem hiding this comment.
The long-press fix works: in real Chromium the menu now stays open as the finger lifts (it closes on main), and mouse presses and finger taps outside still close it. Approval waits on the ContextMenu owner approving or adjusting the new draft FR10, which also changes how pen presses, presses on controls that cancel pointerdown, unpanned drags and second-finger taps dismiss the menu. Separately, FR10 and the Changeset say an outside press closes the menu whatever the pressed element does, but the three document listeners run in the bubble phase, so a press on a ResizeHandle or a Table column-resize handle (both stop their pointerdown) still leaves it open; please register them in the capture phase, or narrow FR10 and the Changeset if the owner keeps that outcome.
[Automated review]
| document.addEventListener('pointerdown', handlePointerDown); | ||
| document.addEventListener('pointerup', handlePointerUp); | ||
| document.addEventListener('pointercancel', handlePointerCancel); |
There was a problem hiding this comment.
These listeners run in the bubble phase, so a press whose pointerdown an outside control stops never reaches them: ResizeHandle (ResizeHandle.tsx 516–524) and Table's column-resize handle (useTableColumnResize.tsx 403–406) call stopPropagation(), and in Chromium a mouse press on a ResizeHandle leaves the menu open. FR10 says the menu closes "whatever the pressed element does with the event"; passing true (capture) to these three calls and their removals closes it in both cases and keeps the ContextMenu suite green. Otherwise FR10 and the Changeset need narrower wording.
There was a problem hiding this comment.
Fixed in 34c2b5a. The three listeners and their removals now pass true (capture), so a control that stops the press still closes the menu, as FR10 says: ResizeHandle and Table's column-resize handle both call preventDefault() and stopPropagation() on pointerdown. The new case "closes on a press whose control stops it, as a resize handle does, for a mouse and for a finger" fails with bubble-phase listeners. The changeset and description name the resize handle among the changed presses.
| | 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 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 | |
There was a problem hiding this comment.
FR10 is a new normative rule in a draft record, so the ContextMenu owner needs to approve it before this can be approved: it makes a pen close the menu on its lift (main: at the press) and makes presses on controls that cancel pointerdown, unpanned finger drags and second-finger taps close it (main: open). Its "whatever the pressed element does with the event" clause also covers presses that a control stops, which the implementation does not close yet.
There was a problem hiding this comment.
As of 34c2b5a the implementation matches that clause, since the listeners capture. Every press FR10 now closes on that main leaves open is named in the changeset and description: a pen's press (on its lift), a cancelled or stopped pointerdown, an unpanned finger drag and a second finger's tap. The owner decision on FR10 stays open.
|
/approve-spec 34c2b5a |
github-actions
Bot
removed
the
needs:spec-owner-review
There was a problem hiding this comment.
The long-press fix works: in real Chromium the menu now stays open as the finger lifts (it closes on main), and mouse presses and finger taps outside still close it. Approval needs the ContextMenu owner, as the maintainer for this record, to approve or narrow the new draft FR10, the governing spec for five changed outside presses: a pen now closes the menu on its lift, and presses on controls that cancel or stop pointerdown (NumberInput steppers, ResizeHandle, Table column resize), unpanned finger drags and second-finger taps now close it. Separately, #7303 proposes a different FR10 in the same record, so please name it in the description and give one of the two rules the next free number.
[Automated review]
| | 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 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 | |
There was a problem hiding this comment.
There was a problem hiding this comment.
Done in 0f5830a. The outside-press rule is now FR11 (contract row, verification row and the ownership paragraph's reference), leaving FR10 to #7303's open-focus rule. The description names #7303. If #7303 merges first, this one keeps FR11 and only rebases. The branch is also rebased onto main as one commit, whose message now says FR11.
34c2b5a to
0f5830a
Compare
github-actions
Bot
added
the
needs:spec-owner-review
|
/approve-spec 0f5830a |
github-actions
Bot
removed
the
needs:spec-owner-review
There was a problem hiding this comment.
The long-press fix works: in Chromium the menu now stays open as the finger lifts (it closes on main), and mouse presses and finger taps outside still close it. Approval waits on the ContextMenu owner, as the maintainer for this record, approving or narrowing the new draft FR11, the governing spec for five outside presses that now behave differently: a pen closes the menu on its lift, and presses on controls that cancel or stop pointerdown (NumberInput steppers, ResizeHandle, Table column resize), unpanned finger drags and second-finger taps now close it. Your /approve-spec clears the repository gate for the layer-runtime.md edit, but FR11 still reads "pending owner review". Separately, a line listing the search terms behind the #7303 overlap check would complete the description.
[Automated review]
| | 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 | |
There was a problem hiding this comment.
This row is the decision the ContextMenu owner still has to make: approving FR11 as written keeps five outcomes that differ from main (a pen closing on its lift; presses that cancel or stop pointerdown, unpanned finger drags and second-finger taps closing the menu). The repository gate is green from the author's /approve-spec, but the row still reads "Proposed; pending owner review".
There was a problem hiding this comment.
The description now lists the overlap search: open PRs and records for component:ContextMenu, its spec and source, and layer-runtime.md, plus the terms "outside press", "light dismiss", "long press" and "pointerdown". Only #7303 overlaps. #7677, #7358 and #6505 touch the same files for unrelated reasons: forced-colors edges, a translator swap, and a table re-pad. On the row: FR11 stays "Proposed; pending owner review" until the ContextMenu owner approves or narrows it. The five changed outside presses are listed in the changeset and the description for that decision.
|
/revoke-spec 0f5830a |
github-actions
Bot
added
the
needs:spec-owner-review
|
Four older review threads, addressed in this branch's commit (0f5830a):
Resolving these four. |
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.
0f5830a to
a5784a5
Compare
There was a problem hiding this comment.
The long-press fix works: in Chromium the menu now stays open as the finger lifts (it closes on main), while mouse presses, finger taps and pans outside end as before. Approval waits on the ContextMenu owner, as the maintainer for this record, approving or narrowing draft FR11, which also changes five outside presses: a pen closes the menu on its lift, and presses on controls that cancel or stop pointerdown (NumberInput steppers, ResizeHandle, Table column resize), unpanned finger drags on a Slider and second-finger taps now close it. This head also needs an exact-head owner approval for the layer-runtime.md edit, since the earlier /approve-spec named a previous head and was revoked. Optionally, the description could name the reproduced case as a lift the browser reads as a tap, since a lift Chromium reads as its own long press does not close the menu on main either.
[Automated review]
User impact
On a phone, a
ContextMenua long press opens closes at once: as the finger lifts, Chromium sends the held element a compatibilitymousedown, read as a press outside.Problem and solution fit
mousedownoutside it, and a touch's compatibility events target the held trigger.Expected behavior and authority
architecture:interaction-modality(current) INV4: every modality has an operable path.family:overlay-dismissalleaves outside dismissal to components.component:ContextMenuis a draft; this PR proposes FR11 (#7303 proposes FR10, focus on open; #7840, FR12). Overlap search over open PRs and records (component:ContextMenu,layer-runtime.md; "outside press", "long press", "pointerdown"): #7303 and #7840 touch the same listener.Smallest restoration
The document
mousedownlistener becomespointerdown,pointerupandpointercancel, installed once per open (a re-render mid-tap keeps the finger), which compatibility events never reach. A mouse press outside closes at once; a finger or pen closes it on an uncancelled tap; a scroll from outside leaves it open. In a shadow root the roots and hosts around the menu listen too, asmaindoes formousedown. No API or default changes.Changed presses, named in the changeset: a pen closes it on its lift, not at the press; presses
mousedownnever saw now close it: on a control that cancels or stopspointerdown(a NumberInput stepper, a resize handle; the listeners capture), and on the lift a drag on atouch-action: nonesurface (a Slider) or a second finger's tap.architecture:layer-runtimenames the new listeners.Evidence
pointerup,touchend, then trustedmousedown,mouseupandclickon the trigger, and it closes.click; a tap or mouse click outside still closes it.mousedownmid-hold; its listener is gone, but WebKit was not run here.main; the tap, scroll and mouse guards pass on both. Cases for a mid-tap re-render, a pen's lift, a cancelled or stopped press, an unpanned drag and a second finger fail onmain.Writing rubric results
R1 narration: FR11 and the layer-runtime line are present tense.
R2 research: none.
R3 evidence: FR11 cites INV4; its verification row names the tests. The layer-runtime line names no change.
R4 rejected alternatives: none.
R5 change references: none.
R6 open questions: none.
Scope
Testing