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.
|
github-actions
Bot
added
the
needs:spec-owner-review
11d17f0 to
30320a7
Compare
![astracat-bot[bot]](https://avatars.githubusercontent.com/in/4796231?s=60&v=4){kind=small}
There was a problem hiding this comment.
This pull request is on hold for an engineering owner's decision: sliding the finger that opened a ContextMenu onto an action is a clear win on touch, but a lift before the finger has moved 10 px acts on nothing even over a row, which departs from the current useMenuPress FR1 ("A row MUST act when a press is released over it"), and no spec record states that handoff yet. An engineering owner can settle it by amending the useMenuPress record with the handoff exception (or asking for FR1 behavior), approving or rejecting the new draft requirements (AST-094, the touchOpen press mode in FR9 and DEC-8), and recording the decision by commenting /approve-spec 30320a7995ce2444f8b9a8a3abfee712ce6c1655 on this pull request; the author may do so as an engineering owner. Independently of that decision, please narrow this pull request to the ContextMenu change once its dependencies land (it currently carries about 27,000 authored lines) and fix these:
useMenuPress.spec.mdstill lists a 500 ms long-press delay whileuseMenuPress.tsnow uses 150 ms.- The
isOpenEffect inContextMenu.tsxsets state after the menu closes; do that work in the close handler instead. MenuPressContinuationdocumentsxandy, whichcontinueGesturenever reads.
[Automated review]
| const wasOpenRef = useRef(isOpen); | ||
| useEffect(() => { | ||
| if (wasOpenRef.current && !isOpen) { | ||
| setIsTouchOpen(false); |
There was a problem hiding this comment.
This Effect clears isTouchOpen and releases the press feedback after isOpen turns false. Under architecture:react-component-runtime INV2 and INV3, do this where the menu closes (the close or toggle handler), or name the external system this Effect synchronizes.
| * UIKit's hold-open delay for a pull-down button (`menu.holdOpenMs`). A tap | ||
| * released sooner opens the menu through the trigger's click. | ||
| */ | ||
| export const MENU_PRESS_LONG_PRESS_MS: number = menu.holdOpenMs; |
There was a problem hiding this comment.
This lowers the held-finger delay from 500 ms to 150 ms for every DropdownMenu, but useMenuPress.spec.md still lists the long-press delay as 500 ms. Update the record and FR9, or keep 500 ms.
| pointerId: number; | ||
| pointerType: MenuPressPointerType; | ||
| /** Where the pointer is now, in viewport px. */ | ||
| x: number; |
There was a problem hiding this comment.
continueGesture never reads x or y; the handoff counts from the first move after the open. Remove these fields or document that they are ignored.
| // The finger never travelled the handoff distance: whatever row is | ||
| // under it now appeared under a press nobody moved, so nothing acts, | ||
| // the menu stays, and the trigger's click for this press is spent. | ||
| return idle({type: 'settle', stray: true, dismiss: false}); |
There was a problem hiding this comment.
A finger lifted here before 10 px of travel acts on nothing even over a row, which departs from current module:DropdownMenu/useMenuPress FR1. This waits on an engineering owner's decision; if accepted, the module record should state the handoff.
PR Analysis Report
Generated by PR Enrichment workflow | View current CI run |
30320a7 to
8507258
Compare
There was a problem hiding this comment.
This pull request is on hold for an engineering owner's decision: letting the finger that opened a ContextMenu slide onto an action is a clear improvement on iPhone and iPad, but a lift before that finger has moved 10 px acts on nothing even over a row, which departs from the current useMenuPress FR1 ("A row MUST act when a press is released over it"), and the branch also adds draft requirements no owner has approved yet (AST-094, the rewritten useMenuPress FR9 and DEC-8, and DropdownMenu FR8). An engineering owner can settle it by amending the useMenuPress record with the handoff exception (or asking for FR1 behavior), approving or rejecting those draft requirements, and recording the decision by commenting /approve-spec 8507258b8d930890afc2e5c68bc86d6ca7bbe152 on this pull request; the author may do so as an engineering owner. Independently of that decision, please narrow this pull request to the ContextMenu change once its dependencies land (it currently carries about 28,000 authored lines) and fix these:
useMenuPress.spec.mdstill lists a 500 ms long-press delay whileuseMenuPress.tsnow uses 150 ms.- The
isOpenEffect inContextMenu.tsxsets state after the menu closes; do that work in the close handler, or name the external system it synchronizes. MenuPressContinuationdocumentsxandy, whichcontinueGesturenever reads.
[Automated review]
8507258 to
53ad03a
Compare
github-actions
Bot
added
the
needs:design-review
|
Addressed in 53ad03a:
|
There was a problem hiding this comment.
This pull request is on hold for an engineering owner's decision: letting the finger that opened a ContextMenu slide onto an action is a clear improvement on touch screens, but a lift before that finger has moved 10 px acts on nothing even over a row, which departs from the current useMenuPress FR1 ("A row MUST act when a press is released over it"), and the branch also adds draft requirements no owner has approved yet (AST-094, the rewritten useMenuPress FR9 and DEC-8, and DropdownMenu FR8). An engineering owner can settle it by amending the useMenuPress record with the handoff exception (or asking for FR1 behavior), approving or rejecting those draft requirements, and recording the decision by commenting /approve-spec 53ad03a281b02154cf8d36087e0436518cc2dfb8 on this pull request; the author may do so as an engineering owner. Independently of that decision, please narrow this pull request to the ContextMenu change once its dependencies land (it carries about 34,000 authored lines today) and fix these:
- ContextMenu's import grows by about 28 KB gzip of touch-only code; load it lazily for coarse pointers or justify the cost, as
component-style-authoringINV6 requires. - The new
platformtheme-adaptation condition is outside the closed vocabulary ofAST-012FR2; drop it or have FR2 amended first. - CI's
pressableCoverage.test.tsfails becauseContextMenu.tsxcallsusePressFeedback()without a press arm the guard recognises. useMenuPress.spec.mdstill lists a 500 ms delay and the settle rule for the opening finger; #8078's current head already fixes both.- The slide-to-pick changeset says Android keeps today's menu, and the touch-presentation changeset says a theme's
mdis followed; neither matches the code. MenuPressContinuationdocumentsxandy, whichcontinueGesturenever reads.
[Automated review]
| '@astryxdesign/core': patch | ||
| --- | ||
|
|
||
| [feat] On iPhone and iPad, a finger still down when ContextMenu's menu opens can slide onto an action and lift to pick it, as on iOS. Once the finger has moved 10 pt in the open menu, the highlight follows it, and the action under the lift runs. A lift before that picks nothing and leaves the menu open, and a slide off the rows leaves none lit, with no focus ring on the menu. Other touch screens, Android included, keep today's menu: it stays open for a tap. |
There was a problem hiding this comment.
This says Android keeps today's menu, but the continuation runs wherever the recognizer runs (isEnabled: !isDisabled && isTouch, a coarse primary pointer, ContextMenu.tsx:805), the slide tests run on iOS and Android stubs alike, and the docs say every touch device. Say every touch screen here.
| * is everything else. Pair `ios` with `pointer: 'coarse'` for a touch | ||
| * profile, such as touch motion tuned per platform. | ||
| */ | ||
| platform?: 'ios' | 'other'; |
There was a problem hiding this comment.
spec:AST-012 FR2 (current) closes the when vocabulary to width, height, pointer, contrast and motion: "Future environment fields require explicit additions to this closed vocabulary." Nothing in this branch amends it, so drop platform (motion roles can be set without it) or have an owner amend FR2 first.
| // grows it while that machine believes the press, springs back when it | ||
| // ends with no menu, and stays grown while the menu it opened is up. Not | ||
| // shown on an inline trigger. | ||
| const pressable = usePressFeedback(); |
There was a problem hiding this comment.
CI test-ui (2/4) fails pressableCoverage.test.ts:190 on this file: it calls usePressFeedback() but carries no ':active' or '[data-astryx-press=' arm, and the guard does not see the growth that contextMenuPressFeedback.ts reads through PRESSED_ATTRIBUTE. Teach the guard about that reader, or give the trigger an attribute arm.
| import {useLongPress} from '../hooks/useLongPress'; | ||
| import {useScrollLock} from '../hooks/useScrollLock'; | ||
| import {useTouchPresentation} from '../utils/touchPresentation'; | ||
| import {getGestureHost, playHaptic, useGestureRecognizer} from '../gestures'; |
There was a problem hiding this comment.
These static imports put the gesture host, arena, recognizers, animator and UIKit tables in every ContextMenu import, desktop included: +28,282 bytes gzip measured base against head, against the 2 KB line in architecture:component-style-authoring INV6. Load the touch layer lazily on the first coarse-pointer press, or justify the cost.
53ad03a to
263437c
Compare
There was a problem hiding this comment.
This pull request is on hold for an engineering owner's decision: letting the finger that opened a ContextMenu slide onto an action is a clear improvement on touch screens, but a lift before that finger has moved 10 px acts on nothing even over a row, which departs from the current useMenuPress FR1 ("A row MUST act when a press is released over it"), and the branch also adds draft requirements no owner has approved yet (AST-094, the rewritten useMenuPress FR9 and DEC-8, and DropdownMenu FR8). An engineering owner can settle it by amending the useMenuPress record with the handoff exception (or asking for FR1 behavior), approving or rejecting those draft requirements, and recording the decision by commenting /approve-spec 86076e91586049ffb2f1c493270f5b0998440577 on this pull request; the author may do so as an engineering owner. Independently of that decision, please narrow this pull request to the ContextMenu change once its dependencies land (it carries about 38,500 authored lines today) and fix these:
- An explicit
presentation="popover"no longer opens the menu from a held finger, so on iPhone and iPad that ContextMenu cannot be opened by touch at all; keep the previous long press there, asAST-017FR3 requires of the restoring value. - ContextMenu's import grows by about 31 KB gzip of touch-only code, including spring rows it never reads; load it lazily for coarse pointers or justify the cost, as
component-style-authoringINV6 requires. useMenuPress.spec.mdstill lists a 500 ms delay and the settle rule for the opening finger, while the code uses 150 ms and the 10 px handoff.- The slide-to-pick changeset says Android keeps today's menu, and the touch-presentation changeset says a theme's
mdis followed; neither matches the code. MenuPressContinuationdocumentsxandy, whichcontinueGesturenever reads.- The new
@astryxdesign/core/motion/reducedMotion.stylexsubpath is stable and cites anAST-094open question that does not exist; drop it or keep it experimental.
[Automated review]
| break; | ||
| } | ||
| }, | ||
| {isEnabled: !isDisabled && touchModes}, |
There was a problem hiding this comment.
With presentation="popover" this turns the recognizer off, and nothing else listens for a held finger, so on iOS Safari (no contextmenu for a long press) that ContextMenu can no longer be opened by touch. On the base every ContextMenu opened from a 500 ms hold through useLongPress. spec:AST-017 FR3 requires the restoring presentation value to keep the previous input mode; keep the base long press when presentation is 'popover'.
| '@astryxdesign/core': patch | ||
| --- | ||
|
|
||
| [feat] Under a finger, a ContextMenu whose `presentation` is omitted now opens from a held finger on iOS's timeline (the touch model). Pass `presentation="popover"` to keep today's menu on every device: the menu at the pointer, with a held finger left to the browser. |
There was a problem hiding this comment.
"Today's menu" opened from a held finger on every touch device, iOS included. With this change presentation="popover" leaves the finger to the browser, which on iOS opens nothing. Either restore the long press for 'popover' or describe the regression here.
| contextMenu, | ||
| registerContextMenuTable, | ||
| registerSpringsTable, | ||
| springs, |
There was a problem hiding this comment.
Importing the whole springs table (and registering it at line 112) ships every spring row with ContextMenu, including springs.sheetPresent and springs.sheetDismiss, which no ContextMenu path reads. architecture:component-style-authoring INV6: importing a component does not pull in rows only other components read.
| @@ -177,6 +190,13 @@ const STATIC_EXPORTS = { | |||
| types: './dist/utils/interactionOverlay.stylex.d.ts', | |||
| default: './dist/utils/interactionOverlay.stylex.js', | |||
| }, | |||
| // Proposed; pending owner review (spec:AST-094 OQ4): the Reduce Motion | |||
There was a problem hiding this comment.
spec:AST-094 at this head has only OQ1 and OQ2, so this export cites nothing. As an ordinary subpath it becomes a stable import once released (spec:AST-017 FR21 puts experimental surfaces behind an experimental boundary). Drop it here or keep it experimental until an owner decides.
…droid 15 Chromium 158)
86076e9 to
a885d27
Compare
github-actions
Bot
added
the
needs:spec-owner-review
|
/approve-spec b9c1d9b |
github-actions
Bot
removed
the
needs:spec-owner-review
There was a problem hiding this comment.
Changes requested for this stack; a pull request in it lands only when it and every pull request below it are approved.
#9814: approve, pending CI. No blocking findings; CI has not run on this head yet.
#8125: changes requested.
- Attach this head's Chrome-with-Gboard Android-emulator result (animations at normal scale and removed, plus a slow-device profile), which
spec:AST-094Platform support and DEC-27 require for the touch model; the description reports an event pass with no result and says the curve runs still wait. - Regression:
mainnow carriesIndicator/boxHold.test.tsx"leaves a CheckboxList row in a ContextMenu unchanged after a long press that opened the menu", which holds a finger 600 ms and expects the menu open, so it fails once this branch takesmainbecause the touch model opens at 735 ms; move that case to the touch model's timeline or pin it topresentation="popover", since the behavior it guards still holds.
#9816: changes requested.
- Miss:
contextMenuPressFeedback.tsimports the wholespringstable and callsregisterSpringsTable(), which puts every spring role row, including rows only other components read, in every ContextMenu import, againstarchitecture:component-style-authoringINV6; importCONTEXT_MENU_LIFT_SPRINGandCONTEXT_MENU_DISMISS_SPRINGand drop the registration, since each role row registers itself as it loads. - Attach this head's Android-emulator result for the growth, which DEC-27 requires.
#8133: changes requested.
component:ContextMenuFR5 ends its slide sentence with "(FR12)", but FR12 is the hold growth and the slide is FR13, so point it to FR13.- Attach this head's Android-emulator result for the slide, which DEC-27 requires; the description names a run but gives no result.
Whole stack: changes requested on the findings above; otherwise the pull requests fit together and merge with main without conflicts. The record changes carry an engineering owner's approval for this head; the FR5 fix makes a new head, so an engineering owner (the author included) comments /approve-spec <full head SHA> again.
[Automated review]
| import {observeMutations} from '../utils/sharedMutationObserver'; | ||
| import {createMotionEngine, type Animator} from '../motion/animator'; | ||
| import {contextMenu} from '../motion/uikit/contextMenu'; | ||
| import {registerSpringsTable, springs} from '../motion/uikit/springs'; |
There was a problem hiding this comment.
Miss: this imports the whole springs table, and line 123 registers it, so every ContextMenu import now carries every spring row (sheet, alert, banner, slider, navigation) that only other components read. architecture:component-style-authoring INV6: a shared spring table "ships to a component only the rows that component uses". Import CONTEXT_MENU_LIFT_SPRING and CONTEXT_MENU_DISMISS_SPRING here and drop the registerSpringsTable() call: each role row registers itself as it loads.
| | FR2 | The `context-menu` target stays on the painted Pointer menu surface and alternative Touch menu surface, not on the Trigger area or cursor anchor. | `architecture:component-theming-surface`; current source | | ||
| | 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 | | ||
| | 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 | | ||
| | 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. The long press that opened the menu does not close it as it lifts; a tap outside closes it, and a mouse press outside closes it at the press. Under a finger the menu MUST close as a pan that starts outside it begins (`spec:AST-094` FR32), MUST hand the held finger that opened it to the menu as the opening press, and MUST treat a pen's press outside as FR32 does (FR33). The held finger may then slide onto an item and lift to select it (FR12). The lift and the grow apply only to a menu opened by touch (a touch-initiated open on a coarse pointer); a menu opened by mouse or keyboard keeps its released appearance and motion, with no lift, no tint and no grow. A touch-opened menu MUST grow out of the finger's point and shrink back into it (`spec:AST-094` FR31) as its own surface, by transform and opacity only (`spec:AST-094` FR5), with no second element such as a glass surface growing behind it, and on a block trigger MUST lift the trigger above the page with the menu beside it (FR33; DD1, DEC-2). The long-press presentation is a gesture component, so the trigger's lift plays `springs.contextMenuLift`, the menu's grow `springs.contextMenuMorph`, and the trigger's return, part of the menu's exit, `springs.contextMenuDismiss` (`spec:AST-094` FR4, FR7, FR45). Implementation gap: it keeps the layer's own entrance and exit, no trigger lift or page tint exists, and a pen press outside closes it at the press. Right-click, Shift+F10, long-press invocation, and keyboard navigation are as FR1 and FR8 state. | `module:DropdownMenu/useMenuPress`; `spec:AST-094` FR26–FR33 | |
There was a problem hiding this comment.
After the merge of main renumbered the rows, FR12 is the hold growth and the slide is FR13, so this sentence now points to the wrong row. Suggested: "The held finger may then slide onto an item and lift to select it (FR13)."
| * @input A DOM node | ||
| * @output Exports parentAcrossShadow, containsAlongFlatTree, which walks it, | ||
| * and deepActiveElement | ||
| * deepActiveElement, shadowHostOf, a shadow root's host, and |
There was a problem hiding this comment.
Non-blocking: "a shadow root's host" restates the export's name (architecture:knowledge-contracts INV29). Suggested: list shadowHostOf and shadowRootsOutward alone.
cixzhang
added
the
ci:run
The headers of `ContextMenuLongPress.interaction.browser.spec.ts`, `contextMenuPress.test.ts`, `outsidePress.test.ts` and `outsidePress.ts` ran past INV29's sentence length, and one line past 100 columns. Each now says one thing per sentence. Comments only.
The headers of `outsidePress.ts` (`@output`) and `flatTree.ts` (`@position`) ran past INV29's sentence length. Each now says one thing per sentence. Comments only.
…branch (outsidePress header: both splits)
…e-header sentences: the slide onto an item kept)
|
/approve-spec 1a63672 |
There was a problem hiding this comment.
Changes requested for this stack; a pull request in it lands only when it and every pull request below it are approved. The record changes carry an engineering owner's approval at each head, so what remains are the fixes below.
#9814 — changes requested
- Regression:
packages/core/src/Layer/outsidePress.tsno longer merges withmain, which now importsisNode,isElementandisShadowRootfromutils/nodeType.ts, so take that import, drop the local copies (lines 282-292) and haveshadowHostOfinutils/flatTree.tsuseisShadowRootfrom the same module.
#8125 — changes requested
- Regression: merged with
main,Indicator/boxHold.test.tsx"leaves a CheckboxList row in a ContextMenu unchanged after a long press that opened the menu" fails because it holds the finger 600 ms under a coarse pointer and the touch model opens at 735 ms, so hold past 735 ms there or pin that case topresentation="popover". - The description reports no Android-emulator result under
spec:AST-094DEC-27's conditions (the exact head, animations at normal scale and removed, and a slow-device profile), so report those three runs.
#9816 — changes requested
- Miss:
contextMenuPressFeedback.tsimports the wholespringstable and callsregisterSpringsTable()(lines 33 and 123), so every ContextMenu import carries spring rows only other components read, againstarchitecture:component-style-authoringINV6; importCONTEXT_MENU_LIFT_SPRINGandCONTEXT_MENU_DISMISS_SPRINGinstead. - The description reports no Android-emulator result under DEC-27's conditions for the growth, so report those three runs.
#8133 — changes requested
docs/specs/components/ContextMenu/ContextMenu.spec.mdFR5 (line 109) points the slide to FR12, which is the hold growth, where it should say FR13.- The description reports no Android-emulator result under DEC-27's conditions for the slide, so report those three runs.
Whole stack: changes requested. The slide itself follows spec:AST-094 FR33 and useMenuPress FR11, and mouse, keyboard and presentation="popover" keep their released behaviour.
[Automated review]
| import {UIKIT_PAN} from '../motion/uikit/gestures'; | ||
| import {shadowHostOf, shadowRootsOutward} from '../utils/flatTree'; | ||
| import {subscribeInput} from '../utils/inputStream'; |
There was a problem hiding this comment.
Regression: main (#10224) now imports isElement, isNode and isShadowRoot from ../utils/nodeType here and deletes this file's local copies, so these lines and the local isNode/isElement further down conflict with main. When this branch takes main, keep main's import (isElement, isNode) beside these three and drop the local helpers.
| * any other node. | ||
| */ | ||
| export function shadowHostOf(node: Node): Element | null { | ||
| return node.nodeType === node.DOCUMENT_FRAGMENT_NODE && 'host' in node |
There was a problem hiding this comment.
Regression: main now has isShadowRoot in utils/nodeType.ts, the same cross-window check. Once this branch takes main, use it here so there is one shadow-root check: return isShadowRoot(node) ? node.host : null;.
`utils/nodeType.ts` now holds the cross-window shadow-root check, so `shadowHostOf` uses it and the flat tree has one shadow-root test.
Under a coarse pointer the menu now opens on the touch model's timeline by default. This case covers the box-side rule for the popover menu's long press, so it asks for `presentation="popover"` and keeps its 600 ms hold.
`contextMenuPressFeedback.ts` imported the whole `springs` table and registered it, which put every spring role row in every ContextMenu import. It now imports `CONTEXT_MENU_LIFT_SPRING` and `CONTEXT_MENU_DISMISS_SPRING`, which register their own roles when the module loads.
After the renumber, the slide onto an item is FR13 and FR12 is the hold growth. FR5's slide sentence still named FR12.
github-actions
Bot
added
the
needs:spec-owner-review
|
/approve-spec ebe2c47 |
github-actions
Bot
removed
the
needs:spec-owner-review
There was a problem hiding this comment.
Changes requested for this stack; a pull request in it lands only when it and every pull request below it are approved.
#9814 — approved pending CI
No blocking findings; approved pending CI, which has not run on this head yet.
#8125 — changes requested
- Report the Android-emulator result for the touch model's open under
spec:AST-094DEC-27's conditions: this head, animations at normal scale and with animations removed, plus a slow-device profile.
#9816 — changes requested
- Report the Android-emulator result for the trigger's growth and both of its returns under DEC-27's conditions.
#8133 — changes requested
- Miss: the slide changeset and the title promise the slide "after a long press opens the menu at the finger", but the docs and tests call the
presentation="popover"and fine-pointer 500 ms path "the long press" and that path hands nothing over, so scope both to "where the primary pointer is coarse andpresentationis unset, after the held finger opens the menu at the finger". - Report the Android-emulator result for the slide onto an item under DEC-27's conditions.
Whole stack: changes requested on the findings above; it merges cleanly with main, the slide follows spec:AST-094 FR33 and the useMenuPress FR11 handoff, and mouse, keyboard and presentation="popover" keep their released behaviour.
[Automated review]
| | FR10 | ContextMenu MUST be built on the public headless context menu (`spec:AST-107` FR8), with its invocation, open state, and cleanup the factory's and no behavior of its own beside the factory's. Implementation gap: no headless context menu ships; ContextMenu owns its invocation, open state, outside-press listener, and cleanup itself. | `spec:AST-107` FR8 | | ||
| | FR11 | Tab or Shift+Tab on a row of the pointer menu, its own or an open sub-menu flyout's at any depth, closes every open level and the menu and calls `onOpenChange(false)` once, as `component:DropdownMenu` FR23 states for its menu. | WAI-ARIA Menu Button pattern; current tests | | ||
| | FR12 | Under a finger, a block trigger whose menu opens at the finger grows during a hold toward 1.0735 times its size on `springs.contextMenuLift`, from the onset, 150 ms after the finger lands, and it returns after a lift before the commit on `springs.contextMenuDismiss`, as FR5 names them. The hold commits at 400 ms and the menu displays at 735 ms, as UIKit's do. They are measured input, not motion, and stay internal (`spec:AST-094` FR4, FR45). | Measured on iOS 26.2 (`motion/uikit/contextMenu.ts`): UIKit commits at 400 ms, shows the menu at 735 ms, and grows the view on a linear ramp chased by a spring. `spec:AST-094` FR4, FR45: the two moments are measured input, and the long-press presentation is a gesture component (FR5), so the growth and its return play their spring roles | | ||
| | FR13 | After a long press opens the menu at the finger, the same finger may slide onto an item without lifting; lifting over an item selects it, and lifting over no item leaves the menu open. A lift before the finger has moved UIMenu's handoff distance selects nothing (`spec:AST-094` FR33). | `module:DropdownMenu/useMenuPress` FR11; `spec:AST-094` FR33 | |
There was a problem hiding this comment.
Non-blocking: the docs call the presentation="popover" 500 ms path "the long press" (ContextMenu.doc.mjs:135), and that path hands nothing over. Suggested: "After the touch model's hold opens the menu at the finger, the same finger may slide onto an item without lifting; …" The slide changeset says the same and could follow.
Stack: #9814 → #8125 → #9816 → #8133
Merge group: #9814, #8125, #9816, #8133
User need
A long press opens the ContextMenu under the finger, but that finger can't pick anything: the user lifts and presses again. iOS context menus and DropdownMenu let it slide onto an item and lift.
Problem and solution fit
pointerIdOfnames its pointer, and the internalcontinueGesturetakes it as the opening press.spec:AST-094FR33): past 10 px from the first position the open menu sees, the item under the finger lights and the lift selects it; a still lift selects nothing.Current specification
module:DropdownMenu/useMenuPressFR11 asks for this continuation, andcomponent:ContextMenuFR5 for the handoff. New FR13 states the slide: "After a long press opens the menu at the finger, the same finger may slide onto an item without lifting; lifting over an item selects it, and lifting over no item leaves the menu open", then FR33's handoff.Public delta and caller burden
No API change; touch only. Own commits: 691 authored lines, mostly tests. It adds 202 B gzip to ContextMenu's import and 87 B to DropdownMenu's (Testing).
Design and alternatives
Rejected: ContextMenu tracking the finger itself, since the shared press model owns the highlight, pick and stray click.
continueGestureis ported from #8507, which closes as absorbed.Evidence
Human check: the owner's card 33c pass, on this head's product files. iPhone, iOS 26.2 Simulator at this head: hold, menu, tap outside and early lift. Android Chrome: Code Quality's Chrome-with-Gboard emulator run on this head's product files passes every event claim, the slide to an action included (
spec:AST-094DEC-27); the curves wait for a quiet host.Scope
Writing rubric results
Over
component:ContextMenu: FR13 adds the slide, then FR33's handoff; FR5 points to it in place of its gap, at the same cell width. Owner-gated.module:DropdownMenu/useMenuPressFR11 andspec:AST-094FR33.Testing
On #9816 at 4efcbc4, over #8125 c4f8964 and the merge worker's #9814 head (main 3731ec3). This pull request carries only its own five commits, the same ones that passed the stack's gates on main 9e559e9 (6b4f1dc). Two lines met #9816's: the touch story's wording keeps the coarse-pointer condition, and FR5's row points to FR13 on main's text. FR12 keeps #9816's springs.
The stack, as the owner checked it on cards 33 and 33b at fec928b, differs in what #9816 and #8125 name: the growth and both returns play
springs.contextMenuLiftandsprings.contextMenuDismiss(#9915), one outside-press watch serves every pointer (INV16), and the growth keeps its 150 ms onset under #9765's touch-down mark. This pull request's slide onto an item is unchanged.Local checks at efe3f12. Later commits change changesets, docs, comments, the story text, test titles and FR13's scope (a long press that opens the menu at the finger), with #9816's never-read field and its anchor fix (only the trigger's own growth is cancelled). At this head, f913915: core typecheck and the ContextMenu units (171 passed). On its code before a one-helper move (8bb478c):
CI=true pnpm lint(0 errors), the ContextMenu, Layer, DropdownMenu, hooks and utils units (3,010 passed), check:knowledge, check:changesets, the ContextMenu interaction specs in Chromium (22 passed, 1 skipped) and WebKit (19 passed, 4 skipped), and the ContextMenu a11y specs in Chromium (24 passed). At efe3f12:CI=true pnpm lint(0 errors); core typecheck; the ContextMenu, Layer, DropdownMenu, motion, hooks, gestures and utils units (4,701 passed); check:knowledge; check:changesets; the Chromium interaction specs for ContextMenu, Layer and DropdownMenu (44 passed, 1 skipped); WebKit's ContextMenuLongPress, inputHold and DropdownMenuDragHighlight (34 passed, 8 skipped).useMenuPress.test.tsx, each fail before the change.presentation="popover"path changes.Size (
architecture:component-style-authoringINV6), gzip, import with its transitive modules:Of ContextMenu's growth, its own touch code is about 4.2 KB minified. The rest is core's shared gesture host, arena, long-press recognizer and motion engine, which a page that uses Switch, Slider, SegmentedControl or DateInput already loads. The spec owners accepted the cost (Code Quality's measurement), and splitting the gesture host is a separate core item.