Repository navigation
architecture(react-component-runtime): the animator animates a number (INV15) - #8603
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
AI review status for this pull request.
|
github-actions
Bot
added
the
needs:spec-owner-review
PR Analysis Report
|
meta-cla
Bot
added
the
CLA Signed
a0fd3e6 to
ef1e4d5
Compare
cixzhang
changed the title
|
/approve-spec ef1e4d5 |
github-actions
Bot
removed
the
needs:spec-owner-review
![astracat-bot[bot]](https://avatars.githubusercontent.com/in/4796231?s=60&v=4){kind=small}
There was a problem hiding this comment.
Requesting changes: INV15’s direction is approved for this head, but its Reduce Motion clause conflicts with the current reduced-motion design record, and two smaller points need fixing. Please address the points below here; a new commit needs a fresh /approve-spec for its head.
- Reduce Motion: “Under Reduce Motion it swaps at once” skips the opacity-only cross-fade of 200 ms or less that
design:reduced-motionDR2 requires under a coarse primary pointer, so restate it as DR1–DR3 (asspec:AST-105FR4 does) with both pointers in the verification row, or amend DR2 in this pull request. - Adopters:
component:BadgePR1 and AR1 rule out the timer and live region a Badge adoption needs, so make “the component’s own live region announces the final value” conditional and note that Badge’s record changes first;@astryxdesign/chartscannot reach Core’s internal subpath, so the chart adopter is Lab’s. - Routing: add INV15 to the record’s
review-applicabilitytriggers (beside INV13 underreact-runtime), sincearchitecture:knowledge-contractsINV20 brings a global claim into a component review only through that map.
[Automated review]
| a jump from twelve to over a million does not spin. Under Reduce Motion it | ||
| swaps at once. It announces nothing: the component's own live region |
There was a problem hiding this comment.
Under a coarse primary pointer, design:reduced-motion DR2 requires an opacity-only cross-fade of 200 ms or less, and its verification fails “a change with no cross-fade”; the animator already does this for its animate calls. Suggested wording, as spec:AST-105 FR4 states it: “Under Reduce Motion it does not count: the new value cross-fades in, opacity only and within 200 ms, under a coarse primary pointer, and appears at once otherwise (design:reduced-motion DR1–DR3).” If counts should snap on touch too, DR2 needs amending in this pull request.
| the change size past which it swaps to the new value instead of counting, so | ||
| a jump from twelve to over a million does not spin. Under Reduce Motion it | ||
| swaps at once. It announces nothing: the component's own live region | ||
| announces the final value, never the values between. Number formatting stays |
There was a problem hiding this comment.
component:Badge keeps no live region (AR1: “a changed label is not announced”) and registers no timer or effect (PR1), yet the description names Badge as a first adopter. Suggest making this conditional: “where the component’s own record announces the value, only the final value is announced, never the values between.”
| | INV11 | Source boundary check plus hook tests across StrictMode replay and unmount | A second keyboard listener or measurement in Core; a consumer reading a CSS custom property on the root or a shared ancestor for the height; an export of the hook; a component outside Core measuring by a rule other than AST-099 FR2, or riding the keyboard; a listener left after unmount; a Core rider driven by JavaScript per frame or by its own transition; or more than one layout read per keyboard change | | ||
| | INV12 | Source boundary check plus real-browser tests on a throttled phone profile over a 10-level by 50-sibling tree and two changes in one frame | A per-frame height or layout, a walk past the area or into a subtree other than `display: contents`, an element moved twice for one frame, an added element, or an exported reporting function fails | | ||
| | INV13 | Source boundary check plus bundle analysis of a page with several touch components, and real-browser tests on a throttled phone profile, under a fine pointer, and on a hybrid device | A second copy of a touch module on one page, a component bundling its own copy of a module's primitive, an `import()` of a module in a component, a public export or doc entry for the modules, an install call or installed check in the public API, a touch listener under a fine primary pointer, or an import of `@astryxdesign/core/internal/*` outside Core and Lab fails | | ||
| | INV15 | Unit tests of the animator's value mode and real-browser proofs of its adopters | A value off its motion role, a count past the component's swap size, motion under Reduce Motion, an intermediate value announced, or a public export fails | |
There was a problem hiding this comment.
With the Reduce Motion clause restated, this failure signal would read “a count under Reduce Motion, or a change with no cross-fade under a coarse primary pointer”; as written, “motion under Reduce Motion” would also fail the DR2 cross-fade. INV15 also needs a trigger in the review-applicability block at the top of the record (beside INV13 under react-runtime) for adopters’ reviews to load it.
…tes the value at once (DR2); live region conditional; routing
|
/approve-spec 1ae8907 |
There was a problem hiding this comment.
Requesting changes: INV15’s direction is owner-approved for this head, but its Reduce Motion clause cites design:reduced-motion DR2 for a value written at once on any pointer, while DR2 on main still requires an opacity-only cross-fade of 200 ms or less under a coarse primary pointer and fails “a change with no cross-fade”. That in-place value exception exists only in the open #8645, so land #8645 first, carry its DR2 amendment here, or restate INV15 and its verification row as DR1–DR3 (a cross-fade on a coarse pointer, at once otherwise). A new commit here needs a fresh /approve-spec for its head.
[Automated review]
| a jump from twelve to over a million does not spin. Under reduced motion the | ||
| new value is written at once on any pointer, as an in-place value change | ||
| (`design:reduced-motion` DR2). It |
There was a problem hiding this comment.
DR2 on main has no in-place value exception: under a coarse primary pointer it requires that “each DR1 transition MUST change opacity only and MUST finish in 200 ms or less”, and its verification fails “a change with no cross-fade”. The exception this cites exists only in #8645. Land that first, or restate as DR1–DR3: “Under reduced motion it does not count: under a coarse primary pointer the new value cross-fades in, opacity only and within 200 ms, and with any other pointer it is written at once.”
| | INV11 | Source boundary check plus hook tests across StrictMode replay and unmount | A second keyboard listener or measurement in Core; a consumer reading a CSS custom property on the root or a shared ancestor for the height; an export of the hook; a component outside Core measuring by a rule other than AST-099 FR2, or riding the keyboard; a listener left after unmount; a Core rider driven by JavaScript per frame or by its own transition; or more than one layout read per keyboard change | | ||
| | INV12 | Source boundary check plus real-browser tests on a throttled phone profile over a 10-level by 50-sibling tree and two changes in one frame | A per-frame height or layout, a walk past the area or into a subtree other than `display: contents`, an element moved twice for one frame, an added element, or an exported reporting function fails | | ||
| | INV13 | Source boundary check plus bundle analysis of a page with several touch components, and real-browser tests on a throttled phone profile, under a fine pointer, and on a hybrid device | A second copy of a touch module on one page, a component bundling its own copy of a module's primitive, an `import()` of a module in a component, a public export or doc entry for the modules, an install call or installed check in the public API, a touch listener under a fine primary pointer, or an import of `@astryxdesign/core/internal/*` outside Core and Lab fails | | ||
| | INV15 | Unit tests of the animator's value mode and real-browser proofs of its adopters | A value off its motion role, a count past the component's swap size, a count or cross-fade under reduced motion with a coarse or a fine pointer, an intermediate value announced, or a public export fails | |
There was a problem hiding this comment.
Against the current DR2, failing “a … cross-fade under reduced motion with a coarse … pointer” fails exactly what DR2 requires. Restated as DR1–DR3, this would read “a count under reduced motion, or a change with no cross-fade under a coarse primary pointer”.
|
/approve-spec 1ae8907 |
… component's own record; merge main (DR2 value exception)
|
/approve-spec 61c65cd |
There was a problem hiding this comment.
Recommending approval: the animator's value mode (INV15) is owner-approved for this head, and its reduced-motion, announcement and internal-only clauses match the current reduced-motion, status-announcement and release-compatibility specs. The reduced-motion conflict flagged on earlier heads is resolved now that DR2's in-place value exception is on main. For the adopting pull requests: Badge's own record (PR1, AR1) rules out the timer or effect a count needs and keeps no live region, so a Badge adoption starts with that record, and only Lab's Chart, not the separate charts package, can reach Core's internal animator.
[Automated review]
Decided (Cindy, 2026-10-26: D34, resolved by convergence: "resolve these with the gesture modules… can they converge?").
Summary
This amends an existing internal module, with no new module and no public API: the animator gains a value mode. It plays a number on the same springs and motion roles and hands it to
onUpdateeach frame; the component owns the swap threshold, Reduce Motion swaps at once, and it announces nothing in between.spec:AST-017FR21).The record adds INV15 and its verification row.
Writing rubric results