Repository navigation
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
AI review status for this pull request.
|
There was a problem hiding this comment.
The tap-during-growth fix itself follows spec:AST-094 FR35, but approval waits on an engineering owner settling spec direction this branch carries from its dependencies: where DropdownMenu and MoreMenu sit on a phone (over their trigger here, which spec:AST-094 OQ9 still leaves as an open owner decision, so it needs a governing spec), architecture:layer-runtime INV13 with its Exit copy section, and component:DropdownMenu FR21. An engineering owner records that decision by commenting /approve-spec <full head SHA> on this pull request, and the author may do so as an engineering owner. Independently of that decision:
- Rebase onto main and resolve the
useMenuPress.spec.mdconflict by keeping main's current FR10, FR11, AR5 and DEC-8. - Land the 13 dependency pull requests first, so this one carries only its own 338-line commit rather than 41,237 authored lines.
[Automated review]
| | FR7 | While a press is tracked in a menu that scrolls, a pointer resting within the edge zone of the menu's top or bottom MUST scroll the menu toward that edge and re-read the row under the pointer as the rows move. Where the primary pointer is coarse, a finger or pen follows `spec:AST-094` FR30 instead; where it is fine, a finger or pen keeps this rule (`spec:AST-094` FR19). | `useMenuPress.test.tsx` autoscroll case | verify | | ||
| | FR8 | On a mouse a DropdownMenu trigger MUST open its menu on press-down (button 0, no Control), and a drag from the trigger into the menu that releases over an enabled row MUST act on it. The opening gesture's release MUST act only if the pointer entered the menu or the press lasted the settle time; otherwise nothing acts and the menu stays open. Pressing the trigger of an open menu MUST close it and MUST NOT reopen it in the same gesture. | Proposed DEC-4; `DropdownMenu.test.tsx` press model suite, `menuPressGesture.test.ts` trigger states | proposed; pending owner review | | ||
| | FR9 | Where the primary pointer is coarse, opening a menu from its trigger under a finger and the opening finger's handoff are governed by `spec:AST-094` FR29. Where it is fine, a finger keeps the released behavior (`spec:AST-094` FR19): a tap on the trigger opens the menu through its click, and a press held on the trigger for the long-press delay opens the menu with the finger down, prevents the default of the finger's later `touchmove`s until the gesture ends, and continues under FR1–FR4. | `spec:AST-094` FR29 | as `spec:AST-094` states | | ||
| | FR11 | A pointer that went down elsewhere (a context menu's long press) and is handed to the menu as it opens MUST be the opening press: a finger picks only after UIMenu's handoff distance, counted from the first position the menu sees after it opened (as `spec:AST-094` FR29 counts it), whatever the touch model, and its lift before that acts on nothing and leaves the menu open; a mouse follows FR8's settle rule. | Proposed; `useMenuPress.test.tsx` continuation suite | proposed; pending owner review | |
There was a problem hiding this comment.
Main now carries a current FR11 under DEC-8 (#8451): “Where the primary pointer is coarse, a finger picks only after the handoff distance, counted … as spec:AST-094 FR33 counts it”. This proposed row drops that condition, counts the handoff as FR29 does, and makes this file conflict with main in three hunks. Rebasing onto main's FR10, FR11, AR5 and DEC-8 resolves it; continueGesture, which has no caller at this head, would then need to meet the current FR11.
There was a problem hiding this comment.
Resolved by the dependency merge in this push: the module record is main's again (#8089 now carries FR10, FR11, AR5 and DEC-8 as main has them), so this file matches main and no longer conflicts. continueGesture meets the current FR11: where the primary pointer is coarse a handed-off finger picks only after the handoff distance, counted from the first position the menu sees; main's cases for it pass at this head.
There was a problem hiding this comment.
The tap-during-growth fix itself follows spec:AST-094 FR35, but approval waits on an engineering owner settling spec direction this branch carries from its dependencies: where DropdownMenu and MoreMenu sit on a compact touch screen (over their trigger here, which spec:AST-094 OQ9 still leaves as an open owner decision, so it needs a governing spec), architecture:layer-runtime INV13 with its Exit copy section, and component:DropdownMenu FR21. An engineering owner records that decision by commenting /approve-spec <full head SHA> on this pull request, and the author may do so as an engineering owner. Independently of that decision:
- Land the eight dependency pull requests first and rebase, so this one carries only its own 338-line commit rather than 34,904 authored lines.
- Add a pinch during the menu's growth to the FR35 browser cases, which FR35's verification row lists.
[Automated review]
| await expect(story.trigger).toHaveAttribute('aria-expanded', 'false'); | ||
| }); | ||
|
|
||
| test('a pan that starts while the menu grows starts nothing: no scroll takes it, and the menu stays (spec:AST-094 FR35)', async ({ |
There was a problem hiding this comment.
spec:AST-094's verification row for FR34 and FR35 also lists a pinch during the menu's growth (a blocked pinch fails). The only pinch at this head is the generic hold's in presentationHold.interaction.browser.spec.ts, over a held popover and dialog; a pinch case beside this one would cover the claim on the menu itself.
There was a problem hiding this comment.
The tap-during-growth fix itself follows spec:AST-094 FR35, but approval waits on an engineering owner settling spec direction this branch carries from its dependencies: where DropdownMenu and MoreMenu sit on a compact touch screen (over their trigger here, which spec:AST-094 OQ9 still leaves as an open owner decision, so it needs a governing spec), architecture:layer-runtime INV13 with its Exit copy section, and component:DropdownMenu FR21. An engineering owner records that decision by commenting /approve-spec with the full head SHA on this pull request, and the author may do so as an engineering owner. Independently of that decision:
- Resolve the merge conflict with main in three
internal/interaction-harnessfiles (main changed them in #8505), so the pull request can merge and CI can run. - Land the eight dependency pull requests first and rebase, so this one carries only its own 338-line commit rather than 33,166 authored lines.
- Add a pinch during the menu's growth to the FR35 browser cases, which FR35's verification row lists.
[Automated review]
| '@astryxdesign/core': patch | ||
| --- | ||
|
|
||
| [chore] Internal: one presentation hold for every layer (`Layer/presentationHold`). While a layer presents, a touch, a press or a click anywhere reaches nothing (keys and assistive technology still act, and a pinch still zooms the page); while it dismisses, the same, and the layer's content is inert. The gesture host holds the layer's touches through its touch gates, and `isPresentationShield` lets a hit test see past a hold's shield. Not exported from the package entry point; no component uses it yet. |
There was a problem hiding this comment.
At this head DropdownMenu uses the hold (packages/core/src/DropdownMenu/menuSurfaceMotion.ts:289), and .changeset/dropdownmenu-presentation-hold.md announces that fix, so released together the changelog would say both. "no component uses it yet" holds only if #8342 releases first; otherwise drop the clause.
There was a problem hiding this comment.
The tap-during-growth fix itself follows spec:AST-094 FR35, but approval waits on an engineering owner settling spec direction this branch carries from its dependencies: where DropdownMenu and MoreMenu sit on a compact touch screen (over their trigger here, which spec:AST-094 OQ9 still leaves as an open owner decision, so it needs a governing spec), architecture:layer-runtime INV13 with its Exit copy section, and component:DropdownMenu FR21. An engineering owner records that decision by commenting /approve-spec with the full head SHA on this pull request, and the author may do so as an engineering owner. Independently of that decision:
- Land the dependency pull requests first and rebase, so this one carries only its own 340-line change rather than 32,310 authored lines.
- Add a pinch during the menu's growth to the FR35 browser cases, which FR35's verification row lists.
- Drop "no component uses it yet" from
.changeset/layer-input-hold.mdunless #8342 releases first, and update the layer-runtime record's Current gaps, which still say no exit copy exists.
[Automated review]
| await expect(story.trigger).toHaveAttribute('aria-expanded', 'false'); | ||
| }); | ||
|
|
||
| test('a pan that starts while the menu grows starts nothing: no scroll takes it, and the menu stays (spec:AST-094 FR35)', async ({ |
There was a problem hiding this comment.
spec:AST-094's verification row for FR34 and FR35 also lists a pinch during the menu's growth (a blocked pinch fails). The only pinch at this head is the generic hold's in inputHold.interaction.browser.spec.ts, over a held popover and dialog; a pinch case beside this one would cover the claim on the menu itself.
| '@astryxdesign/core': patch | ||
| --- | ||
|
|
||
| [chore] Internal: one input hold for every layer (`Layer/inputHold`). While a layer presents, a touch, a press or a click anywhere reaches nothing (keys and assistive technology still act, and a pinch still zooms the page); while it dismisses, the same, and the layer's content is inert. The gesture host holds the layer's touches through its touch gates, and `isInputShield` lets a hit test see past a hold's shield. Not exported from the package entry point; no component uses it yet. |
There was a problem hiding this comment.
At this head DropdownMenu uses the hold (packages/core/src/DropdownMenu/menuSurfaceMotion.ts:289), and .changeset/dropdownmenu-presentation-hold.md announces that fix, so released together the changelog would say both. "no component uses it yet" holds only if #8342 releases first; otherwise drop the clause.
There was a problem hiding this comment.
The tap-during-growth fix itself follows spec:AST-094 FR35, but approval waits on an engineering owner settling spec direction this branch carries from its dependencies: where DropdownMenu and MoreMenu sit on a compact touch screen (over their trigger here, which spec:AST-094 OQ9 still leaves as an open owner decision), architecture:layer-runtime INV13 with its Exit copy section, and component:DropdownMenu FR21. An engineering owner records that decision by commenting /approve-spec with the full head SHA on this pull request, and the author may do so as an engineering owner. Independently of that decision:
- Land the dependency pull requests first and rebase, so this one carries only its own 340-line change rather than 33,141 authored lines at older versions of those branches, whose newer commits already fix the exit copy's form ownership and the FR29 wording in the DropdownMenu usage doc.
- Keep
pan-xfor a screen-edge pan from two edges and claim only inside the edge band, as the current FR18 and DEC-13 require, where the gesture foundation lands. - Add a pinch during the menu's growth to the FR35 browser cases, which FR35's verification row lists.
- Drop “no component uses it yet” from
.changeset/layer-input-hold.mdunless #8342 releases first, and update the layer-runtime record's Current gaps, which still say no exit copy exists.
[Automated review]
| // Out of every form and radio group: a cloned checked radio with the live | ||
| // group's name would take its selection, and a named field would submit. | ||
| for (const node of copy.querySelectorAll('[name], [form]')) { | ||
| node.removeAttribute('name'); | ||
| node.removeAttribute('form'); | ||
| } |
There was a problem hiding this comment.
Removing name and form leaves each cloned control owned by its nearest ancestor <form>, and the copy is inserted right after the layer (:237), so in a layer rendered inside a form the copy's controls join that form while the close plays. INV13 and this file's header say the copy has no form owner. #8186's branch has since fixed this (every cloned control gets form=""); a rebase onto it brings the fix.
| anatomy, | ||
| description: | ||
| 'A dropdown menu that displays a list of actionable items in a popup triggered by a button. Use to present action options as a next step in a process, or to offer contextual actions without cluttering the interface. Like the menus of macOS and iOS, the row under the pointer when it is released is the row that acts, the highlight follows a held mouse or finger, a mouse opens it on press and can drag straight into it, and a finger held on the trigger opens it with the finger down.', | ||
| 'A dropdown menu that displays a list of actionable items in a popup triggered by a button. Use to present action options as a next step in a process, or to offer contextual actions without cluttering the interface. Like the menus of macOS and iOS, the row under the pointer when it is released is the row that acts, the highlight follows a held mouse or finger, a mouse opens it on press and can drag straight into it, and a finger held on the trigger opens it with the finger down. Under a finger it behaves like UIMenu on iOS: a finger resting on the trigger for 150 ms opens the menu, the finger that opened it picks a row only once it has moved 10 px (so the row that appears under it is never picked by accident), the menu grows out of its trigger and collapses back into it on UIKit\u2019s springs, and a pan that starts outside it dismisses it as the page scrolls. On a phone the popover menu sits over its trigger as UIMenu does: centered on it, top-aligned with it (bottom-aligned when the trigger is in the lower half of the viewport), 8 px from the viewport\u2019s sides.', |
There was a problem hiding this comment.
“A finger resting on the trigger for 150 ms opens the menu” contradicts spec:AST-094 FR29: a trigger in a fixed bar opens as the finger presses, and only a trigger inside a scroll view waits for the 150 ms content-touch delay, as this file's presentation row already says. #8089's branch has since corrected the sentence.
| override get nativeTouchBehavior(): NativeTouchBehavior { | ||
| if (this.edges.length === 1) { | ||
| return ALL_NATIVE_TOUCH; | ||
| } | ||
| const horizontal = this.edges.some( | ||
| edge => edge !== 'top' && edge !== 'bottom', | ||
| ); | ||
| const vertical = this.edges.some( | ||
| edge => edge === 'top' || edge === 'bottom', | ||
| ); | ||
| return { | ||
| panX: !horizontal, | ||
| panY: !vertical, | ||
| pinchZoom: true, | ||
| doubleTapZoom: false, | ||
| }; |
There was a problem hiding this comment.
With two horizontal edges this reports panX: false, so the element's derived touch-action drops pan-x and a horizontal scroll that starts outside the edge band is blocked. The current spec:AST-094 FR18 says a screen-edge pan claims pan-x only for a touch that lands in the band, and DEC-13 rejects claiming pan-x across the whole element.
PR Analysis Report
Modified ComponentsContextMenu (@astryxdesign/core)
MoreMenu (@astryxdesign/core)
Component Size
Sizes are gzip. Import counts the JavaScript the import loads eagerly; CSS counts the package stylesheet rules referenced by the modules it can load from its own package, lazily loaded ones included.
|
| Component | JavaScript | CSS |
|---|---|---|
| core/DropdownMenu | +30.1KB |
0B |
| core/MoreMenu | +30.1KB |
0B |
Bundle Size Summary
Gzip figures measure each package's entry file, not the code an import pulls in.
| Package | Entry (ESM) | Entry gzip before | Entry gzip after | Delta |
|---|---|---|---|---|
| @astryxdesign/core | 4.9KB | 1.2KB | 1.2KB | 0B |
| Package stylesheet | Gzip before | Gzip after | Delta |
|---|---|---|---|
| @astryxdesign/core | 34.6KB | 34.6KB | 0B |
Accessibility Audit
Status: No accessibility violations detected.
Visual Regression
55 of 502 shot(s) changed.
A change here is a question, not a failure: check whether the after is the
picture you intended. Record that review on the PR. Baseline maintenance is an
explicit dispatch of CI; this report never rewrites the baseline or adds a release gate.
| component | story | theme | mode | pixels |
|---|---|---|---|---|
| Selector | Indicator Space Bottom Sheet Narrow End RTL | neutral | light | 753,933 |
| Selector | Indicator Space Bottom Sheet Wide Start | neutral | light | 740,072 |
| RichTextEditor | Markdown parity: side by side | probe | dark | 558,840 |
| RichTextEditor | Markdown parity: side by side | probe | light | 540,756 |
| Chat | Full AI Chat | probe | light | 230,823 |
| Chat | Full AI Chat | probe | dark | 229,805 |
| AppShell | With Mobile Nav | probe | light | 217,010 |
| AppShell | With Mobile Nav | probe | dark | 216,960 |
| Collapsible | Dividers | probe | dark | 66,445 |
| Collapsible | Dividers | probe | light | 66,445 |
| AppShell | Playground | neutral | dark | 65,198 |
| AppShell | Playground | neutral | light | 63,788 |
| Chat | Default | neutral | dark | 36,304 |
| Chat | Default | neutral | light | 35,767 |
| Chat | Empty State | probe | light | 23,531 |
| Chat | Empty State | probe | dark | 22,622 |
| ChatAutoScroll | Scroll Behavior Comparison | neutral | dark | 15,378 |
| ChatAutoScroll | Scroll Behavior Comparison | neutral | light | 14,107 |
| Chat | With Attachments | probe | light | 13,406 |
| Chat | With Attachments | probe | dark | 11,232 |
and 35 more.
Generated by PR Enrichment workflow | View full report
There was a problem hiding this comment.
The tap-during-growth fix itself follows spec:AST-094 FR35, but approval waits on an engineering owner settling spec direction this branch carries from its dependencies: where DropdownMenu and MoreMenu sit on a compact touch screen (over their trigger here, which spec:AST-094 OQ9 still leaves as an open owner decision), architecture:layer-runtime INV13 with its Exit copy section, and component:DropdownMenu FR21. An engineering owner records that decision by commenting /approve-spec with the full head SHA on this pull request, and the author may do so as an engineering owner. Independently of that decision:
- Land the dependency pull requests first and rebase, so this one carries only its own two commits rather than 33,806 authored lines at older versions of those branches, whose newer commits already fix the exit copy's form ownership, the FR29 wording in the DropdownMenu usage doc, the two-edge screen-edge pan and the layer-runtime record's Current gaps.
- Route Lab's access to core's motion and gesture modules through
@astryxdesign/core/internal/touch, asarchitecture:react-component-runtimeINV13 now states, or drop that plumbing until Lab needs it. - Leave the action-group and
perEventPointerfine-pointer paths in the gesture host to their own pull request, sincespec:AST-094FR19 does not allow them yet. - Add a pinch during the menu's growth to the FR35 browser cases, which FR35's verification row lists, and drop “no component uses it yet” from
.changeset/layer-input-hold.mdunless #8342 releases first.
[Automated review]
| const CORE_INTERNAL_SPECIFIER = | ||
| /^@astryxdesign\/core\/((?:motion|gestures)(?:\/[\w.-]+)*|utils\/touchPresentation)$/; |
There was a problem hiding this comment.
architecture:react-component-runtime INV13, current on main, says Core and Lab reach the shared touch modules through @astryxdesign/core/internal/touch, limited by lint to Core and Lab. This specifier makes @astryxdesign/core/motion/*, gestures and utils/touchPresentation Lab's route instead, paths core's exports do not list, so a published Lab build importing them would fail in a consumer's bundler. No Lab file imports them yet; dropping this plumbing until Lab needs a module, then using ./internal/touch, would match the record.
| const needsGroupListener = | ||
| registration.actionGroup && | ||
| !this.isPrimaryPointerCoarse() && | ||
| (registration.recognizers.some(r => r.isEnabled) || | ||
| (registration.consumer != null && | ||
| registration.consumer.isEnabled?.() !== false)); | ||
| if (needsGroupListener && registration.groupDownListener == null) { | ||
| const listener: EventListener = event => | ||
| this.beginGroupPress(event as PointerEvent, registration); | ||
| registration.groupDownListener = listener; | ||
| element.addEventListener('pointerdown', listener, {passive: true}); |
There was a problem hiding this comment.
Under a fine primary pointer this registers a pointerdown listener and serves the group's recognizers, while spec:AST-094 FR19 says “Where the primary pointer is fine, no recognizer attaches and no touch listener is registered”, and family:action-groups, which the comment cites, does not exist on main. Nothing at this head passes actionGroup: true; this path belongs with #8072 and the amendment it waits on.
… that content The presented hit test takes the document's elementFromPoint, which answers a shadow host for anything inside its open root, and compares with Node.contains, which stops at the boundary: a recognizer on a control inside a moving sheet's shadow root gets no finger, and the tap's click goes to the host. Fails until the hit test descends into open roots and compares along the flat tree.
Where a moving element is drawn away from its layout box, the host hit-tests what is drawn under the finger with the document's elementFromPoint, which answers a shadow host for anything inside its open root. The hit test now descends through each open root's own elementFromPoint and compares with the movers along the flat tree, so a recognizer on a control inside a moving sheet's shadow root gets its finger and the tap clicks that control.
…nt lets go The host's touch-action rules sit in an adopted sheet or a style element per document or shadow root, and were never removed, so a page whose last recognizer detached kept them, against spec:AST-094 FR24 (nothing touch-related remains after the last instance unmounts). Fails until a root's rules go with its last marked element.
…th its last marked element The touch-action writer records which elements it marked in each document or shadow root, and when the last one is unmarked (its last recognizer detached) it removes that root's adopted sheet or style element, so nothing touch-related remains after the last instance unmounts (spec:AST-094 FR24). A later mark writes the rules again; an element marked in another root first lets go of the old one.
… the inline style it reads - The motion registry, the host's engine gate and the README name the presented hit test as an engine workaround (spec:AST-013 FR6) with its review condition (re-measure on each iOS release; remove it once iOS WebKit hit-tests a compositor-moved element where it is drawn) and its regression fixture. - While a curve runs the animator holds the value it starts from inline, so presentation.ts and the README no longer say the inline style holds the end.
…core/internal/touch Core ships the internal entry @astryxdesign/core/internal/touch, which re-exports the gesture primitives for Astryx's own packages, and the Lab-import guard fails a Lab import of core outside core's exports (architecture:react-component-runtime INV13). The README and the barrel's header named internal workspace imports and said nothing here is exported; they now name that entry.
…ill needs the element, and the listeners that cancel The release note, the stream's header, the host's comment and the README said the stream is one passive listener per event type and that a document-capture stopPropagation no longer starves the recognizers. Such a stop never hid a listener on the same node, and some subscriptions cancel. They now say what changed: an app's window-capture stopPropagation, or a document-capture stopImmediatePropagation registered before core, no longer hides a touch; a recognizer that must claim still needs the touch to reach its element; the gesture host's touchend, click and contextmenu cancel while a claimer is attached and the page hold's touchmove while it holds; and the stream stays ahead of an app's window-capture listener only until a type's listener is re-added. The README's list of blocking touchmove listeners gains the claimed touch's target and the document while a claiming element moves.
…default it moves
The {stylexOptions} form of astryxStylex now passes StyleX
treeshakeCompensation: true unless the caller sets it. spec:AST-017 FR3
asks the release note to name the old and new default and the option
that restores the old one; it now does.
…er and for the two stops the input stream fixes - A still finger with sub-pixel jitter over a scroll view keeps its tap and scrolls nothing (spec:AST-094 FR18–FR22). - A tap is still recognized under an app's window-capture stopPropagation, and under a document-capture stopImmediatePropagation registered before core's: the recognizers hear touches on the window's capture phase. Chromium and WebKit pass.
…s once Where nothing that claims is attached, the host's touchend listener is passive and cannot cancel the engine's own click after a retargeted tap clicks what is drawn. The shared click guard swallows that late click whatever is attached; this case pins it, as asked on meta#8293.
meta#8360 carries this pull request's predecessors and the review fixes of the foundation it holds; this pull request's own diff is now its six commits (a second finger fails a one-finger pan wherever fingers come from, and the check reruns once that finger is delivered; a press's selection lock gets what it landed on; a pinch's second finger on another pan claims neither; the wider-band edge pan's real-touch proof; text). Two conflicts: - GestureHost.ts: the selection lock's target takes meta#8360's presented target, which is what the press landed on unless an element under it is moving, so it keeps this pull request's fix and meta#8293's correction. - .changeset/gesture-press-tracking.md: meta#8360's text, which names both writers of the press attribute.
This pull request sat on meta#8076 and carried its old copies of the harness meta#8045 and meta#8046 landed on main. meta#8360 carries meta#8076 with main's harness, so after the merge this pull request's own diff is the presentation resolver's commits. The one conflict, the gestures README, keeps both sides: this pull request's paragraph on the presentation prop ends the touch presentation section, and meta#8360's migration section follows it.
…esture host's subscriptions The gesture host subscribes no click: the one cancelling click subscriber is the click guard's, added at a window's first arm and kept. The release note, the stream's header and the gestures README listed click with the host's touchend and contextmenu; they now name the guard's own.
… as a dependency meta#8627 is now one commit on main; meta#8360 carries meta#8076, meta#8073 and meta#8446. useMenuPress.ts: the trigger half lives in menuTriggerPress.ts, so it takes meta#8360's trigger changes there. The trigger's holds run on the long-press detector and pass FR29's 13 px drift allowance as each press starts, under every primary pointer (FR43). The hold duration comes from motion/uikit/longPress.ts. MENU_PRESS_TRIGGER_DRIFT_PX is re-exported beside MENU_PRESS_LONG_PRESS_MS. The stray-click window is the shared click guard's, from meta#8360.
meta#8089 now takes meta#8080, which takes meta#8078 (with meta#8627, meta#8360 and meta#8298) and meta#8186. The DropdownMenu spec's FR5 row takes meta#8080's list (FR26–FR32, FR34 and FR35) and its state names FR35 verified by this PR; FR28's cancel clear and FR34 stay pending on this line. Prettier re-pads the table.
There was a problem hiding this comment.
The tap-during-growth fix follows spec:AST-094 FR35, but approval waits on an engineering owner deciding two spec edits this branch carries without approval: architecture:layer-runtime INV13 with its Exit copy section (from #8186), and the spec:AST-057 change that puts the row swipe's slide and spring motion on theme motion roles. An engineering owner records that decision by commenting /approve-spec with the full head SHA on this pull request, and the author may do so as an engineering owner; otherwise those edits settle in their own pull requests and this branch is rebased without them. Independently of that decision:
- Land the dependency pull requests first and rebase, so this one carries its own 407 lines instead of 35,586 authored lines from nine other open pull requests.
- Conform the gesture host to the current every-pointer rule (
architecture:react-component-runtimeINV13,spec:AST-094FR19 and FR43); it still attaches only where the primary pointer is coarse. - Hold only for a menu a finger or pen opened: where the primary pointer is coarse, a mouse click during the grow is swallowed, though FR35 says a mouse keeps today's behavior.
- Land the touch-support bundle test change (#8671) first or carry it, since main's
scripts/touch-support-bundle.test.mjsfails at this head. - Fix
.changeset/layer-input-hold.md("no component uses it yet"), drop.changeset/dropdownmenu-uimenu-placement.md, and add a pinch during the menu's growth to the FR35 browser cases.
[Automated review]
| // layer's own entrance). | ||
| const surface = contentRef.current; | ||
| isTouchPresentationRef.current = touchModes === true; | ||
| const isUikitMotion = touchModes === true; |
There was a problem hiding this comment.
This keys the grow, and with it the FR35 hold, on touchModes, a read of the primary pointer (utils/presentation.ts:106), not on the pointer that opened the menu. On a device whose primary pointer is coarse, a menu opened with a mouse or trackpad grows and its shield swallows a click for the length of the open spring, while spec:AST-094 FR35 ends “A mouse keeps today's behavior (DEC-16)”. Holding only for an open a finger or pen drove, plus a mouse case under a coarse primary pointer, would cover it.
| * | ||
| * ONLY WHERE THE PRIMARY POINTER IS COARSE | ||
| * | ||
| * - Recognizers attach, and the host registers listeners, only while the |
There was a problem hiding this comment.
architecture:react-component-runtime INV13 on main now says “The gesture host attaches for every pointer, with no touch-only query; under a fine pointer only a component that uses a gesture listens”, and spec:AST-094 FR19 and FR43 say recognizers attach for every pointer. This host still installs nothing where the primary pointer is fine (and warns), so it needs to conform before it lands, here or through #8076.
| '@astryxdesign/core': patch | ||
| --- | ||
|
|
||
| [chore] Internal: one input hold for every layer (`Layer/inputHold`). While a layer presents, a touch, a press or a click anywhere reaches nothing (keys and assistive technology still act, and a pinch still zooms the page); while it dismisses, the same, and the layer's content is inert. The gesture host holds the layer's touches through its touch gates, and `isInputShield` lets a hit test see past a hold's shield. Not exported from the package entry point; no component uses it yet. |
There was a problem hiding this comment.
At this head DropdownMenu holds its layer with this (menuSurfaceMotion.ts:296) and .changeset/dropdownmenu-presentation-hold.md announces that fix, so released together the changelog contradicts itself; drop “no component uses it yet”, or let #8342 release first.
| '@astryxdesign/core': patch | ||
| --- | ||
|
|
||
| [fix] DropdownMenu and MoreMenu opened with a mouse no longer start the touch presentation's motion code; on a phone the menu keeps opening beside its trigger, as on a larger screen. |
There was a problem hiding this comment.
Against main this release note describes no change: no release started the touch motion for a mouse open, and a phone menu already opens beside its trigger. It belongs with #8089 and can be dropped here.
| - **Gesture tuning.** Axis-lock distance, commit ratio, fling velocity, | ||
| resistance past the panel: behavior constants in source, not public API and | ||
| not design tokens (the same ruling the touch press model received for its | ||
| clocks). The row's slide and spring motion plays on theme motion roles |
There was a problem hiding this comment.
Main's AST-057 calls the row's slide and spring durations behavior constants that are not design tokens; this line moves them onto theme motion roles. It comes from a commit of #8378, which closed unmerged, and no file in this branch changes the row swipe's code, so an engineering owner approves it for this head or it settles in its own pull request.
| : `${active.tagName}#${active.id}[${active.getAttribute('role') ?? ''}]`; | ||
| }); | ||
|
|
||
| test('a tap that lands while the menu grows is ignored: it finishes, stays open and keeps focus; once it is in, a tap outside dismisses it (spec:AST-094 FR35)', async ({ |
There was a problem hiding this comment.
spec:AST-094's FR34–FR35 verification row also lists a pinch during growth, and DropdownMenu.spec.md FR5 now marks FR35 verified; a pinch beside this tap case would prove the page still zooms over the growing menu, where today only MenuPinchZoom.a11y.chromium.spec.ts pinches over a settled menu.
Depends on #8089, #8342. Review only this PR's change: vjeux/astryx@d336441...7ffa3cb
User impact
On a phone, a second tap that lands while a DropdownMenu is still growing open closes it halfway, and focus falls to the page. iOS ignores input until a menu is presented: the menu finishes opening and stays.
What led to it: comparing our menus with UIKit's on the iOS 26.2 simulator (
spec:AST-094FR35).Problem and solution fit
For the length of the open spring, the menu holds its layer with the layer runtime's presentation hold (#8342): a transparent shield over the viewport catches the tap, which closes nothing, scrolls nothing, moves no focus and clicks nothing. A pinch still zooms; keys and assistive technology stay live. The finger that opened the menu still slides onto a row: the press model looks through the hold's shield, and only it.
Expected behavior and authority
spec:AST-094FR35 (current): a tap or drag that lands while an anchored menu is entering must not close it, scroll the page or move focus.component:DropdownMenuFR5 points there.Smallest restoration
Two places change: the menu's motion holds around its open spring, and the press model skips the shield. A mouse and
presentation="popover"hold nothing.Evidence
iOS 26.2 simulator, Safari, a second tap outside about 200 ms into the open, before and after:
Android Chromium 158 emulator (a dot marks each finger), a second tap about 150 ms in, then one once the menu is in:
Before, the tap closed the menu and moved focus to the page; after, the menu finished and stayed, and a later tap outside dismissed it.
Writing rubric results
R1 narration: none. R2 research: none. R3 evidence: none in the record. R4 rejected alternatives: none. R5 change references: none. R6 open questions: none added.
Scope
Testing
On the DropdownMenu "Default" story, with touch emulation:
Automated: the hold's unit tests and
DropdownMenuTouchMotion(Chromium, WebKit), which fail without the change. The drill-in accessibility spec now counts only popovers a person can perceive (the shield sits in the top layer while the menu enters). Main'stouch-support-bundletest fails here; #8671 updates it.Stack (merge in this order): #8360, #8298 and #8627 (any order) → #8671 → #8078 and #8186 (any order) → #8080 → #8089; #8342; then this PR.