Repository navigation
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
AI review status for this pull request.
|
There was a problem hiding this comment.
Approval waits on an engineering owner's decision: this head adds draft spec:AST-094 requirements and new component:Item requirements (FR9–FR13, AR3–AR5, ORD2–ORD3, PR2, DEC-3–DEC-4) that main does not yet state, and an engineering owner records that decision by commenting /approve-spec <full head SHA> on this pull request (the author may do so as an engineering owner). Whatever that decision, a few changes are still needed:
- Land the stacked pull requests first and rebase, so this one carries its own three commits instead of 122 commits and about 50,000 authored lines.
- A quick flick no longer fires a swipe row's outermost action, which current
spec:AST-057FR5 requires (packages/core/src/Item/swipeRelease.ts), so restore the fling rule or get FR5 amended onmainfirst. .changeset/touch-motion-theme-roles.mdofferstiming.reducedMotionCrossFadeMsas a theme role, butdefineThemerejects it as an unknown role.
The swipe refactor itself keeps its recorded release storyboards within tolerance.
[Automated review]
| if (isConfirmed) { | ||
| return 'perform'; | ||
| } | ||
| if (behavior === 'commit' || wasConfirmed) { | ||
| 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.
Only a confirmed full swipe returns perform here, so a quick flick opens or closes the row and never fires. Current spec:AST-057 FR5 says a fling MUST fire the outermost entry under reveal and MUST slide the row out and fire it under commit; the base fires at 0.6 px/ms past 44 px. Either restore a fling branch that performs, or have FR5 amended on main before this lands.
| '@astryxdesign/cli': patch | ||
| --- | ||
|
|
||
| [feat] Themes can retune touch motion roles. `defineTheme({motion})` takes `springs`, `durations` and `curves` keyed by motion role, the table path of a measured touch motion value (`springs.sheetPresent`, `timing.reducedMotionCrossFadeMs`), at the root or in an adaptation rule, and writes one `--motion-*` variable per role it sets. A role the theme leaves out keeps its measured default, an unknown role id is refused, and the duration scale (`fast`, `medium`, `ratio`) is optional when a theme sets only roles. No shipped component reads the roles yet. Adaptation rules gain a `platform` condition: `ios` for the browsers of iPhone and iPad, or `other`. `astryx docs motion` lists every motion role with its CSS variable and default, and the theme authoring guide and template describe the new fields. |
There was a problem hiding this comment.
timing.reducedMotionCrossFadeMs is not a theme role at this head: registerTables.ts excludes it, roleKinds.generated.ts omits it, and expandMotionRoles throws "is not a motion role" for it (reduced-motion-record.md says it is no longer one). Name a registered role here, such as navigation.transitionMs.
PR Analysis Report📚 Storybook Preview🧪 Sandbox PreviewModified ComponentsItem (@astryxdesign/core) · View in Storybook
MultiSelector (@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/BottomSheet | +33.1KB |
+98B |
| core/BreadcrumbItem | +33.8KB |
+87B |
| core/CheckboxListItem | +33.5KB |
+16B |
| core/ContextMenu | +34.6KB |
+109B |
| core/DateInput | +33.2KB |
+85B |
| core/DateTimeInput | +33.3KB |
+89B |
| core/DropdownMenu | +34.8KB |
+115B |
| core/DropdownMenuCheckboxItem | +33.2KB |
+41B |
| core/DropdownMenuItem | +33.1KB |
+34B |
| core/DropdownMenuRadioItem | +33.2KB |
+41B |
| core/DropdownMenuSubMenu | +33.2KB |
+46B |
| core/Item | +33.1KB |
+27B |
| core/ListItem | +33.5KB |
+16B |
| core/Markdown | +34.7KB |
+75B |
| core/MoreMenu | +34.6KB |
+115B |
| core/Pagination | +35.0KB |
+88B |
| core/PowerSearch | +41.0KB |
+124B |
| core/RadioListItem | +33.5KB |
+10B |
| core/Selector | +35.2KB |
+117B |
| core/SelectorOption | +33.2KB |
+29B |
| core/Table | +34.6KB |
+85B |
| core/TableCell | +34.6KB |
+63B |
| core/TableHeaderCell | +34.6KB |
+71B |
| core/TimeInput | +33.4KB |
+92B |
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 | +38B |
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
7d39d7e to
f8fa243
Compare
There was a problem hiding this comment.
Approval waits on an engineering owner's decision: this head adds the draft spec:AST-094 requirements (FR1–FR23, DEC-1 and DEC-2) that main does not yet state, plus the stable @astryxdesign/core/motion/reducedMotion.stylex subpath and the experimental useFingerScroll it marks pending owner review, and an engineering owner records that decision by commenting /approve-spec <full head SHA> on this pull request (the author may do so as an engineering owner). Whatever that decision, a few changes are still needed:
- Land the stacked pull requests first and rebase, so this one carries its own three commits instead of 141.
- A quick flick no longer fires a swipe row's outermost action, which current
spec:AST-057FR5 requires (packages/core/src/Item/swipeRelease.ts), so restore a fling rule or get FR5 amended onmainfirst. - The swipe now needs a coarse primary pointer, so a finger on a touchscreen laptop no longer swipes, against
spec:AST-057FR4 (packages/core/src/Item/Item.tsx). spec:AST-094FR17 ("never decayed through a rest") contradicts the stillness reset in currentspec:AST-100FR2, which the code already follows.- Add the physical-device swipe check (AST-057), iOS Safari and Android Chrome velocity runs (AST-100 FR4) and fine-pointer before-and-after evidence (AST-017 FR59) in the pull requests that own those changes.
The swipe refactor itself keeps its recorded release storyboards within tolerance.
[Automated review]
| const {isTouch} = useTouchPresentation(); | ||
| const hasSwipe = isSwipeServed && isTouch; |
There was a problem hiding this comment.
Serving the swipe only while the primary pointer is coarse takes it away from a finger on a touchscreen laptop. Current spec:AST-057 FR4 says "A sideways drag on a coarse pointer MUST move the row", and its Platform support says "Pointer type is read from the pointer event", which the base did per drag. Keep starting the drag from a touch pointer event wherever one can occur, or have AST-057 amended on main first.
| - **FR17 — UIKit's readings.** A recognizer's thresholds and readings MUST | ||
| follow UIKit's and come from the table. A pan begins at 10 pt of travel on | ||
| either axis, and its translation excludes that travel; its velocity is | ||
| UIKit's estimate, 0.2 × the newest event's instantaneous velocity + 0.8 × | ||
| the previous event's, never decayed through a rest. |
There was a problem hiding this comment.
"never decayed through a rest" contradicts current spec:AST-100 FR2 ("It is 0 when the contact stayed still for longer than a fixed stillness interval before lifting") and architecture:interaction-modality INV10, which VelocityTracker already follows. Rebase this record on main and defer the velocity to AST-100.
| // Proposed; pending owner review (spec:AST-094 OQ4): the Reduce Motion | ||
| // media keys and fade constants, for a transition in another package. |
There was a problem hiding this comment.
This stable subpath is marked pending owner review, but spec:AST-094 at this head has no OQ4 (only OQ1 and OQ2), so nothing records the open question it points at. Name the record that will decide it, or keep the constants internal until an owner does.
f8fa243 to
da1c3f0
Compare
There was a problem hiding this comment.
Approval waits on an engineering owner's decision: this head adds spec:AST-094 (FR1–FR40, DEC-1–DEC-4), which main does not have yet, and writing authority: current into the record does not approve it, so an engineering owner records the decision by commenting /approve-spec <full head SHA> on this pull request (the author may do so as an engineering owner) or rejects it so it is settled in #8378 first. Whatever that decision, these changes are still needed:
- Land the stacked pull requests first and rebase, so this one carries its own four commits instead of the whole touch stack.
- A quick flick no longer fires a swipe row's outermost action, which current
spec:AST-057FR5 requires (packages/core/src/Item/swipeRelease.ts), so restore a fling rule or get FR5 amended onmainfirst. - The swipe now needs a coarse primary pointer, so a finger on a touchscreen laptop no longer swipes, against
spec:AST-057FR4 (packages/core/src/Item/Item.tsx). - AST-094's FR36 and FR37 (decide by projection; a release moving back always cancels) disagree with the swipe's measured release rule, and
.changeset/touch-motion-theme-roles.mdsays no component reads the motion roles while BottomSheet already does. - Add the physical-device swipe check (AST-057), iOS Safari and Android Chrome velocity runs (AST-100 FR4) and fine-pointer before-and-after evidence (AST-017 FR59) in the pull requests that own those changes.
The swipe refactor itself keeps its recorded release storyboards within tolerance.
[Automated review]
| - **FR37 — Moving back cancels.** A release whose velocity points back | ||
| toward the start MUST cancel, whatever the travel so far. |
There was a problem hiding this comment.
FR37 (and FR36's projection rule, which it says backs "completing a row swipe") disagrees with the swipe this head ships: resolveRelease opens a slow backward release past half (packages/core/src/Item/swipeRelease.ts:69-75), as UIKit's measured releases [95, -16, 'open'] and [87.7, -5, 'open'] in swipeRelease.test.ts show. Either carve the row swipe out of FR36–FR37 with its measured velocity rule, or change the rule.
| '@astryxdesign/cli': patch | ||
| --- | ||
|
|
||
| [feat] Themes can retune touch motion roles. `defineTheme({motion})` takes `springs`, `durations` and `curves` keyed by motion role, the table path of a measured touch motion value (`springs.sheetPresent`, `navigation.transitionMs`), at the root or in an adaptation rule, and writes one `--motion-*` variable per role it sets. A role the theme leaves out keeps its measured default, an unknown role id is refused, and the duration scale (`fast`, `medium`, `ratio`) is optional when a theme sets only roles. No shipped component reads the roles yet. `astryx docs motion` lists every motion role with its CSS variable and default, and the theme authoring guide and template describe the new fields. |
There was a problem hiding this comment.
"No shipped component reads the roles yet" is no longer true at this head: BottomSheet registers the springs table before its release, presentation and dismissal springs (packages/core/src/BottomSheet/useSheetGestures.ts:1176, :1630, :1718), so a theme's springs.sheet* values change its touch motion. Name BottomSheet here.
da1c3f0 to
7e12943
Compare
There was a problem hiding this comment.
Approval waits on an engineering owner's decision: this head adds spec:AST-094 (FR1–FR40, DEC-1–DEC-4), which main does not have, and writing authority: current into the record does not approve it, so an engineering owner records the decision by commenting /approve-spec <full head SHA> on this pull request (the author may do so as an engineering owner) or rejects it so it is settled in #8378 first. Whatever that decision, these changes are still needed:
- Land the stacked pull requests first and rebase, so this one carries its own four commits instead of the whole touch stack (about 50,000 authored lines).
- A quick flick no longer fires a swipe row's outermost action, which current
spec:AST-057FR5 requires (packages/core/src/Item/swipeRelease.ts), so restore a fling rule or get FR5 amended onmainfirst. - The swipe now needs a coarse primary pointer, so a finger on a touchscreen laptop no longer swipes, against
spec:AST-057FR4 (packages/core/src/Item/Item.tsx). - AST-094's FR4 role table names 20 roles that
defineThemerejects and its FR37 contradicts the swipe's release rule, and.changeset/touch-motion-theme-roles.mdsays no component reads the roles while BottomSheet does. - Add the physical-device swipe check (AST-057), iOS Safari and Android Chrome velocity runs (AST-100 FR4) and fine-pointer before-and-after evidence for BottomSheet's mouse release (AST-017 FR59) in the pull requests that own those changes.
The swipe refactor itself keeps its recorded release storyboards within tolerance.
[Automated review]
| if (isConfirmed) { | ||
| return 'perform'; | ||
| } | ||
| if (behavior === 'commit' || wasConfirmed) { | ||
| 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.
Only a confirmed full swipe returns perform, so a quick flick opens or closes the row and never fires. Current spec:AST-057 FR5 says a fling MUST fire the outermost entry under reveal and MUST slide the row out and fire it under commit; the base fired at 0.6 px/ms past 44 px. In Chromium phone emulation a 130 px, 60 ms flick removes "Message 1" on the base and only opens the row here. Restore a fling rule, or get FR5 amended on main first.
| const {isTouch} = useTouchPresentation(); | ||
| const hasSwipe = isSwipeServed && isTouch; |
There was a problem hiding this comment.
Serving the swipe only while (pointer: coarse) matches takes it away from a finger on a touchscreen laptop, which reports its trackpad as the primary pointer. Current spec:AST-057 FR4 requires the drag on a coarse pointer and says the split is "Read from the pointer event, not a prop or a media query"; the base read pointerType === 'touch' per event. The layer can still load lazily while the decision is made per pointer event.
|
|
||
| | Kind | Roles | | ||
| | -------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | | ||
| | Spring | `springs.sheetPresent`, `springs.sheetDismiss`, `springs.sheetDismissSwipe`, `springs.sheetDetent`, `springs.sheetReleaseFast`, `springs.actionSheetPresent`, `springs.actionSheetDismiss`, `springs.alertPresent`, `springs.alertDismiss`, `springs.popoverPresent`, `springs.popoverDismiss`, `springs.contextMenuLift`, `springs.contextMenuDismiss`, `springs.menuOpen`, `springs.menuClose`, `springs.subMenuOpen`, `springs.subMenuBack`, `springs.rowSwipeOpen`, `springs.rowSwipeReturn`, `springs.rowSwipeCommit`, `springs.banner`, `springs.keyboard`, `springs.navigationPush`, `springs.navigationPop`, `springs.navigationInteractivePopFinish`, `springs.navigationBarElements`, `springs.segmentedThumb`, `springs.switchThumbMove`, `springs.switchThumbStretch`, `springs.fingerTracking`, `springs.emphasis`, `springs.settle`, `springs.quickChange`, `springs.viewChange`, `springs.timedChange`, `springs.stateChange`, `springs.directChange`, `scroll.paging.snap` | |
There was a problem hiding this comment.
This table names 20 roles that defineTheme refuses with "is not a motion role" at this head (packages/core/src/theme/expandMotionRoles.ts:143-146 against motion/roleKinds.generated.ts), including springs.rowSwipeOpen, springs.rowSwipeReturn, springs.rowSwipeCommit, springs.subMenuOpen, springs.popoverPresent, timing.stateChangeMs and timing.curves.entrance, while 15 registered roles such as springs.snappy and timing.curves.easeOut are absent. Make this the registered role list, or register the roles, before the record is approved.
| - **FR37 — Moving back cancels.** A release whose velocity points back | ||
| toward the start MUST cancel, whatever the travel so far. |
There was a problem hiding this comment.
FR37, and FR36's projection rule that it says backs "completing a row swipe", disagree with the swipe this head ships: resolveRelease opens a slow backward release past half (packages/core/src/Item/swipeRelease.ts:69-75), as the measured releases [95, -16, 'open'] and [87.7, -5, 'open'] show (swipeRelease.test.ts:43, :67), and fires only on a confirmed full swipe. The record and the code need to state one rule.
| '@astryxdesign/cli': patch | ||
| --- | ||
|
|
||
| [feat] Themes can retune touch motion roles. `defineTheme({motion})` takes `springs`, `durations` and `curves` keyed by motion role, the table path of a measured touch motion value (`springs.sheetPresent`, `navigation.transitionMs`), at the root or in an adaptation rule, and writes one `--motion-*` variable per role it sets. A role the theme leaves out keeps its measured default, an unknown role id is refused, and the duration scale (`fast`, `medium`, `ratio`) is optional when a theme sets only roles. No shipped component reads the roles yet. `astryx docs motion` lists every motion role with its CSS variable and default, and the theme authoring guide and template describe the new fields. |
There was a problem hiding this comment.
"No shipped component reads the roles yet" is not true at this head: BottomSheet registers the springs table before its release, presentation and dismissal springs (packages/core/src/BottomSheet/useSheetGestures.ts:1176, :1630, :1718), so a theme's springs.sheet* values change its touch motion. Say so here.
| if (isSwipeOpen(stateRef.current)) { | ||
| latestRef.current.onOpenChange?.(false); |
There was a problem hiding this comment.
This unmount cleanup calls onOpenChange(false), which is Item's setSwipeOpen, so an Effect sets Item's state, and its comment names no external system (architecture:react-component-runtime INV2, INV3). Deriving Item's press gate from hasSwipe && isSwipeOpen keeps a removed layer from leaving the row unpressable without the cleanup.
Takes the wave and the gesture line's new base. The motion foundation's files and the gesture table's open captures take the base's side; the AST-094 record is main's (no lane copy); the motion README keeps the swipe actions table's row.
…0b0002) into meta#8344 C12 rebuilt meta#8134 after the 6520284 this PR carried, so the swipe files are merged three-way against that head: C12's changes to the tests and the a11y spec come in, and this PR's driver keeps the settle (one clock for the root and every part by construction, which C12's shared start also gives the per-part settle): useSwipeAction.ts keeps the driver's version of that block and drops the per-part start state it no longer needs; swipeRelease is this PR's (C12 left it unchanged). The motion README keeps every row.
…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.
…erting it The settle test asserted today's settle, the theme's 300 ms --ease-standard transition, frame by frame. That curve starts a release at rest at about 1,400 px/s, where spec:AST-094 FR38 starts a BottomSheet's release at the finger's speed, so the test pinned a curve the spec rules out. It now records the curve, its easing, its duration and its frame-by-frame fit, in the evidence and an annotation, as it already did for the velocity hand-off, and keeps asserting where the settle lands: the half detent.
Folds this pull request onto the canonical gesture foundation, meta#8360's head, which carries meta#8076, and onto main, in place of the old meta#8076 and harness copies it carried. The one conflict is the motion README's Files table: meta#8360's table, with this pull request's progress.ts row added after presentation.ts.
There was a problem hiding this comment.
Approval waits on an engineering owner's decision about current spec:AST-057: the swipe this pull request would merge follows UIKit's release rule (a flick opens or closes a row and never fires; only a full swipe fires), closes an open row on a scroll or on a tap on its own content that then activates nothing, and moves the row by its translate property, which departs from FR5, AR3, FR7 and FR2. An engineering owner either updates those claims for this behaviour, on main or as a spec change in this pull request with the decision recorded by commenting /approve-spec <full head SHA> here (the author may do so as an engineering owner), or the row keeps AST-057's rules. Whatever the decision, these changes are still needed:
- Land the predecessor pull requests and rebase, so this one carries its own seven commits instead of about 49,200 authored lines.
- In the pull requests that own them, make the gesture host attach for every pointer (
spec:AST-094FR19, FR43), play the swipe row's springs through motion roles (FR4), and hold an entering non-modal sheet without cancellingtouchmove(FR19). - Make the driver's offset layer follow a track rule's state in
track(), or narrow its comment. - Add a physical touch-device check of the swipe (AST-057), device runs for the "every touch device" claims (AST-094) and fine-pointer before-and-after evidence for BottomSheet's mouse release (
spec:AST-017FR59) in the pull requests that change them.
[Automated review]
| if (isConfirmed) { | ||
| return 'perform'; | ||
| } | ||
| if (behavior === 'commit' || wasConfirmed) { | ||
| 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.
Only a confirmed full swipe returns perform, so a flick opens or closes the row and never fires, and a release past the panel's width that moves back at 48 px/s or more closes. Current spec:AST-057 FR5 says a fling MUST fire the outermost entry under reveal and commit and a release past the panel's width MUST leave the row resting open under reveal, and spec:AST-094 FR36 says "A row swipe keeps spec:AST-057's own release rule". This needs an engineering owner to amend FR5 (on main, or here with /approve-spec on this head), or the rule kept as FR5 states it.
| event.target.closest('[data-swipe-panel]') != null) | ||
| ) { | ||
| return; | ||
| // A tap on an open row's own content closes it and activates nothing, as |
There was a problem hiding this comment.
A tap on an open row's own content now closes it and activates nothing (and Item.tsx:852 drops the root's press while open). Current spec:AST-057 AR3 says swipeActions MUST NOT change the primary element's activation, and FR7 does not list this closing or the scroll closing at :896-941; both wait on the same owner decision as the release rule.
| * 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.
This gates the host on a coarse primary pointer and cites FR19 for it, but current spec:AST-094 FR19 says "Recognizers attach for every pointer, with FR43's exceptions", FR43 "Gestures attach for every pointer, a mouse, a pen, and a finger, not only a coarse primary pointer", and architecture:react-component-runtime INV13 "The gesture host attaches for every pointer, with no touch-only query". Drop the gate in the gesture-host pull request, as the 27 October owner ruling asked.
| animator: entry, | ||
| property: 'opacity' as const, | ||
| value: (track: SwipeTrack) => 1 - confirmOf(track), | ||
| spring: SWIPE.tracking, |
There was a problem hiding this comment.
SWIPE.tracking (and SWIPE.settle at :758) is played as a literal: no registerMotionTable call registers the swipeActions table, so these springs are not motion roles. spec:AST-094 FR4: "A component MUST read touch motion through these roles and MUST NOT play a literal." Register the swipe's springs as a motion table and play them through their roles.
| // One finger: no scroll. Two: the page's pinch. | ||
| if (touchEvent.touches.length === 1 && event.cancelable) { | ||
| event.preventDefault(); | ||
| } |
There was a problem hiding this comment.
This cancels a one-finger touchmove that no recognizer owns. spec:AST-094 FR19 allows that only under a modal scroll lock, for a control keeping its press, or for a scroll-edge pan, and an entering non-modal sheet has none of them. The modal hold gets the same result from its shield's touch-action; the same touch-action on the held sheet would keep this hold within FR19.
| * swipe's slide, with no wrapper element to carry either. A track rule's | ||
| * moves write it 1:1, the offset stepping to its new value. |
There was a problem hiding this comment.
track() never moves an offset layer: it calls updateStateLayers (:376-387), which covers state layers only, and write adds the offset last landed or placed (:328-331). Either set each offset layer's offset from the track rule's new state in track(), with a test, or narrow this comment.
Depends on #8341, #8134 and #8340. Review only this PR's change: https://github.com/vjeux/astryx/compare/c344cfeece01b471f11f14e2f9c2059f0b582e29..84a4c89c7ee09b8849f62cc8bfb310efe6af3350
Primary intent
Item's swipe actions run on the shared interactive-transition driver from #8341, with no change in how a row looks, moves or fires.
User or maintainer impact
No visible change. For maintainers, BottomSheet and Item's swipe each carried their own drag-and-release animation plumbing; after this, both use one driver, so a fix to how a released drag settles lands once.
Problem and solution fit
#8341 built the driver for the sheet. Item's swipe (#8134) still ran its own curves (
swipeCurves,withSlide), animating the row, its track and each action by hand. Options:place(reduced motion's instant follow) and offset layers (an action's pin plus its slide on one translate).The swipe's two rules, where a row tracks and where a release lands, become pure functions equal to #8134's on a grid of sides, behaviors and confirmation states.
Authority
Item.spec.mdand AST-057 govern the swipe, unchanged; one touch model (spec:AST-094);design:reduced-motion;spec:AST-107FR7 with INV13's gaps (#8751).Change and evidence
Seven swipe recordings from before the move (opening, closing, a flick, a full swipe backed out, one that fires, one started late, and reduced motion) hold within 0.5 px and 0.005 opacity after it, with the same fire times. The comparison fails if any curve changes.
Scope
Testing
How to check: in Item's swipe stories, swipe a row open and closed, flick it, back a full swipe out and let one fire, with and without Reduce Motion; each looks and times as on the base.
What ran: the whole core and lab packages pass; Item's and the sheet's browser tests pass in Chromium (56) and WebKit (177; one harness self-test fails on its own, fixed by #8535).
touch-support-bundlefails at this head, since BottomSheet now brings the touch module into the root entry; #8671 replaces that whole-entry check with a per-component one, which passes. Size: Item's JS 43.7 → 45.5 KB gzip, as it now imports the shared driver; BottomSheet's 68.1 → 68.7 KB. These checks ran locally; no CI run is expected on this pull request.