Repository navigation
feat(ContextMenu): createContextMenu and useContextMenu, a headless context menu (spec:AST-107 FR8) - #8693
Draft
vjeux wants to merge 2018 commits into
Draft
feat(ContextMenu): createContextMenu and useContextMenu, a headless context menu (spec:AST-107 FR8)#8693vjeux wants to merge 2018 commits into
vjeux wants to merge 2018 commits into
Conversation
The morph spring moved into the springs table as the motion role springs.contextMenuMorph: the menu's growth from the finger keeps its start width in the context menu's table and plays on the role.
…ads its end before the arena's reset) into meta#8336
The case walked ContextMenu, DropdownMenu, useLongPress and useMenuPress's imports and failed on the UIKit gestures table: it kept that table out of the menus' bundles, the lazy design the no-lazy-load ruling and direct imports retire. The long-press numbers keep one source, still pinned by the table, recognizer and hooks case and by the hooks reaching longPress.ts.
… watcher test's tail fix) into the input hold
…xposes no ontouchstart is taken once Chromium fixes ontouchstart when the page is created, so turning touch emulation on later sends Touch Events without it: the host reads Pointer Events for the finger and must not read its Touch Events too. A one-finger long press fails here, the finger tracked twice.
…from their pointer events install() reads a finger from its pointer events when the window has no ontouchstart, but the four touch handlers still took the Touch Events a browser sends anyway (Chromium after touch emulation turns on), tracking the finger twice, so a one-finger long press never recognized. Each touch handler now returns while usesTouchEvents is false, after noting the fingers down, as install()'s comment promised.
…ter events) into meta#8320
…ter events) into meta#8293
…ter events) into meta#8356
…ter events) into meta#8358
…n before the click guard internal/motion.ts (meta#8562) re-exports UIKIT_CLICK from the barrel, so a tree carrying both fails typecheck and the Storybook build since this pull request dropped it. The barrel keeps it, with the click guard's window, and the table of every gesture group still leaves it out. Fails until the barrel exports it.
…'s window This pull request dropped UIKIT_CLICK when the click guard took the stray-click window over, but importers written before it read the barrel's UIKIT_CLICK (internal/motion.ts in meta#8562 re-exports it), so a tree carrying both failed typecheck and the Storybook build. UIKIT_CLICK stays in the barrel, deprecated, with the guard's 400 ms (CLICK_GUARD_WINDOW_MS), and UIKIT_GESTURES still leaves it out.
…ter events) into meta#8360
…ter events) into the every-pointer host # Conflicts: # packages/core/src/gestures/__tests__/GestureHost.test.ts
…o on close (spec:AST-107 FR8) With focus on another control (a tap there, then a hold or open() on the element), closing the menu returned focus to that control: the menu restored whatever had focus before it opened. Closing now focuses the element, or what had focus inside it before the open (a focused control in ContextMenu's trigger keeps it). An element that takes no focus (ContextMenu's trigger wrapper has no tab stop) leaves focus where it was, as before. The factory's doc says so.
…ynchronizes with (react-component-runtime INV3) The effect hands requests from outside React (the element's own listeners and the controller's open() and close()) to the menu's handle once it has rendered; the menu reports onOpenChange from its own show and hide (INV2).
…lement, and keeps an inline one's menu at the finger (spec:AST-094 FR33) FR33 lifts a ContextMenu that a held finger opens on a block trigger. Before this stack took ContextMenu built on the factory, the factory's menu opened at the finger with no lift (on this PR's previous head, a held finger on a block row opens the menu and nothing lifts: isOpen true, lifted false). Now the factory's menu runs ContextMenu's touch model: a block element lifts, and an inline one keeps its menu at the finger.
…s at a blur, keeps a kept press's touchmove to the press, and drives hover Three cases from the maintainer's review of the every-pointer host (spec:AST-094 FR24, FR43), each failing on this head: - a window blur during a mouse press left the host's page listeners subscribed; - a press tracker created with keepsTouch kept its blocking touchmove on its element while idle, where a fine pointer's element keeps one passive pointerdown; - a mouse or a pen moving over an element never reached its hover recognizer, which was wired for a coarse pointer only.
…pec PR, as asked on meta#8693 architecture:layer-runtime INV13 (the exit copy), the three springs.contextMenu* rows in spec:AST-094 FR4, the spec:AST-057 reword and component:ContextMenu DEC-2 return to main's text here; a spec PR carries them for the owner's exact-head approval, so this code can land without waiting on them.
…extMenu's entries Main's guard holds the root entry to no gesture code at all, which ContextMenu's long press cannot meet once the root re-exports it. meta#8671's guard checks each public import and lists ContextMenu as a gesture user, so the root carries only what its touch components import. Co-authored-by: Christopher Chedeau <vjeux@users.noreply.github.com>
…til spec:AST-094 FR4 names them With FR4's three springs.contextMenu* rows moved to their own spec PR, the motion registry here keeps exactly the roles main's FR4 names (its own test reads the record): ContextMenu's lift, dismissal and morph springs play as internal constants, as every spring FR4 does not name, and the menu's close keeps springs.menuClose. The spec PR that adds the rows registers them as roles again.
…del file, not a surface The widened press-arm rule (a script that reads PRESSED_ATTRIBUTE) caught hooks/actionGroupTracker.ts, which writes the press attribute on a group's rows itself and reads it back only to finish its own fade. That is the documented second writer beside the controller (the gestures README names both writers), not a surface that paints from the controller's write, so it joins the controller and the overlay utility in MODEL_FILES. Co-authored-by: Christopher Chedeau <vjeux@users.noreply.github.com>
…ndle guard, and pressableCoverage's model files)
…int (spec:AST-107 FR6) Red: AST-107's public contract imports useContextMenu from @astryxdesign/core/hooks, which lacks it; asked on meta#8693.
…nt (spec:AST-107 FR6) AST-107 imports useContextMenu from @astryxdesign/core/hooks, with the other public gesture hooks; it was only in the root and ContextMenu's entry. The hooks entry now exports it and its types, its doc page's import path is the hooks entry, and the per-import touch bundle guard lists it as a gesture user: its element opens on the context request's hold.
… at a blur, keeps a kept press's touchmove to the press, and drives hover - A window blur during an armed press ends it as its lift does: the page listeners go once its contacts are cancelled. - A press tracker created with keepsTouch gets its blocking touchmove where the primary pointer is coarse, before every touch, and where it is fine only during an armed press, whose pointerdown comes before the touch it brings; a claiming recognizer keeps its listener as before. - Hover reaches a hover recognizer for every pointer: where the primary pointer is fine, a mouse or a pen hovering its element is heard through two passive listeners on that element, pointermove and pointerout, with no page listener (spec:AST-094 FR24, FR43).
…selection for that drag only The engineering owner's consistency edit (on meta#8665): a mouse or pen drag that a recognizer claims suppresses text selection for that drag only, and an unclaimed mouse drag keeps today's behavior.
…s README says The README's every-pointer section says a right click, a pen's barrel button and a ctrl-click on macOS start no press, but the host excluded only the mouse's: a pen's barrel press began and ended a press. Fails until the host excludes it, under a fine and a coarse pointer.
The host starts no press for a non-primary button of any pointer but a finger, so a pen's barrel button, like a right click, is a context click (spec:AST-094 FR43), as the gestures README already says.
…tens under a fine pointer A Pens entry says a pen pans, taps and long-presses as a finger does, keeps text unselected where its hold acts, reaches a hover recognizer, starts no press with its barrel button, never mixes with a finger in a pinch, and does not drive the screen-edge pan. The fine-pointer entry and the every-pointer changeset add the blur, the kept press's touchmove and the hover listeners.
meta#8665 now carries meta#8360's new head and the maintainer's host review: a blur ends an armed press, a kept press's touchmove follows the press, hover reaches every pointer, and a pen's barrel button starts no press. The one conflict, utils/pressFeedback.ts, takes meta#8665's text: both merges of main resolved it the same way, and this pull request does not change that file.
…peat control owns only its hold - A pen held still requests through the hold, and the browser then fires the pen's own context menu event, during the hold or at its lift: a second request for one gesture. One request only, the hold's. - The engineering owner's ruling on meta#8670: a hold-repeat control inside a row owns only its hold; its right click and Shift+F10 still reach the row's context request. Today both are refused there. Both fail until the context request refuses the pen's echo and leaves a kept press only its hold.
…eat control owns only its hold - The context request refuses a pen's own menu event during its hold and within the menu echo after its lift, as it refuses a held finger's and a key's. (A pen's barrel button already starts no press, from meta#8665.) - A hold-repeat control inside a row keeps only its hold: the row's right click and Shift+F10 reach the request through it, as the engineering owner ruled.
… with no options a record does not need The engineering owner's ruling on meta#8670: cut the two-stage commitMs, requestMs and requestOnLift, and the allowableMovement and isPerAxis tolerance, until a record calls for them. The hold is UIKit's long press, 500 ms and 10 px as a distance, and requests as it begins; onHold reports it committed, then ended or cancelled. LongPressGestureRecognizer loses isPerAxis and geometry its axisTravel, with their tests.
…, with no stages (spec:AST-094 FR43) Red: meta#8670 cuts the context request's two-stage hold (commitMs, requestMs, requestOnLift) and its per-axis allowance, as asked there: the hold is UIKit's long press and requests as it begins. ContextMenu and the factory's menu still open a held finger at 735 ms, open at a lift after 400 ms, and allow 11.5 pt on each axis. They should open at 500 ms under any primary pointer, open nothing for a lift before it, and open nothing for a drift past 10 pt.
…press, with no options) into the headless context menu packages/core/src/utils/pressFeedback.ts and its test: meta#8670's text, with this stack's press-paint subscription (subscribePressPaint, which ContextMenu's press feedback follows) kept beside it. packages/core/src/hooks/useLongPress.ts, its test, its page and useMenuPress.ts: meta#8670's text. This stack's useLongPress `stages` and press start times served ContextMenu's two-stage hold, which no longer exists (the context request opens ContextMenu, and its hold has no stages), so they go, with their changeset and the hooks entry's LongPressStage type.
… with none of the hold options meta#8670 cut (spec:AST-094 FR43) meta#8670 cuts the context request's two-stage hold (commitMs, requestMs, requestOnLift), allowableMovement and isPerAxis, as asked there: the hold is UIKit's long press and requests as it begins. ContextMenu and the factory passed them for UIContextMenuInteraction's 400 ms commit, 735 ms menu, open at a lift and 11.5 pt on each axis. They pass none now: under any primary pointer a held finger gets the menu at 500 ms, a lift before it is a tap, and a drift past 10 pt (a distance) opens nothing. Under the touch model the commit, now the long press, plays the haptic and the press feedback plays UIKit's press curve until the menu shows. The UIKit table's per-axis drift guess has no reader left and goes. The tests, the interaction specs, the docs, the story and the changesets that described the two-stage hold describe the long press; the tests of a lift after the commit and of a drag between the commit and the menu go with that window.
…ow, and Escape returns focus to it from another row's focus (spec:AST-094 FR33, spec:AST-107 FR8) The story's Inbox row, with Drafts focused first: a held finger opens its menu with the lift up beside it, and Escape closes the menu and focuses Inbox, in Chromium and WebKit.
7 of 8 tasks
Contributor
Author
![astracat-bot[bot]](https://avatars.githubusercontent.com/in/4796231?s=60&v=4){kind=small}
There was a problem hiding this comment.
Approval waits on an engineering owner's decision on two spec points: the sentence this branch adds to spec:AST-094 FR41 (a mouse or pen drag that a recognizer claims suppresses text selection), which the current FR41 and its verification row do not allow, and ContextMenu's lift playing springs.contextMenuLift, springs.contextMenuDismiss and springs.contextMenuMorph as literals, which FR4 forbids until #8830 names them. An engineering owner, the author included, can record that decision by commenting /approve-spec <full head SHA>, or the FR41 edit can leave this pull request and the lift can wait for #8830. Whatever the decision, please land the stacked predecessors and rebase so this diff is the headless menu and ContextMenu's move onto it (about 46,000 changed lines today), and fix these:
ContextMenu.tsx:459keeps the menu slot's DOM node in React state, whichreact-component-runtimeINV1 forbids.- The every-commit effects at
ContextMenu.tsx:503anduseContextMenu.tsx:151should name the system they synchronize (INV3); ContextMenu's re-renders its menu whenever a caller passes a newitemsarray. ContextMenuContentRendererand the dense docs say therendercleanup runs at close, but it runs atdestroy.
[Automated review]
| // content on scroll (#3465); the menu places it. | ||
| const cursorAnchorRef = useRef<HTMLSpanElement>(null); | ||
| // The menu's container lives here, beside the trigger. | ||
| const [menuSlot, setMenuSlot] = useState<HTMLSpanElement | null>(null); |
There was a problem hiding this comment.
react-component-runtime INV1 says "DOM nodes and whole DOM collections MUST NOT be stored in React state; store only the minimal rendering key or own the node through a ref". This slot node enters state through ref={setMenuSlot} (line 573), so every ContextMenu, each Table cell with context actions included, renders a second time when the ref lands and a third when the effect at line 520 attaches the factory, and the trigger has no context request until that passive effect runs. Owning the slot through a ref, or letting the factory place its container from the trigger, avoids both.
Comment on lines
+503
to
+516
| useIsomorphicLayoutEffect(() => { | ||
| onOpenChangeRef.current = onOpenChange; | ||
| latestRef.current = {factoryOptions, view}; | ||
| const controller = controllerRef.current; | ||
| if (controller == null) { | ||
| return; | ||
| } | ||
| updateContextMenu(controller, factoryOptions(slot)); | ||
| viewContextMenu(controller, { | ||
| ...view, | ||
| cursorAnchor: cursorAnchorRef.current, | ||
| containerParent: menuSlot, | ||
| }); | ||
| }); |
There was a problem hiding this comment.
This every-commit layout effect names no system it synchronizes (INV3: "A new or changed Effect MUST name the external system it synchronizes"), and through updateContextMenu/viewContextMenu it sets this component's menu state whenever the options differ. Table's lazy cell menu passes items={options ?? []} (Table/tableContextMenu.tsx:113), a new array on every render, so each render of such a cell renders its ContextMenu twice, the second synchronously from here. Naming the factory as the synchronized system, and comparing items by content before re-rendering the portal, would fix both.
Comment on lines
+151
to
+166
| useLayoutEffect(() => { | ||
| latestRef.current = {options, wrap}; | ||
| const controller = controllerRef.current; | ||
| if (controller == null) { | ||
| return; | ||
| } | ||
| const portal = portalRef.current; | ||
| updateContextMenu( | ||
| controller, | ||
| factoryOptions(options, handleOpenChange, portal), | ||
| ); | ||
| bridgeContextMenu(controller, wrap); | ||
| if (portal != null) { | ||
| portal.draw(options.menuContent, wrap); | ||
| } | ||
| }); |
There was a problem hiding this comment.
This every-commit layout effect passes options, providers and menuContent to the factory and to the hook's own root, but has no comment naming the system it synchronizes, which INV3 requires of a new Effect. A one-line comment naming the factory's menu and the hook's content root would settle it.
Comment on lines
+73
to
+79
| /** | ||
| * Renders a menu's content into `container` without React; returns a cleanup | ||
| * that runs when the menu closes. | ||
| */ | ||
| export type ContextMenuContentRenderer = ( | ||
| container: HTMLElement, | ||
| ) => (() => void) | undefined; |
There was a problem hiding this comment.
This exported comment says the cleanup "runs when the menu closes", and createContextMenu.doc.mjs:138 says render "fills the menu in plain DOM when it opens". The content mounts with the closed menu, so render runs once as the menu is attached and its cleanup runs at destroy (lines 315-326, 403-405), as createContextMenu.doc.mjs:38 says. A caller who frees resources or rebuilds rows per open, as this comment suggests, sees neither happen.
Comment on lines
+471
to
+473
| (`spec:AST-017` FR59; DEC-9). A mouse or pen drag that a recognizer claims | ||
| suppresses text selection for that drag only; an unclaimed mouse drag keeps | ||
| today's behavior. |
There was a problem hiding this comment.
This sentence adds a requirement the current FR41 does not state: on main, a mouse "otherwise keeps its released behavior", and the unchanged verification row still fails "a change ... to mouse, keyboard, or assistive-technology selection". It also leaves open what happens to a selection made before the drag, which the lock clears at the claim (packages/core/src/utils/selectionLock.ts:148). An engineering owner's exact-head decision is needed for this edit.
vjeux
marked this pull request as draft
October 29, 2026 22:39
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Carries #8125, #8201, #8213 and #8228 (ContextMenu's touch line) and #8670 (the context request); #8747 (ContextMenu built on the factory) is merged in, as asked. This head's own changes: vjeux/astryx@061a70f...b9608e9
User need
Part of the UIKit-parity program (
spec:AST-107FR8): a context menu on an element a product already renders, such as a list row, with or without React. ContextMenu and the factory are one implementation, so they open, present and dismiss alike.Problem and solution fit
createContextMenu(element, options)attaches ContextMenu's menu to the element; it opens on the element's context request (#8670) oropen(at), anddestroy()removes all it added.useContextMenu(ref, options), from@astryxdesign/core/hooks, is its React wrapper (FR6).<ContextMenu>attaches the factory to its own trigger (component:ContextMenuFR10). Under a finger the menu runs ContextMenu's touch model: the press feedback, the haptic and, on a block element, the lift (spec:AST-094FR33).What led to this push, from review:
open()while another control had focus, closing focused that control: focus now returns to the element.Current specification
spec:AST-107FR5, FR6 and FR8;component:ContextMenuFR10;spec:AST-094FR33 and FR43.Public delta and caller burden
New exports:
createContextMenuand its types, anduseContextMenufrom the root and/hooks. The element needs a tab stop for Shift+F10. Size (gzip): ContextMenu JS 177,734 B, CSS 15,189 B; it includesreact-dom/client, which the factory's own menu root imports. Public-API gaps (FR7): the lift, its springs and UIKit tables.Design and alternatives
Evidence
The steps below, automated in Chromium and WebKit; unit tests for focus return, the lift on a factory row, the long press's moment, the shadow root, the nested layer, a moved ref and an unmount that logs nothing.
Scope
Testing
In the story "Headless / createContextMenu on a list row":
menuContent), pick "Restore": it acts once and closes.pnpm test:interaction ContextMenu createContextMenuruns these. Actions are paused on the repository, so this head's results are local: unit 15,389, scripts 2,434, docsite 660, interaction 66 (Chromium) and 58 (WebKit), a11y 27; tsc, lint andpnpm check:repoclean.