Repository navigation
Conversation
…s is released A light-dismissible layer that shows from a contextmenu event captures that press's pointer, so its release lands on the layer and the browser's light dismiss leaves it open. A later press outside still dismisses it (spec:AST-003 FR11-FR13). Fixes #6947
|
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.
|
github-actions
Bot
added
the
needs:spec-owner-review
|
/approve-spec 97e2aaf |
github-actions
Bot
removed
the
needs:spec-owner-review
PR Analysis Report
Generated by PR Enrichment workflow | View current CI run |
![astracat-bot[bot]](https://avatars.githubusercontent.com/in/4796231?s=60&v=4){kind=small}
There was a problem hiding this comment.
Changes requested. The fix works: in Chromium and WebKit, a light-dismiss layer or a DropdownMenu opened from a right click now stays open after the button comes up, and the next press outside closes it.
- The changeset (
.changeset/layer-contextmenu-release.md) and the description say a held finger is fixed too, but every test and repro uses a mouse right-button press, so either narrow the wording to a right click or add a touch reproduction that fails onmain.
Optional: rename LayerContextMenuPress.a11y.chromium.spec.ts to .a11y.browser.spec.ts so the WebKit project runs it too. You could also shorten the @output line in the gestureCounter.ts header, which repeats the new function's doc comment.
[Automated review]
| '@astryxdesign/core': patch | ||
| --- | ||
|
|
||
| [fix] Layer: a light-dismissible layer that opens from a `contextmenu` event (a right click, or a held finger) stays open when that same press is released. Before, the release counted as a press outside, so a DropdownMenu opened through `isMenuOpen` from a right click closed at once. A later press outside still dismisses the layer. |
There was a problem hiding this comment.
"(a right click, or a held finger)" claims more than the tests and repros show: each one is a mouse right-button press, and iOS Safari never fires contextmenu from a long press. Suggest "(a right click)" here, unless you add a touch reproduction that fails on main.
| * @output Exports the current gesture identity, whether its click has run, | ||
| * and claimContextMenuPress, which keeps the release of the press that | ||
| * opened a layer from dismissing it |
There was a problem hiding this comment.
Non-blocking: this restates claimContextMenuPress's doc comment and makes the header longer. Shorter: @output Exports the gesture identity, whether its click has run, and claimContextMenuPress.
… too The changeset, story and claimContextMenuPress comment name the right click that the tests cover. The real-engine spec is a browser spec, so the WebKit project runs it as well.
There was a problem hiding this comment.
The fix works: in Chromium and WebKit, a light-dismiss layer or a DropdownMenu opened from a right click now stays open after the button comes up, and the next press outside closes it. The changeset now names only the right click that the tests cover.
Approval waits on one owner decision. This changes what a mouse right click does for every desktop user, so it passes as a defect fix against spec:AST-003 FR11–FR13 only with an engineering owner's ruling for the current head, and the only /approve-spec on this pull request names the previous commit. An engineering owner (the author counts) can record that ruling by commenting /approve-spec followed by the current head's full SHA.
[Automated review]
|
/approve-spec 229ae3c |
There was a problem hiding this comment.
Changes requested. The fix works: in Chromium and WebKit, a light-dismiss layer or a DropdownMenu opened from a right click now stays open after the button comes up, and the next press outside closes it. The spec change is approved for this head; two implementation points remain.
- Miss:
gestureCounter.ts:55-58addsdocumentlisteners forpointerup,pointercancelandcontextmenu(and rewirespointerdown) instead of subscribing through Core's shared input ownersubscribeInputinutils/inputStream.ts, whicharchitecture:react-component-runtimeINV16 and thepnpm review:preshared-owners check require. - Miss: desktop behavior change needs an owner decision: while the right button that opened a light-dismiss layer is still held, the pointer capture at
gestureCounter.ts:93stops hover and focus from following the mouse over the layer (in DropdownMenu, Duplicate no longer highlights or takes focus until the button comes up and the mouse moves again), so keep hover working during that press, for example by holding the capture only while the pointer is outside the layer, or cite an owner decision that covers it.
[Automated review]
| document.addEventListener('pointerdown', startPress, true); | ||
| document.addEventListener('pointerup', endPress, true); | ||
| document.addEventListener('pointercancel', endPress, true); | ||
| document.addEventListener('contextmenu', markContextMenu, true); |
There was a problem hiding this comment.
Miss: these document listeners sit beside Core's shared input owner. subscribeInput in packages/core/src/utils/inputStream.ts is the one entry for document-wide input listeners (architecture:react-component-runtime INV16), and pnpm review:pre flags all four lines (item i). Subscribing startPress, endPress and markContextMenu with subscribeInput(window, …, {first: true}) keeps the count ahead of other subscribers.
| return; | ||
| } | ||
| try { | ||
| layer.setPointerCapture(pressPointerId); |
There was a problem hiding this comment.
Miss: desktop behavior change needs an owner decision. This capture also takes the pointer's hover for the rest of the press: with the right button still down, moving onto a DropdownMenu item no longer highlights it or moves focus to it (Chromium and WebKit), until the release and the next move. A release over the layer already keeps it open, so holding the capture only while the pointer is outside the layer (release it on a pointermove over the layer, take it again when the pointer leaves) keeps the fix and hover. Otherwise cite an owner decision that covers the pause.
…ners join the input stream Over the layer the capture lets go, so a held right press still highlights a menu's rows; outside it the capture holds, so the release there cannot dismiss the layer. The press and context-menu listeners subscribe through the shared input stream.
A claim lasts until its press is released, so each case releases the press it made and no claim reaches the next case.
|
/approve-spec 0d4a106 |
There was a problem hiding this comment.
Changes requested. The fix works: in Chromium and WebKit, a light-dismiss layer or a DropdownMenu opened from a right click now stays open after the button comes up, the next press outside closes it, and in the light DOM hover keeps working over the menu while the button is held. The spec change is approved for this head, and the gesture counter now listens through the shared input stream.
- Inside an open shadow root,
claimContextMenuPress(gestureCounter.ts:117-121) asksdocument.elementFromPoint, which answers with the shadow host, so the capture never lets go over the layer and, while the opening right button is held, DropdownMenu rows neither highlight nor take focus (Chromium and WebKit); hit-test in the layer's own root asuseMenuPress.tsdoes, and add a shadow-root case to the unit test.
Optional: the new AST-003 verification row could name WebKit beside Chromium, since the browser spec runs in both.
[Automated review]
| const under = layer.ownerDocument.elementFromPoint( | ||
| event.clientX, | ||
| event.clientY, | ||
| ); | ||
| holdCapture(under == null || !layer.contains(under)); |
There was a problem hiding this comment.
Inside an open shadow root, document.elementFromPoint answers with the shadow host, which layer does not contain, so this keeps the capture while the pointer is over the layer: with the right button held, DropdownMenu rows stop highlighting and taking focus (Chromium and WebKit). Hit-testing in the layer's own tree fixes it, as useMenuPress.ts does:
const root = layer.getRootNode();
const scope =
root !== layer.ownerDocument &&
typeof (root as ShadowRoot).elementFromPoint === 'function'
? (root as ShadowRoot)
: layer.ownerDocument;
const under = scope.elementFromPoint(event.clientX, event.clientY);
holdCapture(under == null || !layer.contains(under));In an open shadow root the document's hit test answers with the shadow host, so the capture held over the layer and its rows stopped highlighting. The test now runs in the layer's root, as the menu press does.
…ess itself The case moves to gestureCounter.test.ts, where the layer sits in an open shadow root and the root's own hit test answers with the layer.
|
/approve-spec b6bd39e |
There was a problem hiding this comment.
Approved pending CI: this is a draft, so CI has not run on this head yet, and marking it ready for review starts it.
The claim now hit-tests in the layer's own tree, so inside an open shadow root a DropdownMenu opened from a right click still highlights and focuses the row under the held mouse, and it stays open after the release in Chromium and WebKit. The new AST-003 verification row and the desktop fix carry an engineering owner's /approve-spec for this head.
[Automated review]
cixzhang
added
the
ci:run-a11y
Fixes #6947.
User impact
A person who right-clicks an element that opens a light-dismissible layer from its
contextmenuevent sees the layer flash and close as soon as they let go of the mouse button. The common case is a DropdownMenu opened throughisMenuOpenfrom a right click on a nearby element, as in the issue. With this fix the menu stays open after that release and closes on the person's next press outside, as a context menu does.Problem and solution fit
popover="auto". The browser's light dismiss runs at eachpointerup, and a release outside every open popover reads as a dismissal. The press that opened the layer started before the layer existed, so its own release lands outside it.spec:AST-003FR11–FR13 state that one physical gesture cannot do both, whatever the browser's event order.The fix gives that press to the layer. When a light-dismissible layer shows while a press that fired
contextmenuis still down, the layer captures that pointer while it is outside the layer, so a release there lands on the layer and light dismiss leaves it open. Over the layer the capture lets go, so the held pointer still highlights a menu's rows, and a release there is inside the layer anyway. The claim ends at that press's release. CallingpreventDefaulton thepointerupdoes not stop Chromium's light dismiss (checked in real Chromium), so pointer capture is the lever. The tests use a mouse right click. The same path applies to any press that is still down when it firescontextmenu(Android Chrome's long press is one); iOS Safari fires nocontextmenufrom a long press.Expected behavior and authority
spec:AST-003FR11–FR13: pointer dismissal goes by gesture identity, and the gesture that opens a layer does not also close it. This PR adds one AST-003 verification row for FR11–FR13 that names the context-menu press case (Specs & Decisions asked for this row).Smallest restoration
Layer/gestureCounter.tsalready counts gestures for FR11–FR13. It now also records the pointer of the press in flight and whether that press firedcontextmenu, through the shared input stream (subscribeInput). A newclaimContextMenuPress(layer)captures that pointer to the layer while it is outside the layer, until the press's release. It hit-tests in the layer's own tree, so a layer inside an open shadow root works too.Layer/useLayer.tsx:show()makes that one call for a light-dismissible layer, right after the browser accepts the show.outsidePress.ts, and nothing for a primary-button press. DropdownMenu's own trigger press keeps its invoker hold.Interaction with the entrance input hold in the Popover stack (#8223): that hold ignores new taps while a touch entrance plays. This claim only affects the one press that opened the layer, and only its release target. Where a finger's long press fires
contextmenuand opens a Popover, that finger releases on the Popover instead of outside it. The hold has nothing to ignore there, because that release is not a new tap. A tap that comes later during the entrance is a new gesture, so the hold handles it as it does now. The two do not touch the same event, and theuseLayerchange is one call so that stack rebases simply.Evidence
--repeat-each=3). The layer and the DropdownMenu stay open after the release, also when the release lands 60 px away from the zone, and a later click outside closes both. With the right button still down, moving onto the DropdownMenu focuses the row under the pointer; with a capture that never lets go (the earlier head), that case fails.main's current copy (13 of 13), which is the copy CI runs from the merge ref. The 8 visual audit specs need a git checkout and were not run in that environment; CI runs them.useLayer.test.tsx: a layer shown from a context-menu press captures that press's pointer; over the layer the capture lets go, outside it takes the pointer again, another pointer's move changes nothing, and the release ends the claim.gestureCounter.test.ts: in an open shadow root the claim hit-tests in the layer's own root, so the capture lets go over the layer. A keyboard context menu (Shift+F10, no press) and a layer without light dismiss capture nothing. The first test fails without theshow()call.Layer/__tests__/LayerContextMenuPress.a11y.browser.spec.ts(Chromium and WebKit projects): the real-engine cases above, on the newCore/LayerContextMenuPressOpensALayer story.Scope
Testing
vitest run packages/core/src/Layer packages/core/src/DropdownMenu packages/core/src/Popover packages/core/src/ContextMenu packages/core/src/Tooltipplaywright test --project=chromium packages/core/src/Layer/__tests__/LayerContextMenuPress.a11y.browser.spec.tsagainst a Storybook built from this head (right-button press and release, on and off the zone; a later click outside).Writing rubric results
R1 narration: none; the new verification row states the check and its failure in the present tense.
R2 research: none in the record; the Chromium
preventDefaultcheck lives in this description.R3 evidence: the row names the unit and real-Chromium checks that verify it.
R4 rejected alternatives: none in the record.
R5 change references: none.
R6 open questions: none.