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
453e74c to
2cb42b3
Compare
2cb42b3 to
5c0e2b8
Compare
5c0e2b8 to
e71f692
Compare
![astracat-bot[bot]](https://avatars.githubusercontent.com/in/4796231?s=60&v=4){kind=small}
There was a problem hiding this comment.
Approval waits on an engineering owner's decision: this pull request intentionally changes the current row-actions spec (AST-057 FR5, so a flick opens the row instead of firing its outermost action, and FR2/DEC-5, so the row moves by compositor translate animations instead of a travel custom property), and it adds unapproved requirements to the current Item record (FR9 to FR13, DEC-3/DEC-4) and the new draft AST-094, including keeping swipe motion under Reduce Motion. An engineering owner records that decision by commenting /approve-spec e71f692b6c15a90f3d10adbf4ff781a39f447fa2 on this pull request (the author may do so if they are an engineering owner), and the AST-057 text needs the matching amendment, here or on main, before the change conforms to it. Independently of that decision, please address these:
- The diff carries the whole stack (27,969 authored lines, and the last commit alone is 2,052), so land the predecessors and bring what remains under 2,000 lines or explain why it cannot be split.
.changeset/item-grid-row.mdships arole="row"change that existing callers must adapt to as a patch[feat]; it needs a[breaking]changeset in a declared minor (AST-017 FR32)..changeset/item-swipe-gesture-system.mdsays where a release lands is unchanged, whichitem-swipe-uikit-motion.mdand the head contradict, andpackages/core/src/gestures/README.mddocuments autils/touchPresentation.tsthat does not exist.
[Automated review]
| @@ -0,0 +1,7 @@ | |||
| --- | |||
| '@astryxdesign/core': patch | |||
There was a problem hiding this comment.
This note says existing role="row" callers now find an anchor or button in the label cell and must set its tabIndex through controlProps. A change that needs a caller edit is [breaking] under AST-017 FR32 and belongs in a declared minor, not a patch [feat].
| '@astryxdesign/core': patch | ||
| --- | ||
|
|
||
| [fix] Swipe actions on `Item` and `ListItem` run on the gesture system. A row claims a sideways touch at 8 px, before a browser commits to scrolling it, and its swipe begins at UIKit's 25 px, from 0, so a short wobble no longer moves the row. A second finger before the claim leaves the touch to the browser's pinch, and after it is ignored. The click a browser synthesizes after a swipe is cancelled at its source instead of being swallowed by a 400 ms timer. The row's motion and where a release lands are unchanged. |
There was a problem hiding this comment.
"The row's motion and where a release lands are unchanged" contradicts item-swipe-uikit-motion.md (a flick no longer fires) and the head: a 200 px, 2 s drag that deletes the row on the base only opens it here. Please drop the sentence and describe the change once.
| if (behavior === 'commit') { | ||
| return 'closed'; | ||
| } | ||
| if (velocity >= swipeActions.releaseVelocity) { |
There was a problem hiding this comment.
With this rule no release velocity fires an entry; current AST-057 FR5 says a fling MUST fire the outermost entry under both models. This stays a spec contradiction until an engineering owner amends FR5.
| ## Touch presentation | ||
|
|
||
| Whether a component presents itself the touch (UIKit) way is answered once, | ||
| in `../utils/touchPresentation.ts`: |
There was a problem hiding this comment.
../utils/touchPresentation.ts (and TOUCH_QUERY, COMPACT_TOUCH_QUERY, useTouchPresentation()) does not exist at this head; please drop this section until that module lands.
e71f692 to
24e66ab
Compare
There was a problem hiding this comment.
Approval waits on an engineering owner's decision: this pull request intentionally changes the current row-actions spec (AST-057 FR5, so a flick opens or closes the row instead of firing its outermost action, and FR2/DEC-5, so the row moves by compositor translate animations instead of a travel custom property), and it adds unapproved requirements to the current Item record (FR9 to FR13, DEC-3/DEC-4) and the new draft AST-094, including keeping swipe motion under Reduce Motion. An engineering owner records that decision by commenting /approve-spec 24e66ab7a6465f6e9491dc738eec748495b76636 on this pull request (the author may do so if they are an engineering owner), and the AST-057 text needs the matching amendment, here or on main, before the change conforms to it. Independently of that decision, please address these:
- The diff carries the whole stack (27,997 authored lines, and the last commit alone is 2,080), so land the predecessors and bring what remains under 2,000 lines or explain why it cannot be split.
.changeset/item-grid-row.mdships arole="row"change that existing callers must adapt to as a patch[feat]; it needs a[breaking]changeset in a declared minor (AST-017 FR3, FR31)..changeset/item-swipe-gesture-system.mdsays where a release lands is unchanged, whichitem-swipe-uikit-motion.mdand the head contradict, andpackages/core/src/gestures/README.mddocuments autils/touchPresentation.tsthat does not exist.
[Automated review]
| @@ -0,0 +1,7 @@ | |||
| --- | |||
| '@astryxdesign/core': patch | |||
There was a problem hiding this comment.
This note says existing role="row" callers now find an anchor or button in the label cell and must set its tabIndex through controlProps. A change existing callers must adapt to is an incompatible delta of documented interaction and accessibility behavior (AST-017 FR3), and FR31 maps each one to a [breaking] changeset in a declared minor, not a patch [feat].
| '@astryxdesign/core': patch | ||
| --- | ||
|
|
||
| [fix] Swipe actions on `Item` and `ListItem` run on the gesture system. A row claims a sideways touch at 8 px, before a browser commits to scrolling it, and its swipe begins at UIKit's 25 px, from 0, so a short wobble no longer moves the row. A second finger before the claim leaves the touch to the browser's pinch, and after it is ignored. The click a browser synthesizes after a swipe is cancelled at its source instead of being swallowed by a 400 ms timer. The row's motion and where a release lands are unchanged. |
There was a problem hiding this comment.
"The row's motion and where a release lands are unchanged" contradicts item-swipe-uikit-motion.md (a flick no longer fires) and the head: a 140 px flick and a 200 px drag delete Message 1 on the base and leave it open on the head. Please drop the sentence and say once what changed about where a release lands.
| if (travel >= commitAt) { | ||
| return 'perform'; | ||
| } | ||
| if (behavior === 'commit') { | ||
| return 'closed'; | ||
| } | ||
| if (velocity >= swipeActions.releaseVelocity) { | ||
| return 'open'; | ||
| } | ||
| if (velocity <= -swipeActions.releaseVelocity) { | ||
| return 'closed'; | ||
| } | ||
| return travel >= swipeActions.openFraction * openWidth ? 'open' : 'closed'; |
There was a problem hiding this comment.
With this rule no release velocity fires an entry, and a row released past the panel's width while moving back closes; current AST-057 FR5 says a fling MUST fire the outermost entry under both models and a release past the panel's width MUST rest open. This stays an owner decision on amending FR5 rather than a change request.
| ## Touch presentation | ||
|
|
||
| Whether a component presents itself the touch (UIKit) way is answered once, | ||
| in `../utils/touchPresentation.ts`: |
There was a problem hiding this comment.
../utils/touchPresentation.ts (and TOUCH_QUERY, COMPACT_TOUCH_QUERY, useTouchPresentation()) does not exist at this head; please drop this section until that module lands, or land it with the README.
PR Analysis Report📚 Storybook Preview🧪 Sandbox PreviewModified ComponentsItem (@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/BreadcrumbItem | +30.3KB |
-46B |
| core/CheckboxListItem | +30.1KB |
-115B |
| core/ContextMenu | +30.4KB |
-88B |
| core/ContextMenuItem | +29.9KB |
-98B |
| core/DropdownMenu | +29.7KB |
-83B |
| core/DropdownMenuCheckboxItem | +29.8KB |
-93B |
| core/DropdownMenuItem | +29.7KB |
-98B |
| core/DropdownMenuRadioItem | +29.8KB |
-93B |
| core/DropdownMenuSubMenu | +29.7KB |
-89B |
| core/Item | +29.6KB |
-107B |
| core/ListItem | +30.1KB |
-115B |
| core/Markdown | +30.4KB |
-125B |
| core/MoreMenu | +29.7KB |
-83B |
| core/Pagination | +30.1KB |
-131B |
| core/PowerSearch | +30.3KB |
-67B |
| core/RadioListItem | +30.1KB |
-114B |
| core/Selector | +30.9KB |
-89B |
| core/SelectorOption | +29.8KB |
-104B |
| core/Table | +30.4KB |
-110B |
| core/TableCell | +30.4KB |
-146B |
| core/TableHeaderCell | +30.4KB |
-133B |
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.5KB | -104B |
Accessibility Audit
Status: No accessibility violations detected.
Visual Regression
55 of 504 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
24e66ab to
023a18c
Compare
There was a problem hiding this comment.
Approval waits on an engineering owner's decision: this pull request intentionally changes the current row-actions spec (AST-057 FR5 on iOS, so a flick opens or closes the row instead of firing its outermost action, and FR2/DEC-5 everywhere, so the row moves by compositor translate animations instead of a travel custom property), and it adds requirements no current spec states: Item FR9 to FR13 with DEC-3/DEC-4, the new draft AST-094, theme motion roles with a platform adaptation condition, package-level experimental subpaths, and a new presentation="adaptive" breakpoint. An engineering owner records that decision by commenting /approve-spec 023a18c81dd4346b1e5b713721553242f261915d on this pull request (the author may do so as an engineering owner), and the AST-057 text needs the matching amendment, here or on main. Independently of that decision, please address these:
- Land the predecessors so this pull request holds only its own commit, and bring that commit (2,499 lines) under 2,000 or explain why it cannot be split; the diff now carries 34,324 authored lines.
.changeset/item-grid-row.mdships arole="row"change existing callers must adapt to as a patch[feat]; keep the old rendering for them until a declared minor and a[breaking]changeset (AST-017 FR3, FR31).- Make the theme motion-role API and draft AST-094 FR4/DEC-1 ("physics are not themable") agree, and drop the release-note sentences the head contradicts (
item-swipe-gesture-system.md's "unchanged", the[chore]notes' "no component uses it yet"). - Attach a physical iOS and Android check of the claim, a flick, a long drag and a tap after a swipe, which AST-057's platform support requires for FR5/FR6, and name the external system each changed Effect synchronizes (react-component-runtime INV3).
[Automated review]
| @@ -0,0 +1,7 @@ | |||
| --- | |||
| '@astryxdesign/core': patch | |||
There was a problem hiding this comment.
This note tells existing role="row" callers they now find an anchor or button in the label cell and must set its tabIndex through controlProps. A change callers must adapt to is incompatible under AST-017 FR3 and needs a [breaking] Changeset (FR31), which a patch-declared main does not admit; please keep the old rendering for existing callers until a declared minor.
| '@astryxdesign/core': patch | ||
| --- | ||
|
|
||
| [fix] Swipe actions on `Item` and `ListItem` run on the gesture system. A row claims a sideways touch at 8 px, before a browser commits to scrolling it. On iOS its swipe begins at UIKit's 25 px, from 0, so a short wobble no longer moves the row; on Android and any other touchscreen the row still moves from the touch-down. The pan is published as `Item.swipe`. A second finger before the claim leaves the touch to the browser's pinch, and after it is ignored. The click a browser synthesizes after a swipe is cancelled at its source instead of being swallowed by a 400 ms timer. The row's motion and where a release lands are unchanged. |
There was a problem hiding this comment.
"The row's motion and where a release lands are unchanged" is contradicted on iOS by the 25 px start (useSwipeAction.ts:608-627) and by item-swipe-uikit-motion.md in the same release; please drop the sentence or fold this note into that one.
| if (travel >= commitAt) { | ||
| return 'perform'; | ||
| } | ||
| if (behavior === 'commit') { | ||
| return 'closed'; | ||
| } | ||
| if (velocity >= swipeActions.releaseVelocity) { | ||
| return 'open'; | ||
| } | ||
| if (velocity <= -swipeActions.releaseVelocity) { | ||
| return 'closed'; | ||
| } | ||
| return travel >= swipeActions.openFraction * openWidth ? 'open' : 'closed'; |
There was a problem hiding this comment.
On iOS no release velocity fires an entry, and a reveal row released past the panel's width while moving back closes; current AST-057 FR5 says a fling MUST fire the outermost entry under both models and a release past the panel's width MUST rest open. This stays a spec contradiction until an engineering owner amends FR5.
| - **FR4 — Physics are not themable in touch presentations.** A touch | ||
| presentation MUST read its motion from the table on every theme. Themes keep | ||
| color, shape, type and spacing; the theme's duration and easing tokens keep | ||
| governing pointer presentations. **Proposed; pending owner review:** this | ||
| narrows `architecture:theme-tokens` (owners: cixzhang, imdreamrunner) to | ||
| pointer presentations, and departs from the draft `design:motion` DR3 | ||
| (owners: ernestt, cixzhang) wherever UIKit settles with an overshoot or | ||
| stretches with a rubber band. |
There was a problem hiding this comment.
FR4 says touch physics are not themable, but this stack ships defineTheme({motion: {springs, durations, curves}}) and a platform condition so themes retune touch motion (.changeset/touch-motion-theme-roles.md). Please make the record and the code agree before an owner rules on either.
| * 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.
A fifth condition axis joins the adaptation vocabulary, which the current theme-authoring contract says an adaptation change preserves ("An adaptation change preserves the fixed condition vocabulary"); this needs an engineering owner's decision together with the motion roles.
| useIsomorphicLayoutEffect(() => { | ||
| stateRef.current = state; | ||
| }); |
There was a problem hiding this comment.
react-component-runtime INV3 asks every new or changed Effect to name the external system it synchronizes; this one, the options copy at lines 300-302, and ItemSwipeLayer.tsx:192-194 and :218-223 only copy render values into refs or publish the key bridge.
023a18c to
30d21c1
Compare
|
Thanks for the review. Where each point stands on the new head:
|
There was a problem hiding this comment.
Approval waits on an engineering owner's decision: this pull request intentionally changes the current row-actions spec (AST-057 FR5 on iOS, so a flick opens or closes the row instead of firing its outermost action, and FR2/DEC-5 everywhere, so the row moves by compositor translate animations instead of a travel custom property), and it adds requirements no current spec states (Item FR9 to FR13 with DEC-3/DEC-4, the new draft AST-094, theme motion roles with a platform adaptation condition, and package-level experimental subpaths). An engineering owner records that decision by commenting /approve-spec 30d21c1c57d300126b234b2fec37d03ab1005c3d on this pull request (the author may do so as an engineering owner), and the AST-057 text needs the matching amendment, here or on main. Independently of that decision, please address these:
- Under Reduce Motion the iOS swipe should cross-fade (opacity only, 200 ms or less) instead of keeping its springs, as current AST-057 FR5 and
design:reduced-motionDR2/DR4 now require (packages/core/src/motion/uikit/swipeActions.ts:81). presentation="adaptive"should use the theme's compact definition (below the effectivemdpoint, 768 px by default, or 500 px tall) instead of its own 744 px edge, per current AST-012 FR9 and public-component-api INV17.- Land the predecessors so this pull request holds only its own commits, and bring those under 2,000 lines or explain why they cannot be split; the diff now carries 34,530 authored lines.
.changeset/item-grid-row.mdships arole="row"change existing callers must adapt to as a patch[feat], which AST-017 FR1, FR3 and FR31 classify as breaking.- Make the motion-role theme API and AST-094 FR4/DEC-1 agree, drop the stale "no component uses it yet" release notes, clear a pending fire timer before setting another in
useSwipeAction.ts, and attach a physical iOS and Android check of FR5/FR6.
[Automated review]
| * animators on tracking and release, and the full swipe still performs, | ||
| * Oct 23 2026 | ||
| */ | ||
| reducedMotion: 'keep', |
There was a problem hiding this comment.
With 'keep', the iOS settle, slide-out and spring-back keep their springs under Reduce Motion (the animator skips its substitution for 'keep'). Current AST-057 FR5 now says the slide out, the spring back and the hold-out follow design:reduced-motion, and its DR2 asks for an opacity-only cross-fade of 200 ms or less when a touch drag's release starts the transition, with DR4 admitting no component that keeps springs. Please cross-fade here under prefers-reduced-motion: reduce, unless an engineering owner amends those records first.
|
|
||
| /** Both: presentations such as a sheet in place of a popover. */ | ||
| export const COMPACT_TOUCH_QUERY = | ||
| '(width < 744px) and (pointer: coarse), (height < 500px) and (pointer: coarse)'; |
There was a problem hiding this comment.
Current AST-012 FR9 (and public-component-api INV17) makes compact "width below the effective md point or height below 500", taken from the theme's one definition, and names this web touch presentation resolver as one that must use it and keep no size numbers of its own. With 744 px here, a coarse-pointer screen 744 to 767 px wide opens the popover where FR9 gives the sheet, and a theme's md override is ignored. Please build the query from the theme's compact definition, ANDed with the coarse pointer.
| if (delay === 0) { | ||
| fireIfStill(); | ||
| } else if (delay != null) { | ||
| fireTimerRef.current = setTimeout(fireIfStill, delay); |
There was a problem hiding this comment.
A new timer replaces fireTimerRef.current without clearing a pending one, and every callback nulls the ref before checking its motion (line 557). If a finger catches a sliding-out row and releases it past the commit point again, the older timer drops the newer one's handle, so the unmount cleanup can no longer clear it and onActivate can run for a row that is gone. Clearing any pending timer here, and nulling the ref only when it still holds the callback's own timer, closes that.
|
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. |
…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.
… first lift event, its pointerup or its touchend, whichever the gesture host releases on
…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.
There was a problem hiding this comment.
Approval waits on an engineering owner's decision. This pull request intentionally changes current spec:AST-057 FR5 (a flick opens or closes a row and never fires, and the outermost action fires only once the row's content has left its box, not at activation) and FR2/DEC-5 (the row moves by compositor translate animations instead of the travel custom property). It also leaves open whether AST-057's gesture-tuning non-goal or spec:AST-094 FR4/FR17 governs the row's springs and claim distance. An engineering owner records that decision by amending spec:AST-057 on main, or by adding the amendment to this pull request and commenting /approve-spec followed by the full head SHA; the author may do so as an engineering owner. Whatever the decision, please:
- Land the stacked predecessors so this pull request carries only its own commits, move the
astryxStylexdefault, press-feedback and menu changes to their own pull requests, and bring the swipe change under 2,000 lines or explain why it cannot be split (32,435 authored lines now, 2,959 of them its own). - Merge after #8671 or carry its change, because
scripts/touch-support-bundle.test.mjsfails on this revision at "is absent from the root entry". - Let the gesture host attach for every pointer, as
architecture:react-component-runtimeINV13 andspec:AST-094FR19 now require (GestureHost.tsgates on a coarse primary pointer). - Add physical-device recordings, which
spec:AST-057requires for FR5 and FR6.
[Automated review]
| if (behavior === 'commit') { | ||
| return 'closed'; | ||
| } | ||
| if (velocity >= swipeActions.releaseVelocity) { |
There was a problem hiding this comment.
A release at 48 px/s or faster opens the row here and never fires the outermost entry. Current spec:AST-057 FR5 still reads "a drag past the commit point or a fling MUST fire the outermost entry", so this line waits on an engineering owner amending FR5.
| if (fireTimerRef.current === timer) { | ||
| fireTimerRef.current = null; | ||
| } | ||
| if (motionRef.current === motion) { |
There was a problem hiding this comment.
The fire happens only if no other motion has started since the release. A finger that catches the sliding row, a close, or an unmount (the cleanup at lines 426-434) drops the onActivate call. Current spec:AST-057 FR5 reads "onActivate MUST fire at activation", so this needs the same owner ruling as the release rule.
| * | ||
| * ONLY WHERE THE PRIMARY POINTER IS COARSE | ||
| * | ||
| * - Recognizers attach, and the host registers listeners, only while the |
There was a problem hiding this comment.
Gating every attach on a coarse primary pointer contradicts current architecture:react-component-runtime INV13 ("The gesture host attaches for every pointer, with no touch-only query") and spec:AST-094 FR19/FR43. Please attach for every pointer, keep FR43's exceptions, and drop the fine-pointer warning at line 1355.
| // nothing appears or moves when the layer mounts; the gesture simply attaches | ||
| // once the chunk is ready, as ProgressBar's and Timestamp's lazy affordances do. | ||
| const LazyItemSwipeLayer = lazy(async () => import('./ItemSwipeLayer')); | ||
| import ItemSwipeLayer, {type ItemSwipeBridge} from './ItemSwipeLayer'; |
There was a problem hiding this comment.
Importing the swipe layer eagerly puts the gesture host and the animator in every Item import: 13.5 KB to 42.6 KB gzipped, built minified with React and StyleX external. It also fails scripts/touch-support-bundle.test.mjs at "is absent from the root entry" until #8671's test change lands.
| // compiled (the TypeScript transform drops it as unused), the module is | ||
| // never loaded, and its constants never reach the CSS: a media-query key | ||
| // fails the build and a value ships as an undefined custom property. | ||
| treeshakeCompensation: true, |
There was a problem hiding this comment.
This changes the {stylexOptions} form's default for every @astryxdesign/build consumer. It is unrelated to swipe rows and no current claim backs it, so please land it in its own pull request.
Depends on #8096. Review only this PR's change: vjeux/astryx@40530da...7fb1ae9
Why
A released swipe-action row (
Item,ListItem) stopped dead and restarted on a fixed 200 ms ease-out. On a busy page its panel stalled, then jumped; a finger on a moving row was ignored; and a flick fired the outermost action, often Delete.What led to it
Part of a series bringing Astryx's touch interactions to parity with native iOS (UIKit). A CSS transition drove the release, dropping the finger's speed, and the panel animated its
widthon the main thread, so any script work froze it.Options and tradeoffs
translate, not the custom propertyspec:AST-057FR2 names.Owner decisions proposed: that FR2 change, and the release rule (FR5, asked in #8134).
Why this choice
Only option 3 keeps the finger's speed through a busy page, within 0.07 px of UIKit's spring.
How to check it
On a phone, open the Item story "Swipe Actions":
We ran these with real touch in Chromium and on the iOS 26.2 simulator and the Android 15 emulator; each behaved as listed (recordings below). The unit tests pass, and the browser tests (
pnpm test:interaction ItemSwipeActions) match UIKit's spring frame by frame in Chromium and WebKit, also with the main thread blocked.Evidence
Known red until #8671 merges: its touch-bundle check, which a gesture-using
Itemfails by design.Scope