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:design-review
![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 about the governing spec: spec:AST-107 FR3–FR6, which defines createAnimator, the @astryxdesign/core/animator entry and useAnimator, is only in open #8600. On main, architecture:react-component-runtime INV13 and spec:AST-017 FR21 still keep motion internal, and architecture:public-component-api INV21 has no carve-out yet for a hook that wraps a factory. Any engineering owner can settle it by landing #8600 or by commenting /approve-spec <full head SHA> on this pull request, and the author may do so as an engineering owner. Whatever that decision, these need fixing:
- Land #8076 first and rebase: this diff carries 17,491 authored lines, and that stack alone differs from
mainby 15,672. scripts/motion-node-load.test.mjsstill imports the renamedcreateAnimatorfrommotion/animatorand fails.- The two playing-card templates need a Changeset naming
@astryxdesign/cli, a refreshed ShadCN route lock and the Sandbox's static route list; lint, docsite-data and build-sandbox fail without them. - The templates should read zero release velocity after a still pause and deal the next card once the old one fades, and the docs should call
animate's motion Astryx's state-change spring rather than the theme's, since roles stay internal underspec:AST-094DEC-11.
[Automated review]
| * the press paint's styles). | ||
| */ | ||
| const ENTRY = ` | ||
| export {createAnimator, animatorFor, SETTLE_THRESHOLD} from './motion/animator'; |
There was a problem hiding this comment.
This still re-exports createAnimator from ./motion/animator, which this PR renames to createMotionAnimator. esbuild leaves the binding undefined, and the child-process test fails with TypeError: m.createAnimator is not a function. Lines 103, 106 and 115 need the same rename.
| /** The release velocity in px/s, over the drag's last 80 ms. */ | ||
| function releaseVelocity(samples: Sample[]): {x: number; y: number} { | ||
| const last = samples.at(-1); | ||
| const first = samples.find(s => last != null && last.t - s.t <= 80); |
There was a problem hiding this comment.
The release velocity comes from the last 80 ms of moves and ignores how long the pointer rested before pointerup. A drag that stops and holds still for 600 ms before release still throws the card away (Chromium), although the template says a gentle release springs it back. spec:AST-100 FR2 reads zero after a still pause. Passing the release's timeStamp and returning zero when the newest sample is older than about 80 ms would fix it, here and in the plain-JavaScript template's onUp.
| const speed = Math.hypot(velocity.x, velocity.y); | ||
| const boost = speed < MIN_THROW ? MIN_THROW / Math.max(speed, 1) : 1; | ||
| const throwX = speed < 1 ? MIN_THROW : velocity.x * boost; | ||
| await Promise.all([ |
There was a problem hiding this comment.
This waits for the whole coast, about 800 px at 1,600 px/s. After Discard, the table stays empty and the live region silent for 4.55 s before the next card is dealt. Dealing and announcing once animate({opacity: 0}) resolves, then calling stop(), keeps the keyboard path responsive.
| name: 'animate', | ||
| type: '(target: {x?, y?, scale?, rotate?, opacity?}, options?: {velocity?}) => Promise<void>', | ||
| description: | ||
| "Springs the given channels to `target` from wherever they are on screen, on the theme's state-change motion, and resolves when they arrive (or as soon as a later call takes them over). `x` and `y` are px, `scale` is 1 at the element's size, `rotate` is degrees and `opacity` runs from 0 to 1. Pass `velocity` (units per second, such as a drag's release velocity) to start with it; without it a channel keeps the velocity it has on screen, so interrupting a motion never jumps.", |
There was a problem hiding this comment.
"On the theme's state-change motion": the animator plays springs.stateChange's table value, and no theme can retune it while spec:AST-094 DEC-11 keeps the roles internal (the motion README says the same). "Astryx's state-change spring" would be accurate; the same wording is at line 77, in useAnimator.doc.mjs lines 33 and 38, in the changeset and in the createAnimator JSDoc.
| @@ -0,0 +1,7 @@ | |||
| --- | |||
| '@astryxdesign/core': patch | |||
There was a problem hiding this comment.
This names only @astryxdesign/core, but the two new playing-card templates ship as @astryxdesign/cli output, and CI lint's changeset coverage fails on that. A separate [feat] changeset naming @astryxdesign/cli would clear it (spec:AST-017 FR7).
meta-cla
Bot
added
the
CLA Signed
There was a problem hiding this comment.
Approval waits on an engineering owner's decision about the governing spec: spec:AST-107 FR3–FR6, which defines createAnimator, the @astryxdesign/core/animator entry and useAnimator, is only in open #8600, while on main, architecture:react-component-runtime INV13 and spec:AST-017 FR21 keep the animator internal and architecture:public-component-api INV21 has no exception yet for a hook over a factory. Any engineering owner can settle it by landing #8600 or by commenting /approve-spec <full head SHA> on this pull request, and the author may do so as an engineering owner. Whatever that decision, these need fixing:
- Land #8076 first and rebase: this diff carries 17,530 authored lines, 15,672 of them from that stack.
scripts/motion-node-load.test.mjsstill imports the renamedcreateAnimatorfrommotion/animator(lines 31, 103, 106 and 115), so its child-process test fails.useAnimator.doc.mjsline 33 still says calls work before the ref attaches, which this head no longer does.- The templates should read zero release velocity after a still pause and deal the next card once the old one fades, and the docs and JSDoc should drop
usePanGestureanduseAnimatedLayout, which the package does not have yet, and call the motion Astryx's state-change spring rather than the theme's.
[Automated review]
| * the press paint's styles). | ||
| */ | ||
| const ENTRY = ` | ||
| export {createAnimator, animatorFor, SETTLE_THRESHOLD} from './motion/animator'; |
There was a problem hiding this comment.
./motion/animator now exports createMotionAnimator, not createAnimator, so this re-export finds nothing and m.createAnimator(null) at line 103 throws in the child process: the second test fails whether the bundler stops on the missing name or leaves it undefined. Lines 103, 106 and 115 need the same rename.
| name: 'animator', | ||
| type: 'AnimatorHandle', | ||
| description: | ||
| "The element's animator, the same object on every render: `animate(target, {velocity?})` springs channels to a target on the theme's state-change motion, `decay({x?, y?})` coasts them from a release velocity, `set(values)` jumps them, `get()` and `getVelocity()` read them, and `stop()` halts them. The channels are `x` and `y` (px), `scale` (1 at the element's size), `rotate` (degrees) and `opacity` (0 to 1). `animate` and `decay` return a promise that resolves when the motion ends. Calling it never re-renders the component, and it works before the ref attaches: the element shows its values when it mounts.", |
There was a problem hiding this comment.
"It works before the ref attaches: the element shows its values when it mounts" no longer holds: before the ref attaches every call does nothing, as line 27 and the hook's test an animator used before its ref attaches does nothing and resolves at once say.
| function releaseVelocity(samples: Sample[]): {x: number; y: number} { | ||
| const last = samples.at(-1); | ||
| const first = samples.find(s => last != null && last.t - s.t <= 80); | ||
| if (last == null || first == null || last.t === first.t) { | ||
| return {x: 0, y: 0}; | ||
| } | ||
| const seconds = (last.t - first.t) / 1000; | ||
| return {x: (last.x - first.x) / seconds, y: (last.y - first.y) / seconds}; | ||
| } |
There was a problem hiding this comment.
The 80 ms window is measured back from the newest move, not from the release. After a fast drag that stops and holds still, no more moves arrive, so the release still reads the earlier speed and throws the card, although the copy says a gentle release springs back. Passing the release event's timeStamp and returning zero when the newest sample is older than the window would fix it, here and in the plain-JavaScript template's onUp.
| await Promise.all([ | ||
| animator.decay({x: throwX, y: velocity.y * boost}), | ||
| animator.animate({opacity: 0}), | ||
| ]); |
There was a problem hiding this comment.
This waits for the whole coast: from the Discard button, 1,600 px/s travels about 800 px and settles about 4.5 s later, so the table stays empty and the live region silent long after the card has faded. Dealing and announcing once animate({opacity: 0}) resolves, then calling stop(), keeps the keyboard path responsive.
| export interface AnimatorAnimateOptions { | ||
| /** | ||
| * Each channel's velocity at the start, such as a drag's release velocity | ||
| * (`usePanGesture` reports it at `ended`). A channel without one keeps the |
There was a problem hiding this comment.
usePanGesture is not in the package yet, so this public JSDoc points callers to a hook they cannot import.
| ], | ||
| usage: { | ||
| description: | ||
| "Animates one element with the motion Astryx's own components use, without React. `animate` springs an element to new values on the theme's state-change motion, `decay` flings it with the momentum of a released scroll view, and `set` moves it directly, so a card can follow a finger, snap back and be thrown away. Use the `useAnimator` hook in React. To animate an area's layout as its items change, use `useAnimatedLayout`; for what a component already animates, such as a sheet's detents or a carousel's snapping, use the component's own props.", |
There was a problem hiding this comment.
Two claims here go beyond the package: no theme can retune springs.stateChange while motion roles stay internal (spec:AST-094 DEC-11), so "Astryx's state-change spring" would be accurate, and useAnimatedLayout does not exist in the package yet. The same wording is at line 38, in useAnimator.doc.mjs lines 33 and 38, in the changeset and in the createAnimator JSDoc.
…oduction registry checks The same two checks meta#8653's fix meets, for this pull request's own docs: - createDragToSnapPoints' doc is a core doc that component <Name> reads but the component list leaves out (a vanilla factory), so the CLI's doc inputs test pins it. - The useDragToSnapPoints example imports the hook from the released hooks subpath, so the hidden production registry built it against the published core, which has no such hook yet. Its doc now names useDragToSnapPoints among the components it uses, so production skips it until the hook is released. A latest-target generate shows the item without this line and drops it with it.
There was a problem hiding this comment.
Approval waits on an engineering owner's decision about the governing spec: spec:AST-107 FR3–FR6, which defines createAnimator, the @astryxdesign/core/animator entry and useAnimator, is only in open #8600, while on main, architecture:react-component-runtime INV13 and spec:AST-017 FR21 keep the animator internal and architecture:public-component-api INV21 has no exception yet for a hook over a factory. Any engineering owner can settle it by landing #8600 or by commenting /approve-spec <full head SHA> on this pull request, and the author may do so as an engineering owner. Whatever that decision, these need fixing:
- Land #8076 first and rebase: this diff carries 17,538 authored lines, 15,669 of them from that stack.
useAnimator.doc.mjsline 33 still says calls work before the ref attaches, which this head no longer does.- Both templates should read zero release velocity after a still pause, and deal and announce the next card once the old one fades rather than after the whole 4.5 s coast.
- The docs and JSDoc should drop
usePanGestureanduseAnimatedLayout, which the package does not have yet, and call the motion Astryx's state-change spring rather than the theme's.
[Automated review]
There was a problem hiding this comment.
Approval waits on an engineering owner's decision about the governing spec: spec:AST-107 FR3–FR6, which defines createAnimator, the @astryxdesign/core/animator entry and useAnimator, is only in open #8600, while on main, architecture:react-component-runtime INV13 and spec:AST-017 FR21 keep the animator internal and architecture:public-component-api INV21 has no exception yet for a hook over a factory. Any engineering owner can settle it by landing #8600 or by commenting /approve-spec <full head SHA> on this pull request, and the author may do so as an engineering owner. Whatever that decision, these need fixing:
- Land #8076 first and rebase: this diff carries 17,663 authored lines, 15,794 of them from that stack.
useAnimator.doc.mjsline 33 still says calls work before the ref attaches, which the hook does not do.- Both templates should read zero release velocity after a still pause, and deal and announce the next card once the old one fades rather than after the whole 4.5 s coast.
- The docs and JSDoc should drop
usePanGestureanduseAnimatedLayout, which the package does not have yet, and call the motion Astryx's state-change spring rather than the theme's.
[Automated review]
PR Analysis Report
Generated by PR Enrichment workflow | View current CI run |
github-actions
Bot
added
the
needs:spec-owner-review
There was a problem hiding this comment.
Requesting changes: the public createAnimator and useAnimator match spec:AST-107 FR3–FR6, which is now on main, but this branch also carries the open #8076 and #8677 stacks (19,819 authored lines in all), so please land those first and rebase onto main. These fixes are needed in this pull request's own files:
useAnimator.doc.mjsline 33 says calls work before the ref attaches; they do nothing until it does.- Both playing-card templates should read zero release velocity after a still pause (a 150 px drag held still for 600 ms is still thrown away).
- Both templates should deal and announce the next card once the discarded one fades, not after the whole 4.5 s coast.
- The docs and the
velocityJSDoc should dropuseAnimatedLayoutandusePanGesture, which the package does not export yet.
[Automated review]
| name: 'animator', | ||
| type: 'AnimatorHandle', | ||
| description: | ||
| "The element's animator, the same object on every render: `animate(target, {velocity?})` springs channels to a target on the theme's state-change motion, at its tempo and character, `decay(velocity)` coasts them from a release velocity, `set(values)` jumps them, `get()` and `getVelocity()` read them, and `stop()` halts them. The channels are `x` and `y` (px), `scale` (1 at the element's size), `rotate` (degrees) and `opacity` (0 to 1). `animate` and `decay` return a promise that resolves when the motion ends. Calling it never re-renders the component, and it works before the ref attaches: the element shows its values when it mounts.", |
There was a problem hiding this comment.
"It works before the ref attaches: the element shows its values when it mounts" no longer holds: before the ref attaches every call does nothing and resolves at once, as line 27, createAnimator.ts (inactive()) and the hook's test say.
| } | ||
| origin = null; | ||
| const last = samples.at(-1); | ||
| const first = samples.find(s => last != null && last.t - s.t <= 80); |
There was a problem hiding this comment.
The window is measured back from the newest move, not from the release, so a drag that stops and rests before pointerup is still thrown: a 150 px drag held still for 600 ms lands at about 828 px in Chromium. Passing the release event's timeStamp to onUp and returning zero when the newest sample is older than 80 ms would fix it.
| async function discard(vx: number, vy: number) { | ||
| const speed = Math.hypot(vx, vy); | ||
| const boost = speed < MIN_THROW ? MIN_THROW / Math.max(speed, 1) : 1; | ||
| await Promise.all([ |
There was a problem hiding this comment.
This waits for the whole coast, so the next card comes about 4.5 s after Discard although the card is transparent long before. Dealing once animator.animate({opacity: 0}) resolves, then calling stop(), keeps the keyboard path responsive.
|
Thanks.
|
There was a problem hiding this comment.
Requesting changes: the public createAnimator (package root) and useAnimator (@astryxdesign/core/hooks) match spec:AST-107 FR3–FR6, and the nested-theme springs and the plain-JavaScript card are fixed, but 29,619 of this branch's 31,770 authored lines still come from the open #8076, #8360, #8661 and #8677. Please land those first (or drop them from this branch) and merge main, so the diff holds only the animator's 20 files and its rename edits, which spec:AST-107 FR5 and FR6 then cover on their own. One small fix for the animator itself: the createAnimator page says it writes only translate, scale, rotate and opacity, but it also sets an inline will-change while it moves (restored 500 ms after), which the element description should mention.
[Automated review]
|
Thanks.
|
There was a problem hiding this comment.
Requesting changes: the public createAnimator (package root) and useAnimator (@astryxdesign/core/hooks) match spec:AST-107 FR3–FR6, and the createAnimator page now describes the temporary will-change, but 29,619 of this branch's 31,770 authored lines still come from the open #8076, #8360, #8661 and #8677. Please land those first (or drop them from this branch) and merge main, so the diff holds only the animator's 20 files and its rename edits, which spec:AST-107 FR5 and FR6 then cover on their own.
[Automated review]
8df516e to
587445a
Compare
github-actions
Bot
removed
the
needs:spec-owner-review
There was a problem hiding this comment.
Requesting changes: the public createAnimator (package root) and useAnimator(ref) (@astryxdesign/core/hooks) match spec:AST-107 FR3–FR6, but 13,339 of this branch's 15,546 authored lines still sit in files the open #8071 and #8076 change. Please land those (or drop them) and merge main so the diff holds only the animator, and fix two issues in its own files:
- Both playing-cards examples' Discard button calls
decay, which keeps its momentum under Reduce Motion, so on a touch screen a tapped card slides about 260 px while it fades; keepdecayfor a released flick, have the button callanimate({x: 400, opacity: 0}), and make the docs' keyboard guidance say a control callsanimate. - A
decaythat mixesxorywithscale,rotateoropacityawaits two internal handles, so a later call that takes overxleaves its promise pending until the other coast ends; resolve it as soon as either handle is interrupted, asanimatedoes.
[Automated review]
| <Button | ||
| label={`Discard ${card.name}`} | ||
| variant="secondary" | ||
| onClick={() => void discard({x: MIN_THROW, y: 0})} |
There was a problem hiding this comment.
Under Reduce Motion on a touch screen, this tap coasts the card about 260 px during its 200 ms fade. discard starts decay({x: 1600}), and the animator keeps a coast's momentum because it treats it as a person's throw (design:reduced-motion DR2). Calling animator.animate({x: 400, opacity: 0}) here, and keeping decay for a released flick, gives an in-place fade under Reduce Motion and the same exit otherwise.
| ); | ||
| } | ||
| } | ||
| const onDiscard = () => void discard(MIN_THROW, 0); |
There was a problem hiding this comment.
Same as the React example: the button reaches decay({x: 1600}), so under Reduce Motion on a touch screen the card slides about 260 px while it fades. animator.animate({x: 400, opacity: 0}) for the button path would fade it in place there.
| name: 'Keyboard and assistive technology', | ||
| category: 'Keyboard', | ||
| description: | ||
| 'An animator moves an element; it adds no way to act. Give every action an animation stands for a control a keyboard and a screen reader can reach, such as a button that discards a card, and call the same `animate` or `decay` from it. Keep focus on an element that stays on screen: move it before you fling its element away.', |
There was a problem hiding this comment.
"Call the same animate or decay from it": a decay from a button keeps its momentum under Reduce Motion, because the animator treats every coast as a throw. Saying a control calls animate (and that decay follows a person's throw) would keep products inside DR2. The same sentence is at useAnimator.doc.mjs:68.
| if (Object.keys(others).length > 0) { | ||
| moving.push(motion.play(others).finished); | ||
| } | ||
| await Promise.all(moving); |
There was a problem hiding this comment.
After decay({x: 1200, rotate: 90}), a later set({x: 0}) resolves the translate handle at once, but this Promise.all waits for the rotate coast (about 4 s). animate resolves as soon as any of its channels is taken over (spec:AST-107 FR3). Resolving when either handle finishes false, or when both finish, would match.
meta#8360's change on the internal touch foundation: one click guard shared by the menu press and the gesture host (its click listener stays once the menu press arms it, as its record has it, and lasts only while armed for the gesture host, FR24), the host as the one touch owner (finger scroll, page holds, attachments, the press tracker hook and a held control's kept touch, the long press's single source, presented hit testing on iOS WebKit), its cases in the gesture browser proof (Chromium and WebKit) and the README. It carries meta#8074, meta#8270, meta#8293, meta#8315, meta#8320 and meta#8356, and folds in meta#8358 (the long press's tolerance comment and an unused import). Rebased onto main after section 2 as one commit; the touch-action rule came with meta#8071. Where fingers come from their pointer events, a claimed finger's touchend is cancelled from its element (FR21), under a fine primary pointer wherever the browser sends Touch Events and inside an open shadow root too; at most eight such lifts wait for their touchend, and each goes once taken or after 500 ms, kept across the host's uninstall, which can run in the microtask after the pointerup, before the touchend that comes later in the same task (FR24). An armed press's page listeners go once the arena has decided its last touch, a tap that waits on a double tap included, and after its last detach the host holds no listener, note, frame or timer (FR24). The animator's busy proof is judged against the harness's compositor-only control (meta#8906), with a main-thread mutant as its negative, and the spring's curve is placed against the control's own clock, so the late frames a loaded runner sends fail nothing. With the host serving every pointer (meta#8071), this change's own pieces follow it: a press tracker takes a mouse press as a press, painting at once and braking no scroll (FR11); a finger's or a pen's hold that does something keeps text unselected under any primary pointer, while a mouse keeps its released behavior (FR41); and `perEventPointer`, from before FR43, changes nothing. These come from meta#8665, which keeps only the undecided rule for text selection during a mouse or pen drag. The hover recognizer and the host's hover routing land with hover's first user. Co-authored-by: Cindy Zhang <cindyxz@meta.com>
…finger landed On iOS WebKit the host resolves moving content at touch-down and asks the engine what is under the finger. When a busy main thread hands it the touch later, the content has moved on: the host now asks at the point where what was under the finger at the touch's timestamp is drawn, so a tap on a moving sheet's Done still clicks Done (spec:AST-094 FR11). A presented source reports its offset at a given time for this. The presented browser proof drives the sheet with Core's animator and a critically damped spring, up into place and down off the screen over the scrim: a tap on the drawn Done clicks it once, and a drag on the drawn sheet catches it where it is drawn, in both directions.
…the finger (FR11, FR19); INV13 names the shared input listener Where the engine hit-tests an element a compositor animation moves at the motion's start (iOS WebKit), a touch on that content goes to what was drawn under the finger at touch-down, and a tap there activates that element with one click while the browser's own click is cancelled or swallowed (FR11). While such content around a recognizer that claims moving touches is moving, the document may carry a non-passive touchmove listener that cancels only owned moves and comes off when the motion ends; while it is on, a native scroll waits for a free main thread (FR19). The gesture host, press feedback, page hold, finger-scroll watch and click guard hear document-wide input through one capture-phase listener per event type on the window (architecture:react-component-runtime INV13).
…view fixes A touch the host handles late now also leaves alone content that slid under the finger after it landed: the touch goes to what was beneath it then (spec:AST-094 FR11). The input-stream changeset says what moved: the gesture host's listeners, and the press feedback's scroll and visibility listeners, since its pointer listeners were already on the window's capture phase. The coarse-pointer gate reads TOUCH_QUERY from touchQueries.ts instead of spelling it again. The WebKit interaction job's scope covers every module the gesture host now bundles: hooks/isRtlElement and utils' flatTree, hapticSwitch, inputStream, clickGuard, selectionLock and coarsePointer. The scope test's example of a util outside it moves from clickGuard, which the gesture fixture now bundles, to focusReturn. The presented proof checks that the sheet moves on a spring (its speed changes along the animator's curve) and no longer compares the drawn box with the animator's offset: under a 3-CPU cap the two disagree by 3 to 27 px in about half the frames whatever reference is used, while the tap and drag cases go through the host's real path.
…e origin most frames agree with Under the CI lane's WebKit profile (four CPUs, one worker), "a spring keeps advancing every frame through a 1 s main-thread block" failed in two ways, each about once in a few hundred runs, and a failing run's frames show each cause: - The spring drifted 138.6 ms against the control. The control's clock took its origin from the first usable frame alone, reading that frame's direction from the next usable one: the frame sat at x 210 on its way out, the next usable one at x 210 after the turn, and the tie read as "back". The clock now lives in controlClock.ts and takes the origin most frames agree with. On that run's own frames it reads 6.5 ms (bound 12), and its unit case for that layout fails on the old reading (150.7 ms astray). - The spring "moved on 61 of the 62 frames the control moved". At the block's end it was in its tail, under two of the cast's 1 px steps a frame, where a frame delivered early shows no step and the next catches up two. The harness now reads such a tail at its own pace (busyWindowProof), and the proof judges the whole block: on that run's own frames the earlier proof fails it and this one passes. No bound changes.
… (architecture:public-component-api INV23) Main's lint rule now keeps every private data attribute in its owner module. This change had moved `data-astryx-haptic-switch` from the haptics module to utils/hapticSwitch.ts, so the click guard could skip the hidden switch without importing haptics (architecture:react-component-runtime INV13). The name now lives in privateAttributes/hapticSwitch.stylex.ts, modelled on segmentDisabled.stylex.ts: it exports markHapticSwitch, which the haptics module uses on its hidden switch, and isHapticSwitch, which the click guard reads. utils/hapticSwitch.ts and the deprecated gestures/hapticSwitch.ts re-export go, the haptics module's baseline entry goes with its raw name, and the WebKit interaction job's scope covers the new module. No behavior changes.
…trl-wheel (spec:AST-094 FR43) meta#8676's reviewed change, rebuilt as one commit on meta#8360's head: a wheel gesture drives the recognizers that allow its scroll type as a contact moving with the content, a pan along its own axis once every scroller between it and the page is at its edge, and a ctrl-wheel drives a pinch. A screen-edge pan stays a finger's.
…ain input reads as a mouse wheel, and a pan can take a swipe as one step (spec:AST-094 FR43) The round-4 trackpad behaviors, as a draft for the owner to try each one: - wheelSegmenter.ts splits the wheel stream into gestures and reads a trackpad's lift: once a run of shrinking deltas has lost a fifth of its size, momentum began. A pan the host follows ends there at the fingers' travel, released at their last move with their speed, and the momentum after it never moves the pan. Fingers that stop release still, 180 ms (vertical) or 220 ms (horizontal) after their last wheel. - Input the deltas cannot place reads as a mouse wheel: a delta a hair off a whole pixel, a scroll with no legacy wheelDelta (an engine that sends none), a legacy wheelDelta that is not three times the delta, or a 120 tick over a 40 px delta. - A pan created with wheelStep takes a trackpad swipe as one step: a short flick once the swipe travels 24 px, a slow swipe's travel carried across pauses, its slow tail absorbed, and one undo back to rest. A mouse wheel's gesture and a pinch keep their 100 ms end. In Chromium the harness's wheels arrive as notches (wheelDeltaY -120 whatever the delta), which now read as a mouse wheel, so the two trackpad-only browser cases run in WebKit, whose driver sends a trackpad's precise deltas. The lift's release is stamped at the fingers' last move: stamped at the lift, three held wheels later, it read as a stop and lost the fling.
…speed until the splitter decides, so it never stalls before a lift (spec:AST-094 FR43) The splitter reads a lift only after four shrinking deltas, so a pan that held them stood still for that stretch (about 64 to 130 ms) and then let go at the fingers' speed: a stop, then a go. Through the undecided stretch the pan now moves on from where it last was at the fingers' last speed, never past what that speed projects for the time since. At the lift it keeps that travel and lets go there, still moving at that speed, and nothing more moves. When the fingers carry on, their moves absorb what the stretch showed past their travel or short of it, half a move at a time, so the pan never steps back and ends on their travel; fingers that stop without a lift end on their travel where it runs ahead.
…t, and never a mouse wheel (spec:AST-094 FR43, DEC-24) The merged record decides it: a trackpad's two-finger scroll is direct manipulation, so a pan follows it where its record allows it; a mouse wheel's notches are a scrolling control, so a pan takes none, and a scroll without the trackpad signature, Shift+wheel included, counts as a wheel's. - A pan's allowedScrollTypes takes only 'continuous', and none by default, as UIKit's allowedScrollTypesMask: the sheet, the pager and swipe rows opt in. The host never feeds a wheel's notches to a pan, whatever a caller passes. ScrollType keeps 'discrete' for the pinch, which takes a ctrl-wheel from a trackpad or a mouse. - A scroll with Shift held reads as a mouse wheel's. - wheelStep goes: DEC-24 rejects a discrete step per trackpad swipe for rows and paging, and nothing used it. - The splitter's verification streams: ten swipes that each start in the last one's momentum, ten whose momentum runs out, and a slow drag with 150 ms pauses count ten, ten and one gestures.
The public animator (spec:AST-107 FR3, FR6): createAnimator(element) over the internal animator, with animate, decay, set, get, getVelocity, stop and destroy, from the package root; and useAnimator(ref), which takes the element's ref like the gesture hooks, so a gesture hook can share the element, and returns the animator. After every commit the hook follows the element behind the ref. The internal factory is renamed createMotionAnimator, so no internal factory shares a public factory's name. Docs, a changeset, and playing-cards templates in React and plain JavaScript, with their registrations.
587445a to
28cc019
Compare
github-actions
Bot
added
the
needs:spec-owner-review
There was a problem hiding this comment.
This needs an engineering owner's decision before it can be approved, and it needs changes either way. The branch carries three record changes from #8360 that main does not state (a new MUST in spec:AST-094 FR11, a MAY in FR19 and a new sentence in architecture:react-component-runtime INV13); an engineering owner can approve them on this head by commenting /approve-spec with its full SHA (the author may, as an engineering owner), or they can reach main through #8360 first.
- Land or drop #8360 and #8676 and merge
main, so the diff holds only the animator's own changes (about 2,400 of today's 21,800 lines). - Have
useAnimatorcall the shareduseAttachmentinstead of its own two layout effects (useAnimator.ts:62-69). - Name the card in both playing-cards examples with a role or hidden text instead of an
aria-labelon a plaindiv(AnimatorPlayingCards.tsx:141,AnimatorPlayingCardsVanilla.tsx:22).
[Automated review]
| useIsomorphicLayoutEffect(() => { | ||
| attach(ref.current); | ||
| }); | ||
| // Lets the element go when the component unmounts. | ||
| useIsomorphicLayoutEffect(() => () => attach(null), [attach]); |
There was a problem hiding this comment.
These two layout effects re-implement useAttachment (packages/core/src/gestures/useAttachment.ts:62-99), which already keeps an attach bound to the element a ref holds after each commit, handles replacement and releases at unmount. Calling it here (moved to a gesture-free module so this hook's import stays free of gesture code) keeps one path for both the gesture hooks and the animator.
| onPointerUp={onPointerUp} | ||
| onPointerCancel={onPointerUp} | ||
| style={{touchAction: 'none', cursor: 'grab', userSelect: 'none'}} | ||
| aria-label={card.name}> |
There was a problem hiding this comment.
Card renders a plain div with no role, and ARIA prohibits aria-label on the generic role, so screen readers do not reliably read the card's name. role="img" with this label, or the name as VisuallyHidden text inside the card, fixes it.
| let top = 0; | ||
| const card = document.createElement('div'); | ||
| card.textContent = DECK[0]; | ||
| card.setAttribute('aria-label', 'Top card'); |
There was a problem hiding this comment.
Same as the React example: an aria-label on a role-less div is not reliably read. Give the card a role (for example img) or put its name in visually hidden text.
| `family:action-groups` member tracks activates at the lift (FR26), and a toggle | ||
| under `spec:AST-020` FR9's slide-off exception (Switch, Checkbox, | ||
| CheckboxListItem) may also complete on a lift after the finger slid off. | ||
| CheckboxListItem) may also complete on a lift after the finger slid off. Where the engine hit-tests an element a compositor animation moves at the motion's start (iOS WebKit), a touch on moving content goes to what was drawn under the finger at touch-down, and a tap there (no recognizer claimed it, the browser did not take it, and it stayed within 45 pt) MUST activate it: the browser's own click for that touch is cancelled or swallowed, and that element receives one click, an editable focused first. |
Stacked on #8676 (and #8360 below it), one commit each on
main, with no merge commits: review only this pull request's commit, vjeux@28cc0198df.Why
A product that drags, flicks or throws an element has no public way to give it the motion Astryx's own components use: today it copies springs and decay curves by hand and loses Reduce Motion handling.
What led to it
spec:AST-107FR3 opens the touch components' animator as a public handle, with a non-React factory under the hook (FR6).Options and tradeoffs
Why this choice
The spec picks the small handle, so a product's drag stays in step with Core's components. Following the maintainers' API decision,
useAnimator(ref)takes the element's ref, as the gesture hooks do, so a drag and an animation can share one element. The factory, the hook, both docs pages, both examples and their tests ship together: FR5 asks each hook to ship with its docs and a playing-cards example, and FR6 builds the hook on the factory.What changes
createAnimator(element), from the package root only, returnsanimate,decay,set,get,getVelocity,stopanddestroyoverx,y,scale,rotateandopacity.useAnimator(ref), from@astryxdesign/core/hooks, takes the element's ref and returns the animator. After every render it follows the element behind the ref: it attaches a node that mounts, moves to one that replaces it, and lets an unmounted one go, keeping its values for a remount.decaycoasts any channel; opacity stops at 0 or 1. Under Reduce Motion a thrownxorykeeps its momentum; anything else fades on a touch screen or lands at once with a mouse.createMotionAnimator, so no internal factory shares a public factory's name.Left out
animatefollowing the theme'stempoandcharacter(spec:AST-108FR3) arrives with feat(core): the animator's springs follow the theme's tempo and character #8677, which makes the springs read the theme.usePanGesture, a drag and an animation sharing the card's one ref, lands right after feat(core): createPanGesture and usePanGesture report a drag as values (spec:AST-107 FR1, FR6) #8655.How to check it
useAnimatorplaying-cards example, fling the top card: it coasts away, and the next card is dealt once it has faded.Evidence
pnpm test:ui(17,749),check:repo, lint,check:changesets, and the animator andcreateAnimatorinteraction specs ×10 in Chromium pinned to 2 cores (240) and under the CI WebKit job's command (200, with 40 Chromium-only skips); the node suites fail only the twocache.test.mjslock cases that fail on main.useAnimator.test.tsxchecks the ref signature, a replaced node, an unmount and a remount, and a second hook sharing the ref; it failed against the previoususeAnimator().createAnimator.test.tschecks each method against the spring and decay math,destroyand Reduce Motion.Scope