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:spec-owner-review
1d8b392 to
c502930
Compare
![astracat-bot[bot]](https://avatars.githubusercontent.com/in/4796231?s=60&v=4){kind=small}
There was a problem hiding this comment.
Approval is waiting on an engineering owner, because this full swipe intentionally changes spec:AST-057: FR5 says a fling fires the outermost entry and a release past the panel's width leaves the row resting open, and FR4 says the uncovered panel shows every entry. An owner needs to decide whether FR5 and FR4 are updated to the confirmed-only, back-out-closes and disabled-never-confirms rules and the confirmed look, or whether the row keeps today's rules, and the spec text then has to match that decision. An engineering owner records it by commenting /approve-spec <full head SHA> on this pull request; the author may do this if they are an engineering owner. Whatever the owner decides, these still need to change:
- Land the stacked pull requests first and rebase, so this diff holds only its own commit (about 1,100 lines) instead of about 29,000.
- Re-tag
.changeset/item-swipe-full-swipe.md: replacing released, documented swipe behavior is not a[fix]underspec:AST-017FR5 and FR7. - Record a check on a physical touch device of confirm and lift, back-out and flick, which
spec:AST-057requires for FR5.
[Automated review]
| if (isConfirmed) { | ||
| return 'perform'; | ||
| } | ||
| if (behavior === 'commit' || wasConfirmed) { | ||
| return 'closed'; | ||
| } |
There was a problem hiding this comment.
This replaces spec:AST-057 FR5. A fling no longer fires the outermost entry, and a swipe that confirmed and then backed out closes even when it is released past the panel's width, where FR5 says the row rests open. Approval waits on an engineering owner's decision on FR5.
| '@astryxdesign/core': patch | ||
| --- | ||
|
|
||
| [fix] A full swipe on an `Item` or `ListItem` swipe-action row works like a UIKit list row's. Past the open width the entries ride the row and the outermost action stretches out to the edge. Dragged on past the confirm point (judged on the finger, as UIKit does) the row runs ahead and the outermost action covers its place, and letting go there fires it at any speed; backing out undoes that and closes the row. A disabled outermost action never confirms. |
There was a problem hiding this comment.
[fix] does not fit here. This replaces released, documented behavior: the 0.7.x changelog says "a long drag or a fling fires the outermost". Under spec:AST-017 FR5 and FR7 it needs [feat], or [breaking] with the old behavior, the new behavior and the migration, once FR5 is decided.
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 | +31.9KB |
+87B |
| core/CheckboxListItem | +31.7KB |
+16B |
| core/ContextMenu | +32.0KB |
+31B |
| core/ContextMenuItem | +31.4KB |
+36B |
| core/DropdownMenu | +31.3KB |
+35B |
| core/DropdownMenuCheckboxItem | +31.4KB |
+41B |
| core/DropdownMenuItem | +31.3KB |
+34B |
| core/DropdownMenuRadioItem | +31.4KB |
+41B |
| core/DropdownMenuSubMenu | +31.4KB |
+46B |
| core/Item | +31.2KB |
+27B |
| core/ListItem | +31.7KB |
+16B |
| core/Markdown | +32.1KB |
-11B |
| core/MoreMenu | +31.3KB |
+35B |
| core/Pagination | +31.8KB |
-11B |
| core/PowerSearch | +32.0KB |
+41B |
| core/RadioListItem | +31.8KB |
+10B |
| core/Selector | +32.5KB |
+27B |
| core/SelectorOption | +31.4KB |
+29B |
| core/Table | +32.0KB |
+6B |
| core/TableCell | +32.0KB |
-16B |
| core/TableHeaderCell | +32.0KB |
-8B |
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 | -56B |
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
c502930 to
eec394e
Compare
There was a problem hiding this comment.
Approval is waiting on an engineering owner, because this full swipe intentionally changes spec:AST-057: FR5 says a fling fires the outermost entry and a release past the panel's width leaves the row resting open, and FR4 says the uncovered panel shows every entry. An engineering owner needs to either update FR5 and FR4 to the confirmed-only, back-out-closes and disabled-never-confirms rules and the confirmed look, or keep today's rules; they record the decision by commenting /approve-spec <full head SHA> on this pull request, and the author may do this if they are an engineering owner. Whatever the owner decides, these still need to change:
- Land the stacked pull requests first and rebase, so this diff holds only its own commit (about 1,100 lines) instead of about 29,000.
- Re-tag
.changeset/item-swipe-full-swipe.md: replacing released, documented swipe behavior is not a[fix]underspec:AST-017FR5 and FR7. - In #8132, report the row's open state from the handler that changes it rather than from the new effect in
ItemSwipeLayer.tsx, asarchitecture:react-component-runtimeINV2 requires. - Record a check on a physical touch device of confirm and lift, back-out and flick, which
spec:AST-057requires for FR5.
[Automated review]
| useEffect(() => { | ||
| if (!isOpen) { | ||
| return undefined; | ||
| } | ||
| onOpenChange(true); | ||
| return () => { | ||
| onOpenChange(false); | ||
| }; | ||
| }, [isOpen, onOpenChange]); |
There was a problem hiding this comment.
This Effect reports the open state to Item by calling onOpenChange (its state setter) after the layer has rendered, so the commit in which the row opens still carries the press props and every open and close costs an extra Item render. architecture:react-component-runtime INV2 asks for the callback to fire from the handler that caused the change, and INV3 asks a new Effect to name the external system it synchronizes. settle already sets landing: 'resting' through setSwipeState, so reporting the change there (or in setSwipeState when isSwipeOpen flips) removes the Effect. This arrives with #8132.
eec394e to
1c419ee
Compare
There was a problem hiding this comment.
Approval is waiting on an engineering owner, because this full swipe intentionally changes spec:AST-057: FR5 says a fling fires the outermost entry and a release past the panel's width leaves the row resting open, and FR4 says the uncovered panel shows every entry. An engineering owner needs to either update FR5 and FR4 to the confirmed-only, back-out-closes and disabled-never-confirms rules and the confirmed look, or keep today's rules; they record the decision by commenting /approve-spec <full head SHA> on this pull request, and the author may do this if they are an engineering owner. Whatever the owner decides, these still need to change:
- Land the stacked pull requests first and rebase, so this diff holds only its own commit (about 1,300 lines) instead of about 29,000.
- Re-tag
.changeset/item-swipe-full-swipe.md: replacing released, documented swipe behavior is not a[fix]underspec:AST-017FR5 and FR7. - In #8132, report the row's open state from the handler that changes it rather than from the new Effect in
ItemSwipeLayer.tsx, asarchitecture:react-component-runtimeINV2 requires. - Record a check on a physical touch device of confirm and lift, back-out and flick, which
spec:AST-057requires for FR5.
[Automated review]
1c419ee to
9922d97
Compare
There was a problem hiding this comment.
Approval is waiting on an engineering owner, because on iOS this full swipe intentionally changes spec:AST-057: FR5 says a fling fires the outermost entry and a release past the panel's width leaves the row resting open, and FR4 says the uncovered panel shows every entry. An engineering owner needs to either update FR5 and FR4 to allow the iOS-only confirmed-only, back-out-closes and disabled-never-confirms rules and the confirmed look, or keep today's rules; they record the decision by commenting /approve-spec <full head SHA> on this pull request, and the author may do so if they are an engineering owner. Whatever the owner decides, these still need to change:
- Land the stacked pull requests first and rebase, so this diff holds only its own commit (about 1,400 lines) instead of about 35,700.
- Re-tag
.changeset/item-swipe-full-swipe.md: replacing released, documented swipe behavior on iOS is not a[fix]underspec:AST-017FR5 and FR7. - Record a check on a physical iPhone or iPad of confirm and lift, back-out and flick, which
spec:AST-057requires for FR5. - In #8132, report the row's open state from the handler that changes it rather than from the Effect in
ItemSwipeLayer.tsx, asarchitecture:react-component-runtimeINV2 requires. - In the motion-roles change, amend
spec:AST-012FR2 or drop the newplatformtheme condition, which its closed vocabulary does not allow.
[Automated review]
| '@astryxdesign/core': patch | ||
| --- | ||
|
|
||
| [fix] On iOS, a full swipe on an `Item` or `ListItem` swipe-action row works like a UIKit list row's. Past the open width the entries ride the row and the outermost action stretches out to the edge. Dragged on past the confirm point (judged on the finger, as UIKit does) the row runs ahead and the outermost action covers its place, and letting go there fires it at any speed; backing out undoes that and closes the row. A disabled outermost action never confirms. On Android and other touchscreens a long drag past the row's commit point still fires the outermost action, as before. |
There was a problem hiding this comment.
[fix] does not fit here. On iOS this replaces released, documented behavior: the 0.6.7 release notes say "a long drag or a fling fires the outermost". Under spec:AST-017 FR5 and FR7 it needs [feat], or [breaking] with the old behavior, the new behavior and the migration, once an engineering owner decides spec:AST-057 FR5 for iOS.
| * 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.
platform adds a field to the adaptation when vocabulary that spec:AST-012 FR2 closes ("Future environment fields require explicit additions to this closed vocabulary"), and nothing in this diff amends AST-012. It needs the record amended with an owner's approval, or the condition dropped, in the pull request it comes from (merged with #8076).
9922d97 to
c7404b2
Compare
|
Thanks for the review. Where each point stands on the new head:
|
There was a problem hiding this comment.
Approval is waiting on an engineering owner, because on iOS this full swipe intentionally changes current specs: spec:AST-057 FR5 says a fling fires the outermost entry, a release past the panel's width leaves the row resting open and, under reduced motion, the release cross-fades (as design:reduced-motion DR2 and DR4 also require), and FR4 says the uncovered panel shows every entry. An engineering owner needs to either update those claims to allow the iOS-only confirmed-only, back-out-closes and disabled-never-confirms rules, the confirmed look and the springs kept under Reduce Motion, or keep today's rules; they record the decision by commenting /approve-spec <full head SHA> on this pull request, and the author may do so as an engineering owner. Whatever the owner decides, these still need to change:
- Land the stacked pull requests first and rebase, so this diff holds only its own commit (about 1,550 lines) instead of about 35,900.
- Record a check on a physical iPhone or iPad of confirm and lift, back-out, flick and Reduce Motion, which
spec:AST-057requires for FR5. - Fix these in the pull requests they come from: the 744 px compact width against
spec:AST-012FR9 (#8062), theplatformtheme condition outsidespec:AST-012FR2 (#8076), and the cleanup Effect that callsonOpenChange(false)without naming what it synchronizes (architecture:react-component-runtimeINV3, #8132).
[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', every iOS swipe spring ignores Reduce Motion, so the release, the confirmed run-ahead and the slide-out still slide and spring. spec:AST-057 FR5 says the slide-out and spring-back reach their place "with no travel or spring and cross-fade", and design:reduced-motion DR2 and DR4 say a swiped row's release cross-fades and that no component setting keeps springs. This is part of the iOS rules waiting on an engineering owner's decision; if the current records stand, the release needs DR2's cross-fade.
| export const COMPACT_QUERY = '(width < 744px), (height < 500px)'; | ||
|
|
||
| /** 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.
spec:AST-012 FR9 defines compact as width below the theme's md point (768 by default) or height below 500, and says no resolver keeps size numbers of its own; public-component-api INV17 says the same. With 744 here, a 744-767 px touch screen gets the popover where FR9 gives the sheet. Reading the theme's compact definition fixes it; this arrives with #8062.
| // Releases the open report when the row leaves while open. | ||
| useEffect( | ||
| () => () => { | ||
| if (settleTimerRef.current != null) { | ||
| clearTimeout(settleTimerRef.current); | ||
| const {idiom: current, onOpenChange: report} = latestRef.current; | ||
| if (current === 'ios' && isSwipeOpen(stateRef.current)) { | ||
| report?.(false); | ||
| } | ||
| if (frameRef.current != null) { | ||
| cancelAnimationFrame(frameRef.current); | ||
| } | ||
| stopPinchWatchRef.current?.(); | ||
| stopPinchWatchRef.current = null; | ||
| }, | ||
| [], | ||
| ); |
There was a problem hiding this comment.
This Effect's only work is a cleanup that calls onOpenChange(false), which is Item's state setter, and its comment names no external system, which architecture:react-component-runtime INV3 asks for. Item decides whether the layer renders, so it can clear its own flag when it stops rendering it, or the comment can name the state the cleanup releases. This arrives with #8132.
c7404b2 to
991993d
Compare
github-actions
Bot
added
the
needs:design-review
… 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
… 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.
This pull request can't be approved until an engineering owner decides whether spec:AST-057 changes for the full swipe: as written, FR5 has a fling fire the outermost entry, keeps a row released past its panel resting open and springs back a release short of the threshold, and FR4 shows every entry in the uncovered panel, while this change fires only from a swipe still confirmed at the lift, closes after a back-out, lets other fast releases go the way they move, never confirms a disabled outermost entry and covers the row with the outermost entry. An engineering owner needs to update FR5 and FR4 for this intentional change, or keep them as they are, and record that by commenting /approve-spec <full head SHA> here; the author may do so if they are an engineering owner. Independently of that decision:
- Land the stacked pull requests and rebase so this diff holds only its own change (about 2,065 lines instead of 34,040), then bring it under 2,000 lines or explain why it can't be split.
- Add a check on a physical phone (a confirmed lift, a back-out, a flick, a disabled outermost entry, Reduce Motion), which
spec:AST-057requires for FR5. - In #8097, keep FR2's custom property for the row's travel or have FR2 and DEC-5 amended, and in the gesture host's pull request attach recognizers for every pointer as
spec:AST-094FR19 and FR43 require.
[Automated review]
| if (isConfirmed) { | ||
| return 'perform'; | ||
| } | ||
| if (behavior === 'commit' || wasConfirmed) { | ||
| return 'closed'; |
There was a problem hiding this comment.
This is where the row departs from current spec:AST-057 FR5: a fling never reaches 'perform', and a swipe that confirmed and then backed out returns 'closed' even when released past the panel's width, where FR5 keeps the row resting open. Approval waits on an engineering owner updating FR5 (and FR4 for the confirmed look) or keeping it, recorded with /approve-spec <full head SHA> on this pull request.
| // two-finger pinch that starts on the row to the browser, so the page stays | ||
| // zoomable (WCAG 1.4.4). The hover transition keeps its own clock beside | ||
| // the transform's. | ||
| // Swipe actions. The gesture moves the root with its `translate` property |
There was a problem hiding this comment.
From #8097 in the stack: the row now moves by the animator's translate, and no Item source writes --_item-swipe-travel any more, while current spec:AST-057 FR2 and DEC-5 say the gesture drives the row's travel through a custom property on the root that the root's and panels' transforms read. Keep the property in #8097, or have FR2 and DEC-5 amended.
| * ONLY WHERE THE PRIMARY POINTER IS COARSE | ||
| * | ||
| * - Recognizers attach, and the host registers listeners, only while the | ||
| * primary pointer is coarse (`spec:AST-094` FR19, FR24; coarsePointer.ts). | ||
| * Where it is fine, an attach is recorded but nothing is installed: no | ||
| * document, window or element listener, no `touch-action` or other style, | ||
| * nothing delivered, so a finger on a touchscreen laptop gets the | ||
| * browser's defaults. The host follows the query: turning coarse installs | ||
| * it and styles every registration; turning fine cancels every touch in | ||
| * flight and removes it all. A development warning names the first attach | ||
| * under a fine pointer. |
There was a problem hiding this comment.
Current spec:AST-094 FR19 says recognizers attach for every pointer, with FR43's exceptions, and FR43 that gestures attach for a mouse, a pen and a finger, not only a coarse primary pointer; this host installs nothing where the primary pointer is fine. Item opts out with perEventPointer, so no row changes here, but the host's rule should follow FR19 and FR43, or have them amended, in its own pull request.
Depends on #8132. Review only this PR's change: vjeux/astryx@536a023...55dc756
Why
A swipe-action row (
Item,ListItem) fired its outermost action, often Delete, past a guessed drag length, with no warning. UIKit's full swipe confirms while the finger is down: the row runs ahead and the action stretches over it; letting go fires, backing out cancels.What led to it
Part of a series bringing Astryx's touch interactions to parity with native iOS (UIKit). Recorded on iOS 26.2, UIKit confirms 0.66–0.73 of the way from the open width to the row's edge, then runs ahead to 29 pt short of it.
Options and tradeoffs
Open owner decision:
spec:AST-057FR5 differs from UIKit in four cases (a fling; backing out after confirming; closing fast past the buttons; opening fast short of halfway). We propose amending FR5 to UIKit's rule;swipeRelease.test.tspins each case.Why this choice
Only option 2 shows a destructive action before it fires.
How to check it
On a phone, open the Item story "Swipe Actions":
We ran these with real touch in Chromium, WebKit, the iOS 26.2 simulator and the Android 15 emulator; each behaved as listed (recordings below). The unit and browser tests pass, including a confirmed row sliding out through a blocked main thread.
Evidence
Known red until #8671 merges: its touch-bundle check, which a gesture-using
Itemfails by design.Scope