Repository navigation
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
AI review status for this pull request.
|
github-actions
Bot
added
the
needs:spec-owner-review
meta-cla
Bot
added
the
CLA Signed
![astracat-bot[bot]](https://avatars.githubusercontent.com/in/4796231?s=60&v=4){kind=small}
There was a problem hiding this comment.
This needs an engineering owner's decision before approval: no current spec places DropdownMenu over its trigger on phones, and this head adds the draft spec:AST-094 plus a proposed touchOpen decision (DEC-8) to the useMenuPress module. An engineering owner can approve that direction, including whether the menu's 8 px side margin replaces the layer runtime's viewport gutter, by commenting /approve-spec <full head SHA> on this pull request; the author can do so as an engineering owner. Whatever the decision, these changes are still needed:
- Base this branch on #8080, or land the stack first, so the diff is this PR's own commit; it currently carries 26,562 authored lines.
- Keep the covering menu inside the viewport: in landscape, a 300 px menu opened from a mid-screen trigger runs past the bottom edge, and its last rows cannot be reached.
- Either document that
placementandalignmentare ignored on phones or honor them, and attach the iOS Safari capture the UIMenu claim rests on.
[Automated review]
| const upward = box.top + box.height / 2 > height * uikitMenu.openUpwardBelow; | ||
| style.setProperty('top', upward ? 'auto' : 'anchor(top)'); | ||
| style.setProperty('bottom', upward ? 'anchor(bottom)' : 'auto'); |
There was a problem hiding this comment.
This pins one edge of the menu to the trigger and leaves the other edge unbounded. In custom mode the layer also emits no fallbacks, gutter or viewport caps, so on a short compact viewport (a phone in landscape) a menu at the 300 px cap runs past the viewport and its last rows cannot be reached. With this CSS on a stand-in at 844 x 390, the menu spans y 161-461. Cap the block size to the room between the pinned edge and the far gutter, or shift the box inward the way UIMenu does.
There was a problem hiding this comment.
Fixed at a6dbadb: as the menu opens, its block size is capped to the room between its pinned edge and the far viewport edge less the margin (and the menu's own cap still holds), so it stays on screen and scrolls, as UIMenu does. New Chromium case at 844×390: the long menu stays inside 8 px of both edges and scrolls.
| coversTrigger | ||
| ? { | ||
| positioning: 'custom', | ||
| style: COVER_TRIGGER_STYLE, |
There was a problem hiding this comment.
On compact touch this branch drops placement and alignment, so a caller's explicit placement="above" stops applying on phones, while the props' JSDoc still says only the bottom sheet ignores them. Either document the override or keep an explicitly given placement. The fixed 8 px left/right in COVER_TRIGGER_STYLE is also a second viewport gutter, which architecture:layer-runtime INV11 reserves to the runtime, and it skips env(safe-area-inset-*) and LayerProvider.inset.
There was a problem hiding this comment.
Fixed at a6dbadb: the placement and alignment JSDoc and doc.mjs entries say a compact touch screen places the menu over its trigger, as UIMenu does. The side inset is now max(8px, env(safe-area-inset-*)) plus the LayerProvider inset for that edge, the runtime gutter’s own terms. Whether UIKit’s 8 pt or the runtime’s 16 px base applies here is listed for the owner decision.
PR Analysis Report📚 Storybook Preview🧪 Sandbox PreviewModified ComponentsContextMenu (@astryxdesign/core) · View in Storybook
MoreMenu (@astryxdesign/core) · View in Storybook
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 | +16.0KB |
0B |
| core/MoreMenu | +16.0KB |
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 | Storybook | Sandbox | View full report
518e4b9 to
a4bb514
Compare
There was a problem hiding this comment.
This needs an engineering owner's decision before approval: no current spec places DropdownMenu over its trigger on phones, and this head adds the draft spec:AST-094 plus a proposed touchOpen decision (DEC-8) to the useMenuPress module. An engineering owner can approve that direction, including whether the menu's 8 px side margin replaces the layer runtime's viewport gutter, by commenting /approve-spec <full head SHA> on this pull request; the author can do so as an engineering owner. Whatever the decision, these changes are still needed:
- Base this branch on #8080, or land the stack first, so the diff is this PR's own commit; it currently carries 28,858 authored lines.
- Keep the covering menu inside the viewport: in landscape, a 300 px menu opened from a mid-screen trigger runs past the bottom edge (or the top, from just below the middle), and its end rows cannot be reached.
- Take the side inset from the runtime's gutter, safe area and
LayerProviderinset included, unless the single-gutter rule is amended in the same decision. - Document that
placementandalignmentare ignored on phones or honor them, and attach the iOS Safari capture the UIMenu claim rests on.
[Automated review]
… platform (#8274) ## Exact decision needed What "compact" means when a component picks a presentation by screen size, and whether it differs by platform. ## Existing authority searched - `spec:AST-012` FR1–FR2: width points (`md` is 768) and a closed `when` vocabulary with no height field. - `architecture:public-component-api` INV14 and change coupling: components keep no parallel hardcoded breakpoint table. - `component:Selector` FR4: `adaptive` uses the sheet on "compact coarse-pointer screens", with no number. - `design:mobile-typography` DR3 uses `(width < 1024px) and (pointer: coarse)` for text sizing, a separate concern this PR leaves alone. - No current record defines compact. Code uses `(max-width: 768px) and (pointer: coarse)`. - Search terms: compact, 744, 768, height 500, size class, adaptive, coarse pointer, breakpoint. Overlapping open pull requests: #8062 (shared touch resolver and its compact query), #8052 (AST-094 resolver), #8015 (NumberInput steppers on coarse pointers; not a size test), #6817 (compact density, a different meaning), #8089, #8104, #8095 (phone presentations that consume the resolver). ## Decision - `spec:AST-012` FR9 / DEC-6: a screen is compact while its width is below the effective `md` point or its height is below 500, with the same numbers on every platform (CSS px on the web, pt on iOS, dp on Android); native does not map compact to platform size classes. The theme exports this one definition; the web touch presentation resolver, Selector's `adaptive` presentation, and `@astryxdesign/native` all use it. - `spec:AST-012` FR2: `when` gains `height`, accepting only `{below: 500}`. - `architecture:public-component-api` INV17: a component that chooses a presentation by screen size uses that definition and keeps no numbers of its own. - `spec:AST-082` FR18: `PortableTheme` carries the compact definition, so native reads the same numbers; FR11 and `spec:AST-093` FR3: a native choice between presentations by screen size uses it, not size classes, while the platform's own control still presents the surface. - Compact governs presentation choice only; a layout width a caller picks, such as AppShell's `mobileNav.breakpoint`, is not compact. - `component:Selector` records the `adaptive` edge change as pending and compatible under `spec:AST-017` FR1. ## Rejected alternatives - Mapping compact to each platform's size classes: the same screen would be compact on one platform and not another. ## Claim scope and non-goals The compact definition and the height condition only. Text sizing under `design:mobile-typography`, which surfaces adopt a touch presentation, and resolver code are outside this PR. ## Evidence and implementation relationship The touch presentation resolver contract lives only in the open `spec:AST-094` revision (#8052), which this PR does not edit; it must use FR9. Native adopts FR9 when it gains a compact resolver. Behavior delta when the code conforms: a coarse-pointer screen exactly 768 wide and at least 500 tall opens the popover, and a landscape phone wider than 768 but under 500 tall opens the bottom sheet. Code follow-up belongs to separate implementation PRs. INV16 is the caller-control invariant from #8273 (merged); this PR adds INV17. No implementation is included. ## Writing rubric results - R1 narration: none. - R2 research in the record: none. - R3 evidence naming a change: none; verification names edge states. - R4 rejected alternatives over one line or outside a decision: one, inside DEC-6, one line. - R5 pull request or issue references remaining: none added. - R6 ruled open questions still listing options: none. ## Scope - [x] This PR contains specification records only. - [x] It records observable behavior and preserves equivalent internal implementations. - [x] Any named internal mechanism is intentionally public, names its dependent caller/system, and explains why an equivalent implementation would not satisfy the contract. - [x] Verification states the evidence layer and failure signal without prescribing CI job/workflow topology. - [x] It records one intentional durable decision, not a rescue for a separable tagalong. - [x] The canonical knowledge owner, affected current claims, and overlapping open work are named; any ownership collision is resolved or called out for human decision. - [x] Adjacent API, visual, accessibility, composition, and implementation facts remain outside scope unless this exact decision depends on them. - [x] Public text and artifacts contain no internal Meta context. ## Validation - `pnpm check:knowledge` (CI lint) - Prettier - `git diff --check`
|
Posted by Cindy's agent on her behalf. Parts of this PR re-implement behavior that already exists, or is being specified, as a shared part of core. Converging keeps one implementation per behavior:
This is about where the behavior lives, not about the PR's intent. |
|
Agreed: one implementation.
|
a4bb514 to
a6dbadb
Compare
github-actions
Bot
added
the
needs:design-review
There was a problem hiding this comment.
This needs an engineering owner's decision before approval: no current spec places DropdownMenu over its trigger on phones, its 8 px side inset is a second gutter definition that architecture:layer-runtime INV11 and spec:AST-059 FR1 reserve to the layer runtime, and this head adds the draft spec:AST-094 plus proposed useMenuPress DEC-8 and FR9 changes. An engineering owner can approve that direction, or name the record that should govern where a phone menu sits, by commenting /approve-spec <full head SHA> on this pull request; the author can do so as an engineering owner. Whatever the decision, these changes are still needed:
- Base this branch on #8080, or land the stack first, so the diff is this PR's own commit; it currently carries 33,746 authored lines.
- In right-to-left apps, add each
LayerProviderinline inset to the edge it names:COVER_TRIGGER_STYLEaddsinlineStartto the left, so the menu runs under a bar declared on the right. - Drop the theme
platformcondition or add it tospec:AST-012FR2 first, and makeastryx docs motionand draft AST-094 FR4 match the motion roles that actually ship.
[Automated review]
| left: coverEdge('left', 'inline-start'), | ||
| right: coverEdge('right', 'inline-end'), |
There was a problem hiding this comment.
In a right-to-left app --astryx-layer-inset-inline-start is the bar on the right, but it is added to left here, so the covering menu runs to 8 px from the right edge, under that bar (spec:AST-059 FR6 declares the inset per logical edge). The runtime's positional gutter takes the larger of both inline insets for both sides (layerViewportInset.stylex.ts:52); using that term, or mapping the inline insets to left/right by the menu's direction, keeps the bar clear. An RTL case with a declared inset would pin it.
| * 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 lists the adaptation conditions as width, height, pointer, contrast and motion and says future fields need an explicit addition to that closed vocabulary. platform needs that amendment first, or should leave this stack.
| content: [ | ||
| { | ||
| type: 'prose', | ||
| text: 'On a touch screen, sheets, alerts, menus, navigation and scrolling play the springs, durations and curves measured from iOS. Each value is a motion role named by its table path, such as `springs.sheetPresent` or `timing.curves.easeInOut`. A theme retunes a role through `motion.springs`, `motion.durations` or `motion.curves`, at its root or in an adaptation rule; a role it leaves alone keeps its measured default.', |
There was a problem hiding this comment.
At this head only DropdownMenu's menuSurfaceMotion.ts plays motion roles, and draft AST-094 keeps scrolling native, so a theme that sets springs.sheetPresent or springs.navigationPush (the example below) changes nothing. Describe the roles that ship, or land this section with the components that read them.
| 'options.getScroller': | ||
| 'element to edge-autoscroll while tracking; default = menu root if it overflows.', | ||
| 'options.longPressDelayMs': 'finger hold before open, ms.', | ||
| 'options.longPressDelayMs': | ||
| 'finger hold before open, ms (iOS: 150, UIKit; elsewhere 500). iOS inside a scroller: +150 (content-touch delay), a caller value too.', |
There was a problem hiding this comment.
The code holds 150 ms on every touch device and adds the scroller delay everywhere (MENU_PRESS_LONG_PRESS_MS = menu.holdOpenMs), as the full doc above says; "elsewhere 500" and "iOS inside a scroller" tell an agent reading --dense otherwise.
| measures (0.55 for a scroll view and a sheet), and a release MUST pick its | ||
| rest point by projecting its velocity the way its UIKit counterpart does: | ||
| with a deceleration rate, or with a measured projection time. | ||
| - **FR4 — Physics are not themable in touch presentations.** A touch |
There was a problem hiding this comment.
This head also ships theme motion roles that retune every touch spring, duration and curve (motion/roles.ts, the touch-motion-theme-roles changeset), which FR4 and DEC-1 forbid. One of the two needs to change before an owner can approve both.
| follows. | ||
| - **FR7 — Exits animate as UIKit's do.** A touch presentation MUST play the | ||
| exit its UIKit counterpart plays. **Proposed; pending owner review:** this | ||
| departs from the draft `spec:AST-090` FR1 (exits off by default) and FR10 |
There was a problem hiding this comment.
No AST-090 record exists at this head or on main, so this amendment (and the one in Current-state impact) points at nothing; cite the record by a path that exists, or drop it until that record lands.
a6dbadb to
206b0b1
Compare
Strict ESLint reported six unused bindings in files this pull request carries: the host's HAPTIC_SWITCH_ATTRIBUTE import, two unused imports in the pan's tests (scrollProbe, ScreenEdgePanGestureRecognizer), the swipe test's drift, the presented-hit-testing test's gestures, and a GestureHost test's before, which now asserts what it was read for: a second finger's moves leave the pan where it was.
…eal angular speed The release velocity of a rotation whose listener sets rotation = 0 on each change, at 2 degrees every 16 ms, read 4.363 rad/s against the 2.182 rad/s of the same stream without a re-base: the test asserted only more than 1.5. It now asserts the 2.18 rad/s (spec:AST-100 FR2), and fails.
The rotation's velocity tracker samples the raw unwrapped angle, which setting rotation does not move, so the setter's shift of the samples read each re-base as extra turn and doubled the release velocity of a listener that re-bases on every change. The setter no longer touches the tracker.
…mates while it runs The proof read the element's animations 230 ms in, after the dip had ended, and asserted only that every animated key is opacity, which an empty list passes: with the cross-fade removed from the plan (the move still landing), the proof still passed. It now reads the keyframe keys right after the call starts and asserts they include opacity and nothing else; the same mutant fails it. Chromium and WebKit pass. (F1's in-flight read from c648034.)
…roller, and its claimed moves are cancelled under pointer events Two comparisons stop at the shadow boundary, each failing a case: - once the host's listener on a recognizer's inner node has seen the finger, isFingerDownIn reads it as outside a light-DOM scroller around that root, so a scroll under it is not the finger's (FR20); - under inputMode 'pointer-events', a claimed pan on a shadow host cancels none of its finger's moves over the inner content, since the finger's target is the inner node and its touches reach the host retargeted (FR19).
…the flat tree isFingerDownIn and the pointer-events finger match compare with containsAlongFlatTree, as the scroll watch already does: a finger on content inside an open shadow root is down in the scroller around that root, and its touches, retargeted to the shadow host, match its pointer finger, so a claimed pan cancels its moves there (spec:AST-094 FR19, FR20).
…achments asks, and only then The host let the last attachment decide whether an element is an action group (useGestureRecognizer always passes the option), and never recomputed it at a detach: a plain attachment after the group's turned the group off, and a group's attachment released left a recognizer that never asked taking presses under a fine pointer. Fails until the host counts the attachments that ask.
…ents asks The host counts the attachments that pass actionGroup: true and keeps the element a group while one remains, recomputed at each detach. An attachment that passes false, as useGestureRecognizer does by default, no longer turns a group off, and a group's last asking attachment taking its leave ends the group.
… 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.
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.
There was a problem hiding this comment.
Approval is on hold until an engineering owner decides two record changes at this head: the new layer-runtime INV13 (one exit copy per closing layer, with its Exit copy section and verification rows) and AST-057's new sentence putting a swipe row's slide and spring on theme motion roles. Any 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. Whatever the decision, these need fixing:
- A tap that lands while the menu is still growing closes it (AST-094 FR35), so the input hold from #8342 and #8458 needs to sit under this change.
main'sscripts/touch-support-bundle.test.mjsfails at this head until #8671 lands or its per-component check comes here.- On a touch-primary device a mouse or keyboard open still plays the finger's grow and exit copy (
DropdownMenu.tsxline 1156), while both DropdownMenu changesets say a mouse keeps the layer's entrance. - AST-057's new sentence describes a motion role the row does not play:
Item/useSwipeAction.tsstill settles on a 200 ms constant. - At 34,002 authored lines, the open stack should land first so this diff is its own change.
[Automated review]
| exiting = null; | ||
| }; | ||
|
|
||
| const enter = (measure: () => MenuSurfaceGeometry) => { |
There was a problem hiding this comment.
enter starts the grow and nothing holds pointer input while it runs, so the popover's light dismiss stays live and a tap that lands mid-grow closes the menu. spec:AST-094 FR35 says such a tap "MUST NOT close it"; the layer runtime's input hold (#8342, adopted for DropdownMenu in #8458) needs to be under this change.
| // layer's own entrance). | ||
| const surface = contentRef.current; | ||
| isTouchPresentationRef.current = touchModes === true; | ||
| const isUikitMotion = touchModes === true; |
There was a problem hiding this comment.
touchModes follows the device's primary pointer, so on a touch-primary device a mouse click or a keyboard open grows the menu here and plays the exit-copy shrink on close, while both DropdownMenu changesets say a mouse keeps the layer's entrance. Deciding from the opening input (menuTriggerPress.ts reads event.pointerType at line 209) would make the notes true.
| '@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.
This holds only where the primary pointer is fine: on a touch-primary device a mouse open still starts the grow and the exit copy (DropdownMenu.tsx line 1156).
| it('a finger’s open creates it', async () => { | ||
| stubPointer(true); | ||
| const trigger = renderMenu(); | ||
| await userEvent.setup().click(trigger); |
There was a problem hiding this comment.
userEvent.setup().click drives a mouse pointer, so this case pins a mouse open on a coarse-primary device creating the touch motion. A touch pointer here, plus a mouse case under the coarse stub, would test what the name says.
| correct, and the runtime polls nothing: an anchor that a scrolled | ||
| container hides is followed again when an intersection observer reports it | ||
| back in view. | ||
| - **INV13 — One exit copy per closing layer.** A layer's close plays on at most |
There was a problem hiding this comment.
INV13 is a new normative rule that main does not state (INV14 names the shared exit copy without these rules). The earlier /approve-spec named a previous commit, so an engineering owner approves or rejects it with /approve-spec and this head's full SHA.
| - **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.
The row does not play a motion role: Item/useSwipeAction.ts still settles on SWIPE_SETTLE_MS = 200 (line 86, played at line 368), and this pull request does not touch it. This sentence (and the one under the API table) would describe the row once it moves onto a role; it also needs an engineering owner's approval as new text in a current record.
|
|
||
| import {useCallback, useMemo, type RefObject} from 'react'; | ||
| import {currentGesture} from '../Layer/gestureCounter'; | ||
| import {createNativeScrollProbe} from '../gestures/nativeScroll'; |
There was a problem hiding this comment.
This import puts gestures/nativeScroll.ts behind DropdownMenu and the public useMenuPress, so main's scripts/touch-support-bundle.test.mjs "is absent from the root entry" (and the hooks entry) fails at this head. #8671 updates that check; it needs to land first or come with this change.
Depends on #8080. Review only this PR's change: vjeux/astryx@2d82af7...e01c0d1
The latest pushes only merge main through #8080 (#8515's
DropdownMenu.spec.mdrows, then #8663'smotion/README.md); nothing else changed since the spec approval at 6091b8d.User impact
On a laptop with a touchscreen, a DropdownMenu or MoreMenu opened with the mouse ran the finger's grow-and-collapse motion (#8080), creating it or reusing one a finger's earlier open left. A mouse must never run touch code (
spec:AST-094FR24).What led to it: this pull request first proposed placing a phone menu over its trigger; the engineering owner decided here that it stays beside its trigger, so the placement was removed and this mouse fix remains.
Problem and solution fit
The touch motion is now created and run only for a touch open. A mouse open creates none and stops one a finger's earlier open left. Placement stays the layer's on every screen.
Expected behavior and authority
spec:AST-094FR24 (current): a mouse runs nothing touch-related. The engineering owner's decision here: a phone menu keeps the layer's placement beside its trigger (the record amendment follows).Smallest restoration
Options considered:
Evidence
Scope
Testing
On the DropdownMenu "Default" story:
Automated: the mouse cases in
DropdownMenuTouchMotionScopeandDropdownMenuTouchPlacement(Chromium, WebKit). 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 → this PR.