Repository navigation
feat(core): createAnimator and useAnimator animate one element - #8653
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 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).
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🔎 Preview ·
|
| Component | Size | LoC | Visual · A11y · RTL |
|---|---|---|---|
| DateTimeInput | 159.0 kB (+517 B) · 17.0 kB CSS (0) | — | — · — · — |
| lab/MobilePowerSearch | 213.8 kB (+110 B) · 1.2 kB CSS (0) | — | — · — · — |
| lab/ChatTypingIndicator | 24.4 kB (+1 B) · 612 B CSS (0) | — | — · — · — |
27 more components
| Component | Size | LoC | Visual · A11y · RTL |
|---|---|---|---|
| lab/CommentPin | 47.1 kB (+1 B) · 1.2 kB CSS (0) | — | — · — · — |
| charts/Chart | 62.9 kB (0) · 20 B CSS (0) | — | — · — · — |
| charts/ChartTooltip | 52.4 kB (0) · 20 B CSS (0) | — | — · — · — |
| Calendar | 54.3 kB (0) · 10.3 kB CSS (0) | — | — · — · — |
| DateRangeInput | 69.9 kB (0) · 11.8 kB CSS (0) | — | — · — · — |
| Drawer | 36.9 kB (0) · 8.2 kB CSS (0) | — | — · — · — |
| SegmentedControl | 81.7 kB (0) · 7.8 kB CSS (0) | — | ✓ · ✓ · ✓ |
| SegmentedControlItem | 10.3 kB (0) · 4.0 kB CSS (0) | — | ✓ · ✓ · ✓ |
| Slider · Storybook | 88.6 kB (0) · 9.4 kB CSS (0) | 2,326 (0) | ✓ · ✓ · ✓ |
| Switch · Storybook | 86.8 kB (0) · 10.5 kB CSS (0) | 1,719 (0) | ✓ · ✓ · ✓ |
| TimeInput | 90.1 kB (0) · 14.3 kB CSS (0) | — | — · — · — |
| lab/ChatEmojiPicker | 65.1 kB (0) · 773 B CSS (0) | — | — · — · — |
| lab/ChatReactionBar | 65.6 kB (0) · 1.1 kB CSS (0) | — | — · — · — |
| lab/Comments | 232.5 kB (0) · 2.5 kB CSS (0) | — | — · — · — |
| lab/CommentThread | 227.1 kB (0) · 2.4 kB CSS (0) | — | — · — · — |
| lab/DiffViewer | 70.7 kB (0) · 2.1 kB CSS (0) | — | — · — · — |
| lab/EmojiPicker | 70.2 kB (0) · 722 B CSS (0) | — | — · — · — |
| lab/ListInput | 81.5 kB (0) · 1.6 kB CSS (0) | — | — · — · — |
| lab/MobileTokenizer | 101.1 kB (0) · 880 B CSS (0) | — | — · — · — |
| lab/ThreeDChart | 4.9 kB (0) · 20 B CSS (0) | — | — · — · — |
| lab/TransferList | 74.4 kB (0) · 1.7 kB CSS (0) | — | — · — · — |
| lab/TransferListSelector | 80.8 kB (0) · 1.9 kB CSS (0) | — | — · — · — |
| richtext/RichTextEditor | 277.7 kB (0) · 2.6 kB CSS (0) | — | — · — · — |
| richtext/RichTextView | 258.2 kB (0) · 2.3 kB CSS (0) | — | — · — · — |
| lab/CommentComposer | 102.4 kB (−1 B) · 1.9 kB CSS (0) | — | — · — · — |
| PowerSearch | 184.1 kB (−47 B) · 20.6 kB CSS (0) | — | — · — · — |
| DateInput | 103.1 kB (−159 B) · 15.3 kB CSS (0) | — | — · — · — |
Package size:
- @astryxdesign/cli 3.31 MB packed (+3.8 kB); no new dependencies
- @astryxdesign/core 6.10 MB packed (+12.1 kB); no new dependencies
API changes: core +10 · ~0 · −0
10 API changes (advisory)
| Package | Where | Signature | Usage | |
|---|---|---|---|---|
| + | core |
. · Hooks · hook useAnimator |
hook useAnimator: (ref: RefObject<HTMLElement | null>) => AnimatorHandle |
import {useAnimator} from '@astryxdesign/core'; useAnimator(…) |
| + | core |
. · Functions · function createAnimator |
function createAnimator: (element: HTMLElement) => Animator |
import {createAnimator} from '@astryxdesign/core'; createAnimator(…) |
| + | core |
. · Types · interface Animator |
interface Animator extends AnimatorHandle |
import type {Animator} from '@astryxdesign/core' |
| + | core |
. · Types · interface AnimatorAnimateOptions |
interface AnimatorAnimateOptions |
import type {AnimatorAnimateOptions} from '@astryxdesign/core' |
| + | core |
. · Types · type AnimatorChannel |
type AnimatorChannel = 'x' | 'y' | 'scale' | 'rotate' | 'opacity' |
import type {AnimatorChannel} from '@astryxdesign/core' |
| + | core |
. · Types · type AnimatorDecayVelocity |
type AnimatorDecayVelocity = AnimatorValues |
import type {AnimatorDecayVelocity} from '@astryxdesign/core' |
| + | core |
. · Types · interface AnimatorHandle |
interface AnimatorHandle |
import type {AnimatorHandle} from '@astryxdesign/core' |
| + | core |
. · Types · type AnimatorSnapshot |
type AnimatorSnapshot = Readonly<Record<AnimatorChannel, number>> |
import type {AnimatorSnapshot} from '@astryxdesign/core' |
| + | core |
. · Types · type AnimatorValues |
type AnimatorValues = Partial<Record<AnimatorChannel, number>> |
import type {AnimatorValues} from '@astryxdesign/core' |
| + | core |
./hooks · Also exported from an earlier entry point · useAnimator |
useAnimator (.) |
import {useAnimator} from '@astryxdesign/core/hooks' |
Touched components checked; add ci:run-visual, ci:run-a11y or ci:run-rtl to run the full suites.
Generated by PR Enrichment workflow | View full report
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.
6d3d599 to
234dda6
Compare
|
/approve-spec 234dda6 |
There was a problem hiding this comment.
Recommending approval: createAnimator and useAnimator match spec:AST-107 FR3–FR6, and the Slider's touch controller now calls the renamed internal factory, so Core type-checks and the Slider tests pass on this head.
The pull request is over 2,000 lines, but the factory, the hook, their docs and both playing-cards examples ship together as one API under FR5 and FR6, so no split is needed. Springs that follow a theme's tempo and character (spec:AST-108 FR3) remain internal-animator work, as the description says.
[Automated review]
234dda6 to
6921f57
Compare
|
Updated in 6921f57, rebased onto current |
|
/approve-spec 6921f57 |
There was a problem hiding this comment.
Changes requested: the animator, its hook, the examples and the record update look right, but the internal rename breaks a caller that landed on main after this head was pushed.
packages/core/src/Switch/useSwitchTouch.ts(from #9636, now onmain) still importscreateAnimatorfrom../motion/animatorand calls it three times, so with this pull request merged Core's type check fails and every Switch with a touch presentation throwscreateAnimator is not a functionwhen it attaches; please mergemainand rename that import, its three calls and thecreateAnimatorkey in theuseSwitchTouch.test.tsmock tocreateMotionAnimator.
Non-blocking: the comment on REST in createAnimator.ts restates the values; a line saying a new or remounted element starts from them would keep only the reason.
[Automated review]
| * ``` | ||
| */ | ||
| export function createAnimator( | ||
| export function createMotionAnimator( |
There was a problem hiding this comment.
With main merged, packages/core/src/Switch/useSwitchTouch.ts:26 (from #9636) still imports createAnimator from this module and calls it at :288, :291 and :296. Core's type check then fails (TS2305) and a touch-presentation Switch throws createAnimator is not a function when it attaches. Please rename that import, the three calls and the createAnimator key of the vi.mock('../motion/animator') stand-in in useSwitchTouch.test.ts too.
| }; | ||
| } | ||
|
|
||
| /** Every channel at rest: in place, full size, upright and opaque. */ |
There was a problem hiding this comment.
Non-blocking (comment style): this restates the constant's values. Keep only the reason, for example /** A new or remounted element starts from these values. */.
|
Fixed in 271fdeb, merged onto current |
|
/approve-spec 271fdeb |
There was a problem hiding this comment.
Recommending approval: createAnimator and useAnimator match spec:AST-107 FR3–FR6, the DEC-4 record update carries an engineering owner's approval for this head, and the Switch touch controller now calls the renamed internal factory, so Core type-checks and the Switch, Slider and pager tests pass with main merged.
No blocking findings. One optional cleanup:
- The renamed JSDoc
Options of createMotionAnimator.inpackages/core/src/motion/animator.ts(lines 300–302) documents nothing where it sits; move it aboveAnimatorOptionsor delete it.
[Automated review]
|
|
||
| /** | ||
| * Options of `createAnimator`. | ||
| * Options of `createMotionAnimator`. |
There was a problem hiding this comment.
Optional (comment cleanup): this JSDoc documents nothing where it sits. It is directly above SetOptions’ own JSDoc, and AnimatorOptions (line 318) has none. Move it above AnimatorOptions, or delete it.
|
Updated in 4be678e, rebased onto current |
|
/approve-spec 4be678e |
There was a problem hiding this comment.
Recommending approval: createAnimator and useAnimator match spec:AST-107 FR3–FR6, the DEC-4 record update carries an engineering owner's approval for this head, and the SegmentedControl selection motion from main now calls the renamed internal factory, so Core type-checks and the SegmentedControl, Switch, Slider and pager tests pass.
No blocking findings. Two optional cleanups:
- The Keyboard entry of both docs pages and the React example's description say Reduce Motion turns a button's
animateinto a fade in place, but with a mouse it lands at once, as the pages' Reduce Motion entry says; say "a fade in place on a touch screen, a change at once with a mouse". - The renamed JSDoc
Options of createMotionAnimator.inpackages/core/src/motion/animator.ts(lines 300–302) documents nothing where it sits; move it aboveAnimatorOptionsor delete it.
[Automated review]
| 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 have it call `animate`, which Reduce Motion turns into a fade in place. Keep `decay` for a throw the person made: Reduce Motion keeps the momentum of a coast, as it continues that throw. Keep focus on an element that stays on screen: move it before you fling its element away.', |
There was a problem hiding this comment.
Optional (docs accuracy): with a fine primary pointer, Reduce Motion lands a button's animate at once, opacity included (design:reduced-motion DR3), as this page's Reduce Motion entry says. Suggest: "…and have it call animate, which Reduce Motion turns into a fade in place on a touch screen and a change at once with a mouse."
| 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 have it call `animate`, which Reduce Motion turns into a fade in place. Keep `decay` for a throw the person made: Reduce Motion keeps the momentum of a coast, as it continues that throw. Keep focus on an element that stays on screen: move it before you fling its element away.', |
There was a problem hiding this comment.
Optional (docs accuracy): same as on createAnimator's page. With a mouse, Reduce Motion lands this animate at once rather than fading it (design:reduced-motion DR3). Suggest "a fade in place on a touch screen and a change at once with a mouse".
| name: 'useAnimator — Playing cards', | ||
| displayName: 'useAnimator — Playing cards', | ||
| description: | ||
| 'A playing card that follows a drag: a gentle release springs it back on its release velocity, and a quick flick throws it away with momentum before the next card is dealt. A Discard button moves it off from the keyboard, fading it in place under Reduce Motion, and the discard is announced. Built only on the public useAnimator hook.', |
There was a problem hiding this comment.
Optional (docs accuracy): a keyboard user on a desktop has a fine pointer, so under Reduce Motion the Discard press removes the card at once rather than fading it. Suggest "…moves it off from the keyboard, fading it in place under Reduce Motion on a touch screen…" or drop the Reduce Motion clause.
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, through the attach lifecycle the gesture hooks share (useAttachment). The internal factory is renamed createMotionAnimator, along with its callers in the Slider's and the Switch's touch controllers, the SegmentedControl's selection motion and the pager's tests, 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. In both templates the card is an image named by its rank and suit, renamed on each deal; one card leaves at a time (a Discard press while a card leaves deals no second card), a leaving card takes no press, a discard ends a drag in progress, and a press on a card fading in moves only its position; and a drag the browser cancels springs back. The plain-JavaScript example mounts its deck from a callback ref on its wrapper, whose cleanup takes it down. The core API report gains the animator's rows. When the element behind useAnimator's ref unmounts, or a node replaces it, the animator's values return to rest, so a remount or a new node starts from the element's resting values. AST-107 records the owner's decision on four edge behaviors of the public animator and pan gesture (DEC-4): FR1 and FR6 state that a cancelled pan reports zero velocity, how the pan factory's destroy and the hook's unmount report, and that an opacity decay stops at 0 or 1; FR3 states the remount.
|
Updated in 025e68b: in both playing-card examples a discard now ends a drag in progress, so a Discard pressed mid-drag can't be cut short by the finger's next move, and the dealt card no longer follows the old drag. A new mid-drag case fails on the previous head and passes here. Nothing else changed. |
|
/approve-spec 025e68b |
There was a problem hiding this comment.
Recommending approval: createAnimator and useAnimator match spec:AST-107 FR3–FR6, the DEC-4 record update carries an engineering owner's approval for this head, and a Discard pressed mid-drag now ends the drag in both playing-card examples, as the new test shows.
Optional, non-blocking:
- The Keyboard sections of both docs pages, and the React example's description, say Reduce Motion turns a button's
animateinto a fade in place; with a mouse it lands at once, as the same pages' Reduce Motion sections say, so "a fade in place on a touch screen, a change at once with a mouse" would be accurate. - The renamed
Options of createMotionAnimatorJSDoc inpackages/core/src/motion/animator.tssits aboveSetOptions; move it aboveAnimatorOptionsor delete it.
[Automated review]
| 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 have it call `animate`, which Reduce Motion turns into a fade in place. Keep `decay` for a throw the person made: Reduce Motion keeps the momentum of a coast, as it continues that throw. Keep focus on an element that stays on screen: move it before you fling its element away.', |
There was a problem hiding this comment.
Optional: with a fine primary pointer, Reduce Motion lands a button's animate at once rather than fading it, as this page's Reduce Motion entry says. Consider "which Reduce Motion turns into a fade in place on a touch screen, and a change at once with a mouse". The same sentence is in useAnimator.doc.mjs, and the React example's description says it too.
|
|
||
| /** | ||
| * Options of `createAnimator`. | ||
| * Options of `createMotionAnimator`. |
There was a problem hiding this comment.
Optional (comment cleanup): this JSDoc sits directly above SetOptions' own JSDoc, and AnimatorOptions (line 318) has none. Move it above AnimatorOptions, or delete it.
|
/approve-spec 025e68b |
User need
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.
Problem and solution fit
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.Current specification
spec:AST-107FR3 opens the touch components' animator as a public handle, with a non-React factory under the hook (FR6).Public delta and caller burden
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 commit 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; a remount, or a node that replaces it, starts at rest. It does this throughuseAttachment, the attach lifecycle the gesture hooks share, so it has no lifecycle of its own.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. Its callers onmainare renamed with it: the Slider's touch controller (feat(Slider): every pointer drags a thumb by its travel, as UISlider does #8190), the Switch's touch controller and its test's stand-in (feat(Switch): the touch presentation, for every pointer #9636), the SegmentedControl's selection motion and its test (feat(SegmentedControl): the selection slides on every change and drags with any pointer #9632), and the pager's tests (chore(core): a pager that pages like a UIKit paging scroll view #8146).role="img") named by its rank and suit, renamed on each deal, and one card leaves at a time: a Discard press while a card leaves, by a flick or by the button, deals no second card, and a press on the leaving card does nothing. A press on a card that is fading in moves only its position, so the fade runs on. A discard ends a drag in progress, so the finger's later moves leave the leaving card and the dealt card alone. A drag that the browser cancels springs back instead of being thrown.useAnimator's ref unmounts, or a node replaces it, the animator's values return to rest, so a remount or a new node starts from the element's resting values.spec:AST-107records the owner's decision on four edge behaviors of the public animator and pan gesture as DEC-4. FR1: a cancelled pan reportsvelocity{0, 0}. FR6:createPanGesture'sdestroy()during a drag reportscancelledonce,usePanGesture's unmount reports nothing, and an opacitydecaystops at 0 or 1, whichcreateAnimatoralready does. FR3: a remount starts from the element's resting values. The pan sentences describe the public pan that comes next in this stack, which already behaves this way; they sit here so the record changes in one place.function createAnimator,hook useAnimator, and theAnimator,AnimatorAnimateOptions,AnimatorChannel,AnimatorDecayVelocity,AnimatorHandle,AnimatorSnapshotandAnimatorValuestypes.spec:AST-107FR3 decides the handle and its channels, and FR6 the factory under the hook. Regenerating the report on this head also rewrites rows thatmainchanged without updating it (text input props, some style hashes); those are left out, so this pull request adds only its own rows.Left out.
decaysets no bound onscale, as the record sets none, so a fast shrink can coast below 0. Stopping it at 0, as opacity stops at its edges, is a behavior change for the owners to rule on.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.Effect on other open pull requests. The internal factory's rename to
createMotionAnimatorbreaks six open drafts that import it ascreateAnimatorfrom the motion module: #8065, #8145, #8189, #8496, #8677 and #8686. Each one renames that import when it rebases; the factory's signature is unchanged. A temporary internal alias is not possible, since no internal factory may share the public factory's name.Size.
import {createAnimator} from '@astryxdesign/core'adds 37.9 KB minified, 14.0 KB gzip, andimport {useAnimator} from '@astryxdesign/core/hooks'adds 38.4 KB minified, 14.2 KB gzip, measured with the size report's build options. Neither import reaches any gesture module, which the touch-support bundle test checks.Pull request size. This pull request's own commit is 2,542 lines with tests, and no part of it can stack apart.
spec:AST-107FR5 reads "Each hook ships with docs and a playing-cards example", so the two playing-cards templates, their test and their registrations (709 lines) ship with the hooks. The rest is the two exports with their docs, changeset and API report rows (716 lines), and the tests that cover them (1,117 lines), which cannot land apart from the code they test.Design and alternatives
Evidence
x, which took the discard's motion over, dealt the next card early and dragged it with the old drag.stopnow ends running fades, as the animator's does.role="img"and its name, which fails on the previous examples.useAnimator's module test checks that the hook imports only the factory,useAttachmentand React, and callsuseAttachment(ref, attach, null).maince667be:pnpm install --frozen-lockfile, build, type check,test:ui(19,041), the node suites (16,057, with 2 expected failures),check:repo,ASTRYX_STRICT_LINT=1 pnpm lint(0 errors),check:changesets,check:knowledgeandpnpm review:prepass. On a fresh Storybook build, the accessibility contracts of Switch and Slider pass (42 of 42). The public and internal animators' browser specs pass ×3 in Chromium pinned to 2 cores at 1 worker (72 of 72), the Switch and SegmentedControl interaction specs pass there (104 of 104), and under the CI WebKit job's command all of them pass (110 passed; 18 skip by their own engine conditions).Writing rubric results
spec:AST-107, with the sentences added to FR1, FR3 and FR6 and the new DEC-4:R1 narration: none; the added sentences and DEC-4 state what the system does, in the present tense. DEC-4's decider and date sit in its Decider line, as in DEC-1 to DEC-3.
R2 research: none in the record.
R3 evidence: no verification cell names a pull request, issue or commit.
R4 rejected alternatives: DEC-4 rejects four, in one
Rejected:line inside the decision; the other three decisions keep theirs.R5 change references: none.
R6 open questions: the section reads "None", and nothing the added text answers was open there.
Scope
Testing
useAnimatorplaying-cards example, fling the top card: it coasts away, and the next card is dealt once it has faded.