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:design-review
PR Analysis Report📚 Storybook Preview🧪 Sandbox PreviewModified ComponentsContextMenu (@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/ContextMenu | +39.8KB |
+12B |
| core/Markdown | +40.2KB |
+11B |
| core/Table | +40.0KB |
+13B |
| core/TableCell | +39.9KB |
+13B |
| core/TableHeaderCell | +39.9KB |
+19B |
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
![astracat-bot[bot]](https://avatars.githubusercontent.com/in/4796231?s=60&v=4){kind=small}
There was a problem hiding this comment.
Not ready to approve yet: closing a touch-opened ContextMenu with UIKit's reverse morph is new direction that no current spec covers (component:ContextMenu and spec:AST-094 FR7–FR8 are draft), so an engineering owner needs to approve or reject it, including whether the Reduce Motion exit should fade out in place (FR8's wording) or still travel into the finger as it does now. Any engineering owner, the author included if they are one, can record that decision by commenting /approve-spec <full head SHA> on this pull request. Separately from that decision, please rebase once #8052–#8213 land so this diff is only the final commit, and address these:
- Closing while the menu's entrance is still playing pops it back to full size and opacity before it shrinks, because
dismissMenuends the morph and then measures the layout box. - A second tap on the fading backdrop removes the shrinking menu at once, because
endExit()runs before the check that the menu is still showing. drop()and the touch-close state resets run from an Effect watchingisOpenrather than from the close handler, whicharchitecture:react-component-runtimeINV2 asks for.
[Automated review]
| return; | ||
| } | ||
| // The menu as it shows now (its entrance ends where it is). | ||
| endMorph(); |
There was a problem hiding this comment.
Closing while the entrance is still playing ends the morph here and then measures the layout box, so the exit starts from the full menu at opacity 1: a 97.8 × 94.7 px glass at opacity 0.46 became the 160 × 169 px menu at opacity 1 in a held capture. Starting the exit from the entrance's presented values when a morph is running would keep the close a reversal of the open.
| } | ||
| // The menu as it shows now (its entrance ends where it is). | ||
| endMorph(); | ||
| endExit(); |
There was a problem hiding this comment.
A second backdrop tap during the exit reaches here with the menu already hidden: endExit() removes the shrinking glass and clone before the 0 × 0 check below returns, while the copy keeps returning. Returning before this when the menu isn't showing (or ignoring backdrop taps once the menu has closed) avoids it.
| // A popover reports itself open a task after the press asked (its toggle | ||
| // event), so only an open → closed change ends the touch-opened menu. | ||
| const wasOpenRef = useRef(isOpen); | ||
| useEffect(() => { |
There was a problem hiding this comment.
This Effect now drives the exit's drop() and resets isTouchOpen and liftPlacement in reaction to isOpen. architecture:react-component-runtime INV2 asks for that work in the handler that closes the menu, and the layer's onHide already runs on this close.
…t-frame fixes (iOS 26.2 simulator)
f903d37 to
b4036a9
Compare
There was a problem hiding this comment.
Not ready to approve yet: closing a touch-opened ContextMenu with UIKit's reverse morph is new direction that no current spec covers (component:ContextMenu and spec:AST-094 FR7–FR8 are draft), so an engineering owner needs to approve or reject it, including whether the Reduce Motion exit should fade out in place (FR8's wording) or travel into the finger as it does now. Any engineering owner, the author included if they are one, can record that decision by commenting /approve-spec <full head SHA> on this pull request. Separately from that decision, please rebase once #8043–#8213 land so this diff is only the final commit, and address these:
- A second quick tap on the fading backdrop still removes the shrinking menu at once, because
dismissMenurunsendExit()before it finds the closed menu at 0 × 0 (ContextMenuLift.tsx:623-634); the new browser test only covers a press held past the exit. - CI
lintfails on the twoflushSynccalls inContextMenu.tsx(lines 783 and 794), which are errors under the strict lint CI runs. drop()and the touch-close state resets still run from an Effect watchingisOpen(ContextMenu.tsx:861-872) rather than from the close handler, whicharchitecture:react-component-runtimeINV2 asks for.- The stacked
--color-background-backdropname and the newadaptivequery conflict with the currentarchitecture:theme-tokensINV10 andcomponent:SelectorDEC-1; settle those in their own pull requests.
[Automated review]
| } | ||
| // The menu's own frame (its entrance ends here). | ||
| endMorph(); | ||
| endExit(); |
There was a problem hiding this comment.
A second quick tap on the fading backdrop reaches here again (the backdrop takes taps until the lift layer hides, and closeMenu is its onBackdropTap): endExit() removes the shrinking glass and menu copy before the 0 × 0 check below returns for the already-hidden menu, so the exit vanishes mid-flight while the copy keeps returning. Measuring first and returning before endMorph()/endExit() when the menu is not showing would keep the exit; a quick second tap in the during-exit browser test would cover it.
| // The menu's side depends on its height: place it below, show it, | ||
| // measure it, and move it above in the same task if it does not fit. | ||
| const below = menuPlacement(lifted.lifted, 0, lifted.viewport); | ||
| flushSync(() => { |
There was a problem hiding this comment.
CI's strict lint reports @eslint-react/dom-no-flush-sync as an error here and at line 794, and the lint check fails on this head. A justified eslint-disable-next-line @eslint-react/dom-no-flush-sync -- <why the placement needs a synchronous commit>, as Drawer/useDrawerDialogPresence.ts does, or a placement without a synchronous commit would clear it.
| // A popover reports itself open a task after the press asked (its toggle | ||
| // event), so only an open → closed change ends the touch-opened menu. | ||
| const wasOpenRef = useRef(isOpen); | ||
| useEffect(() => { |
There was a problem hiding this comment.
This Effect drives the exit's drop() and resets isTouchOpen and liftPlacement in reaction to isOpen. architecture:react-component-runtime INV2 asks for that work in the handler that closes the menu, and the layer's onHide already runs on every close.
|
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.
|
b4036a9 to
d054255
Compare
github-actions
Bot
added
the
needs:spec-owner-review
|
Addressed in d054255:
|
There was a problem hiding this comment.
Not ready to approve yet: closing a touch-opened ContextMenu with UIKit's reverse morph, with its springs exposed as theme motion roles, is new direction that no current spec covers (component:ContextMenu and spec:AST-094 FR7 are draft), so an engineering owner needs to approve, reject or amend it. Any engineering owner, the author included if they are one, can record that decision by commenting /approve-spec <full head SHA> on this pull request. The Reduce Motion exit now fades out in place within 200 ms as design:reduced-motion DR2 requires, and the second-tap, lint and Effect issues from the last round are fixed. Separately from that decision, please address these:
- Rebase once #8043–#8213 land so this diff is only the final commit; today it is 37,195 authored lines.
- Keep the menu's scroll position in the exit's copy (
content.scrollTop = menu.scrollTopafterlayerEl.append(glass, content)inContextMenuLift.tsx); a scrolled long menu jumps back to its first rows as it closes. test-uifails on thelayerViewportInsetgutter guard (the lift reads the safe-area insets itself) and on thepressableCoverageguard (ContextMenu.tsxmarks its trigger withusePressFeedback()without a press the controller paints).- Release the exit's timers and the backdrop's capture
clicklistener incleanUp, asarchitecture:react-component-runtimeINV5 asks.
[Automated review]
| boxShadow: 'none', | ||
| pointerEvents: 'none', | ||
| }); | ||
| layerEl.append(glass, content); |
There was a problem hiding this comment.
The copy of a scrolled menu starts at scrollTop 0: cloneNode does not carry the scroll offset, so a person who scrolled a menu taller than its 300 px cap sees the exit jump back to its first rows (Action 7–14 became Action 1–8 in a 14-item menu). Setting content.scrollTop = menu.scrollTop; right after this append keeps what they were looking at.
| } | ||
| }; | ||
| layerEl.addEventListener('click', finish, true); | ||
| const timer = setTimeout(finish, CLICK_AFTER_LIFT_MS); |
There was a problem hiding this comment.
This timer and the capture click listener above, the exit's timer in drop (line 547) and the entrance's timer in morphMenu (line 696) are not released on unmount: the unmount cleanup only runs cleanUp, which clears none of them. architecture:react-component-runtime INV5 asks for such resources to be released when the component unmounts; keeping the ids and the listener in refs and clearing them in cleanUp would do it.
| probe.setAttribute('data-astryx-safe-area-probe', ''); | ||
| probe.style.cssText = | ||
| 'position:fixed;top:0;left:0;visibility:hidden;pointer-events:none;' + | ||
| 'padding-top:env(safe-area-inset-top,0px);' + |
There was a problem hiding this comment.
Layer/layerViewportInset.test.ts ("is the only gutter definition among anchor-mode layers", spec:AST-059 FR1) fails on this file because of these safe-area-inset reads, and CI's test-ui (2/4) fails with it. Taking the insets from the layer runtime's own inset, or having an owner admit the lift as a gutter owner, would clear it.
| // 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.
utils/pressableCoverage.test.ts ("marks nothing that does not paint a press") fails on this file: the trigger is marked with usePressFeedback(), but its press is painted by useContextMenuPressFeedback's animator, not by an arm the touch press controller reads, so the marker asks the controller to write an attribute nothing reads. CI's test-ui (2/4) fails on it.
| * following it in from a blur (`morphMenu`), and shrinks back into it when | ||
| * the menu closes (`dismissMenu`), from wherever the entrance stands (every | ||
| * part retargets, as UIKit's springs do). When the menu closes, the copy | ||
| * returns to the view's place on the dismissal spring while the backdrop and |
There was a problem hiding this comment.
The header still says the copy returns "on the dismissal spring"; drop now returns it on LIFT.morphSpring with the shadow on shadowExitSpring (lines 526–538).
d054255 to
3b03e6f
Compare
There was a problem hiding this comment.
Not ready to approve yet: closing a touch-opened ContextMenu with UIKit's reverse morph, with its three exit springs exposed as theme motion roles, is new direction that no current spec covers (component:ContextMenu and spec:AST-094 FR4, FR7 and FR33 are draft), so an engineering owner needs to approve, reject or amend it. Any engineering owner, the author included if they are one, can record that decision by commenting /approve-spec <full head SHA> on this pull request. The scrolled-menu jump and the two failing guard tests from the last round are fixed, and the Reduce Motion exit fades out in place within 200 ms as design:reduced-motion DR2 requires. Separately from that decision, please address these:
- Rebase once the predecessor pull requests land so this diff is only the final commit; today it is 43,687 authored lines, including the new
architecture:layer-runtimeINV13 and the rewrite of the currentuseMenuPressmodule record. - Release the exit's timers and the backdrop's capture
clicklistener incleanUp, asarchitecture:react-component-runtimeINV5 asks (ContextMenuLift.tsxlines 439–440, 612 and 761). - Correct
astryx docs motion, which still says no shipped component reads the motion roles while ContextMenu's lift and exit do.
[Automated review]
| content: [ | ||
| { | ||
| type: 'prose', | ||
| text: 'Touch motion has a table of springs, durations and curves measured on iPhone and iPad. 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. No shipped component reads these roles yet.', |
There was a problem hiding this comment.
This still tells theme authors that no shipped component reads these roles, but ContextMenu's lift and exit read contextMenu.lift.* and springs.contextMenuDismiss through resolveMotion (ContextMenuLift.tsx lines 649–651, 834–836 and 913), and this commit adds three more rows to the table below. Naming ContextMenu's touch lift and exit as the reader, or dropping the sentence, would keep the doc true.
…tation has run Under the touch model the menu opens with a presentation, and the layer's input hold (spec:AST-094 FR35) drops every tap until that presentation completes, as UIKit drops a touch while one runs. The check tapped the shadow host two frames after the lift, so its tap landed on the hold's shield and the menu stayed open. It now taps once the page's animations have finished, and its assertions are unchanged.
|
Addressed on efefa0b:
|
There was a problem hiding this comment.
Approval waits on an engineering owner's spec decision: this pull request adds requirements the current specs do not state (the layer runtime's exit-copy invariant INV13, ContextMenu's draft DEC-2 lift elements, the public useLongPress stages option and three new AST-094 FR4 motion-role rows), and any engineering owner, the author included, records that decision by commenting /approve-spec <full head SHA> on this pull request. The close itself follows the current AST-094 FR31 and FR33. Independently of that decision, these need changes:
- Land the stacked predecessors first, so this pull request carries only its 432-line close instead of 38,202 authored lines.
- Remove the AST-057 sentence that puts the row swipe on theme motion roles: current AST-094 FR4 and DEC-11 keep those roles internal, and the row still settles on a fixed 200 ms.
- Remove the fine-pointer press-tracker listener branch in
GestureHost.ts, which current AST-094 FR19 does not allow. - Name the synchronized system in the
ContextMenuTouchLayerhandle effect, and flag and justify ContextMenu's touch-only bundle growth in the size report.
[Automated review]
| - **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.
This sentence contradicts current spec:AST-094 FR4 ("The roles are internal until the public motion-token set is decided, so a theme cannot override them yet (DEC-11)"), and the row still settles on SWIPE_SETTLE_MS = 200 with ease-out (Item/useSwipeAction.ts:86, :368). Please drop it and restore "slide and spring durations" in the behavior constants.
| * `touchstart` and `pointerdown` listeners on its own element, and no | ||
| * style; the host's document and window listeners come for a finger's | ||
| * press on it and go when the press ends. It paints that press's feedback | ||
| * only (an FR19 exception for press feedback only, pending in |
There was a problem hiding this comment.
A press tracker with perEventPointer registers touchstart and pointerdown listeners where the primary pointer is fine, which current spec:AST-094 FR19 rules out ("no recognizer attaches and no touch listener is registered"), excepting only the row swipe. No component passes the option; please remove the press-tracker branch until FR19 is amended.
| }, | ||
| ); | ||
|
|
||
| useEffect(() => { |
There was a problem hiding this comment.
architecture:react-component-runtime INV3 asks a new Effect to name the external system it synchronizes. This one calls onHandleChange on mount and with null on cleanup; a comment naming what the handle keeps in step (the press recognizer and the lift), or a ref instead of an Effect, would cover it.
… when asked, cancels on a drift, and times its request on the gesture clock) into the meta#8228 helper
There was a problem hiding this comment.
Approval waits on an engineering owner's spec decision: this pull request adds requirements the current specs do not state (the layer runtime's exit-copy invariant INV13, ContextMenu's draft DEC-2 lift elements, the public useLongPress stages option and three new AST-094 FR4 motion-role rows), and any engineering owner, the author included, records that decision by commenting /approve-spec <full head SHA> on this pull request. The close itself follows the current AST-094 FR31 and FR33. Independently of that decision, these need changes:
- Land the stacked predecessors first, so this pull request carries only its 432-line close instead of 38,276 authored lines.
- Drop the AST-057 sentence that puts the row swipe's motion on motion roles: the row still settles on a fixed 200 ms ease-out, and the sentence already has its own owner question on #8186.
- Remove the fine-pointer press-tracker listener branch in
GestureHost.tsuntil the FR19 amendment (FR43) is onmain. - Make the press feedback's springs motion roles, as AST-094 FR4 requires of every touch spring (
contextMenuPressFeedback.ts). - Name the synchronized system in the
ContextMenuTouchLayerhandle Effect, as react-component-runtime INV3 asks.
[Automated review]
| clocks). The row's slide and spring motion plays on theme motion roles | ||
| (`spec:AST-094` FR4). |
There was a problem hiding this comment.
This sentence says the row's slide and spring motion plays on theme motion roles, but the row still settles on a literal SWIPE_SETTLE_MS = 200 with ease-out (Item/useSwipeAction.ts:86, :368; Item/ItemSwipeLayer.tsx:108), and the current record keeps those durations as behavior constants. It also has its own owner question on #8186; please drop it from this branch so the current record does not describe motion the row never plays.
| /** | ||
| * The recognizer, or the press tracker, reads each event's pointer itself, | ||
| * so it attaches even where the primary pointer is fine, with no warning. | ||
| * A recognizer gets the host's listeners and its element's style there, as | ||
| * `spec:AST-057`'s row swipe does. A press tracker there gets only passive | ||
| * `touchstart` and `pointerdown` listeners on its own element, and no | ||
| * style; the host's document and window listeners come for a finger's | ||
| * press on it and go when the press ends. It paints that press's feedback | ||
| * only (an FR19 exception for press feedback only, pending in | ||
| * `spec:AST-094`'s amendment): no control event, nothing kept from the | ||
| * browser, which still pans, and no mouse or pen press. Everything else | ||
| * stays inactive there. @default false | ||
| */ | ||
| perEventPointer?: boolean; |
There was a problem hiding this comment.
spec:AST-094 FR19 says "Where the primary pointer is fine, no recognizer attaches and no touch listener is registered (FR24)", excepting only the AST-057 row swipe. The press-tracker half of perEventPointer registers passive touchstart and pointerdown listeners there, and the amendment it waits on (FR43, #8646) is still open. No component passes it today; please remove the press-tracker branch until FR43 is on main.
| useEffect(() => { | ||
| onHandleChange({ | ||
| release: () => feedbackRef.current.release(), | ||
| end: () => feedbackRef.current.end(), | ||
| get presentation() { | ||
| return liftRef.current; | ||
| }, | ||
| placeMenu: (lifted, menuHeight) => ({ | ||
| ...menuPlacement(lifted.lifted, menuHeight, lifted.viewport), | ||
| gap: contextMenu.lift.menuGap, | ||
| }), | ||
| isTracking: () => recognizer.numberOfTouches > 0, | ||
| }); | ||
| return () => onHandleChange(null); | ||
| }, [onHandleChange, recognizer]); |
There was a problem hiding this comment.
architecture:react-component-runtime INV3: "A new or changed Effect MUST name the external system it synchronizes." This Effect hands the layer's handle to ContextMenu and clears it on unmount with no such comment; please add one (the parent's ref to the mounted touch layer), or hand the handle over with a ref callback instead of an Effect.
| animator.animate( | ||
| {scale: 1}, | ||
| {spring: feedbackSpring}, |
There was a problem hiding this comment.
spec:AST-094 FR4: "Every spring, duration and curve a touch presentation plays MUST be a motion role named for its use." This spring-back, and the growth curve pressFeedbackMotion builds, play contextMenu.press.feedbackSpring, which registerContextMenuTable registers as an internal constant ("None is a motion role", motion/uikit/contextMenu.ts:248-259). Please register the feedback's springs as roles through registerMotionTable's roles, as the lift's springs are.
… the presented start A zoom stage already drives its photo with an animator (pinch, pan, settle), and the zoom's grow is a rect morph on the same element: two animators would overwrite each other's channels. createRectMorph takes that animator in `animator` and plays on it; dispose leaves it to its owner. The morph's presentedRect then reads the stage's own transform, so a dismissal from a zoomed photo starts from the shown rect. A present or a dismissal also takes the animator's `start`: with 'presented', the element holds its first values until two frames have presented it, so a surface that appears late (a dialog's first frame in WebKit) plays the whole curve. Both new cases fail before this change and pass after.
…only moves Under Reduce Motion on a touch device, the morph treats a path with no opacity of its own as an entrance on a present and an exit on a dismissal, so the cross-fade lands it at rest (design:reduced-motion). An element that stays visible and only moves, such as a sub-menu's parent card, then cut out and faded in on present, and faded out and popped back on dismissal, with no way for its caller to say so: the animator's crossfadeAs names only an entrance or an exit. A present or a dismissal now takes crossfadeAs: 'move', which passes no cross-fade direction, so the animator plays its own dip: out, the travel lands, back in. The new case does not compile before this change (the option had no value for a move; an untyped 'move' reached the animator only by accident) and passes after.
…t morph's source copy AST-057's record takes main's text throughout (meta#8646 drops two-finger trackpad swipes from the non-goals); this branch keeps only its own change there: the row's slide and spring motion plays on theme motion roles (spec:AST-094 FR4), so its durations leave the behavior constants.
There was a problem hiding this comment.
Approval waits on an engineering owner's spec decision: this pull request adds requirements the current specs do not state (the layer runtime's exit-copy invariant INV13, ContextMenu's draft DEC-2 lift elements, the public useLongPress stages option and three new AST-094 FR4 motion-role rows), and any engineering owner, the author included, records that decision by commenting /approve-spec <full head SHA> on this pull request. The close itself follows the current AST-094 FR31 and FR33. Independently of that decision, these need changes:
- Land the stacked predecessors first, so this pull request carries only its 432-line close instead of 38,360 authored lines.
- Drop the AST-057 sentence that puts the row swipe's motion on theme motion roles: the row still settles on a fixed 200 ms ease-out.
- Bring the gesture host in line with the current AST-094 FR19 and FR43, which attach gestures for every pointer, or keep it out of this branch.
- Make the press feedback's spring a motion role, as AST-094 FR4 requires (
contextMenuPressFeedback.ts). - Name the synchronized system in the
ContextMenuTouchLayerhandle Effect, as react-component-runtime INV3 asks. - Add WebKit evidence that the close keeps moving through a blocked main thread (FR5), or have an owner accept the iOS recording in its place.
[Automated review]
| - **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.
This sentence says the row's slide and spring motion plays on theme motion roles, but the row still settles on a literal SWIPE_SETTLE_MS = 200 with ease-out (Item/useSwipeAction.ts:86, :368; Item/ItemSwipeLayer.tsx:108), current AST-057 keeps those durations as behavior constants, and touch roles are internal under AST-094 DEC-11. Please drop it from this branch and let it follow the owner question on #8186.
| * | ||
| * ONLY WHERE THE PRIMARY POINTER IS COARSE | ||
| * | ||
| * - Recognizers attach, and the host registers listeners, only while the |
There was a problem hiding this comment.
Current AST-094 FR19 now reads "Recognizers attach for every pointer, with FR43's exceptions", and FR43 says gestures attach for a mouse, a pen and a finger. This header, the fine-pointer gate and the development warning at :1198-1209 still describe FR19 as it stood before FR43, so the first component that attaches a gesture for a mouse or pen gets nothing.
| settleBack( | ||
| animator.animate( | ||
| {scale: 1}, | ||
| {spring: feedbackSpring}, |
There was a problem hiding this comment.
contextMenu.press.feedbackSpring plays here as an internal constant ("None of them is a motion role", contextMenuPressCurve.ts:22). AST-094 FR4 requires every spring a touch presentation plays to be a motion role named for its use; registering it as a role, as the lift's springs are, keeps the timing and closes the gap.
| }, | ||
| ); | ||
|
|
||
| useEffect(() => { |
There was a problem hiding this comment.
react-component-runtime INV3: a new Effect must name the external system it synchronizes. This one hands the handle to ContextMenu through onHandleChange and clears it on unmount; a one-line comment, or a ref callback instead of the Effect, would settle it.
Depends on #8213 (with #8485's lift module). Through #8201, it carries the static-import offer on #8125 (16dc55e) as a dependency; if #8125 lands differently, this stack follows her decision. This PR's own change: vjeux/astryx@94744dc...5e40861
User need
Part of the UIKit-parity program. On iOS 26 closing a context menu reverses its opening: the menu shrinks back into the finger and the lifted view settles. With #8213 alone, Astryx's menu vanishes at once, and on main a tap outside also clicks what is under it.
Problem and solution fit
Under a finger (
spec:AST-094FR31), in #8201's lift: the menu's exit copy shrinks into the press as it fades, onsprings.menuClose, while the trigger returns and the shadow and tint clear. A close during the growth shrinks from where the menu is, keeping its velocity (FR6). A tap on the tint, Escape and a pick all close it this way, with transform and opacity only, and a press during the close reaches nothing. Reduce Motion fades everything out where it stands.Current specification
component:ContextMenu(draft);spec:AST-094FR5–FR7, FR31 and FR35;design:reduced-motion. Owner decision, pending: a touch-opened menu shrinks back into the finger, as UIKit's.Public delta and caller burden
No API change. ContextMenu JS goes from 79,600 B on main to 120,356 B gzip with this stack. Public-API gaps (
spec:AST-107FR7): the lift, its springs and UIKit tables, the input hold.Design and alternatives
Shrinking the live menu would keep it in the page while it leaves, focusable and in the way of the next press; a copy can be inert. So the close is the lift module's
dismissSurfaceon the layer runtime's one transient exit copy (architecture:layer-runtimeINV15). Opening and closing ContextMenu's touch story in Chromium at 4× CPU slowdown costs less main thread than the clone it replaced (38–54 ms against 51–71 ms opening, 22–41 ms against 38–57 ms closing).Evidence
iOS 26.2 simulator (time since touch, clicks shown), then Android 15:
Scope
Testing
In the story "Touch / long press", on a phone or the iOS simulator:
pnpm test:interaction ContextMenuLift ContextMenuLongPresschecks the same in Chromium and WebKit, including that nothing is left in the page after the close. Unrelated failures: #8638, #8671. GitHub shows a conflict with main inContextMenu.spec.md: it is resolved in #8201 and reaches this PR at its next push.