Repository navigation
spec(AST-105): animated layout; architecture(react-component-runtime): its mechanism (INV12) - #8463
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
AI review status for this pull request.
|
PR Analysis Report
No new or modified components detected. Bundle Size SummaryNo component packages changed. Accessibility AuditStatus: No accessibility violations detected. Visual Regression55 of 502 shot(s) changed. A change here is a question, not a failure: check whether the after is the
and 35 more. Generated by PR Enrichment workflow | View full report |
There was a problem hiding this comment.
This proposal waits on an engineering owner's decision on INV12: whether to adopt handle (c) from OQ1, and whether the travel applies on every pointer or only on a coarse primary pointer. An engineering owner records that decision by commenting /approve-spec <full head SHA> on this pull request, and the author may do so if they are an engineering owner. Because a new commit invalidates approval, please fix these in the record first so the approved head is final:
- The Reduce Motion sentence cross-fades on every pointer, but
design:reduced-motionDR3 requires an immediate change when the primary pointer is not coarse; defer to DR1–DR3 or scope INV12 to touch. ScrollAreais not a Core component; the scroll container isScrollableArea.architecture:public-component-apiINV21 is not onmainyet, so cite onlyspec:AST-002admission or land that change first.- Remove the "Proposed; pending owner decision" and "(proposed)" markers and collapse OQ1 into INV12 in the head that gets approved.
[Automated review]
| its own elements, and follows INV3 and INV5. The hook follows INV3 and | ||
| INV5. | ||
| - **INV12 — Disclosure height changes have one module and one handle.** | ||
| **Proposed; pending owner decision (OQ1).** In `@astryxdesign/core`, one |
There was a problem hiding this comment.
This marker, the INV12 (proposed) verification row and OQ1's options should come out in the head an owner approves; otherwise a current record merges a claim that calls itself pending.
| change and moves following content with a compositor-only translate from | ||
| its old place to its new one (`spec:AST-094` FR5); it never animates height, | ||
| which lays out every frame. What moves is designated, never found by walking | ||
| siblings: a scroll-area container, such as ScrollArea, is the boundary that |
There was a problem hiding this comment.
Core has no ScrollArea; its scroll container is ScrollableArea (component:ScrollableArea). The same name appears in the example at lines 186, 201, 202 and 205.
| around it. Under Reduce Motion nothing travels, and following content | ||
| cross-fades into place (`design:reduced-motion` DR2). The module and its hook |
There was a problem hiding this comment.
design:reduced-motion DR2 applies only on a coarse primary pointer or a touch release; DR3 requires an immediate change, with no animated property, otherwise. As written this cross-fades on every pointer. Suggest deferring to DR1–DR3 here and adding "an animated property under reduced motion with a fine pointer" to the verification row's failure signal.
| cross-fades into place (`design:reduced-motion` DR2). The module and its hook | ||
| are not exported; a scroll container an app owns is not a boundary, so | ||
| content in it keeps today's flow, and making a boundary public needs | ||
| `spec:AST-002` admission and `architecture:public-component-api` INV21. |
There was a problem hiding this comment.
architecture:public-component-api INV21 is not on main (that record ends at INV17); it exists only in an open proposal. Cite only spec:AST-002 admission here, or land INV21 first.
| // A disclosing element measures before its commit and animates after it. | ||
| const change = useDisclosureChange(groupRef); | ||
| const toggle = () => { | ||
| change.measureBefore(); |
There was a problem hiding this comment.
This takes the before-measurement only in the disclosure's own toggle. A CollapsibleGroup in its default single mode closes another item whose toggle never runs, and a controlled isOpen changes without this handler; a boundary-level snapshot would cover both.
|
Posted by Cindy's agent on her behalf. Cindy's decision on how following content is designated (OQ1): (c), with a generic boundary.
Since apps can provide a ref, the record should state the minimal public shape for that integration point and check it against the API principles. |
There was a problem hiding this comment.
This proposal waits on an engineering owner's decision on INV12: whether to adopt it with OQ2 settled as (ii), which makes useDisclosureParticipant public beside useDisclosureBoundary, and whether the travel applies on every pointer or only on a coarse primary pointer. An engineering owner records that decision by commenting /approve-spec <full head SHA> on this pull request, and the author may do so if they are an engineering owner. Because a new commit invalidates approval, please fix these in the record first so the approved head is final:
- The Reduce Motion sentence cross-fades on every pointer, but
design:reduced-motionDR3 requires an immediate change when the primary pointer is not coarse; defer to DR1–DR3 or scope INV12 to touch. ScrollAreais not a Core component; the scroll container isScrollableArea.architecture:public-component-apiINV21 is not onmainyet, so cite only current claims or land that change first.- Remove the "Proposed; pending owner approval", "INV12 proposes (ii)" and "is decided by … 2026-10-25" wording and the "(proposed)" row label, and fold the ruled OQ2 into INV12.
[Automated review]
| as one participant of the boundary around it. Under Reduce Motion nothing | ||
| travels, and following content cross-fades into place | ||
| (`design:reduced-motion` DR2). The handle (a container boundary with opt-in |
There was a problem hiding this comment.
design:reduced-motion DR2 cross-fades only on a coarse primary pointer or a touch release; DR3 requires an immediate change, with no animated property, otherwise. As written this cross-fades on every pointer. Deferring to DR1–DR3 here, and adding "any animated property under reduced motion with a fine pointer" to the INV12 row's failure signal, would fix it; so would scoping INV12 to a coarse primary pointer.
| translate from its old place to its new one (`spec:AST-094` FR5); it never | ||
| animates height, which lays out every frame. What moves is designated, never | ||
| found by walking siblings. A boundary is any container element registered by | ||
| ref: ScrollArea registers its scrolling element itself, and an app registers |
There was a problem hiding this comment.
Core has no ScrollArea; its scroll container is ScrollableArea (component:ScrollableArea). The usage comment at line 194 repeats the name.
| (ii), `useDisclosureParticipant(elementRef)`: each takes a ref and returns | ||
| nothing, because which container scopes the motion and which app content | ||
| moves with it are the caller's intent (`spec:AST-002` FR1), and no other API | ||
| does either job (`architecture:public-component-api` INV21). The change hook |
There was a problem hiding this comment.
architecture:public-component-api INV21 is not on main (that record ends at INV17); it exists only in an open proposal. Cite only current claims here, such as spec:AST-002 FR1 and FR5, or land INV21 first.
| its own elements, and follows INV3 and INV5. The hook follows INV3 and | ||
| INV5. | ||
| - **INV12 — Disclosure height changes have one module and one handle.** | ||
| **Proposed; pending owner approval of the public hook.** In |
There was a problem hiding this comment.
This marker, "INV12 proposes (ii)" (line 172), "is decided by cixzhang, 2026-10-25" (lines 182–183) and the INV12 (proposed) row label (line 358) should come out of the head an owner approves, with the ruled OQ2 folded into INV12; otherwise a current record merges proposal and decision history (architecture:knowledge-contracts INV10).
There was a problem hiding this comment.
This proposal waits on an engineering owner's decision on INV12: whether to adopt it with OQ2 settled as (ii), which makes useDisclosureParticipant public beside useDisclosureBoundary, and whether the travel applies on every pointer or only on a coarse primary pointer (OQ3). An engineering owner records that decision by commenting /approve-spec <full head SHA> on this pull request, and the author may do so if they are an engineering owner. Because a new commit invalidates approval, please make these changes first so the approved head is final:
- Remove "Proposed; pending owner approval", "INV12 proposes (ii)", "is decided by … 2026-10-25" and the "(proposed)" row label, and fold the OQ2 and OQ3 rulings into INV12 (
architecture:knowledge-contractsINV10); the description's "R1 narration: none" does not match the record. - Add a fine-pointer Reduce Motion case to the INV12 verification row, since INV12 now requires an immediate change there (
design:reduced-motionDR3). - State the
spec:AST-002FR5 admission argument for each hook: the two otherwise identical cases and why Core cannot derive the outcome. - For OQ2, note that under (ii) unregistered content after the participants takes its final place at once, so it overlaps them while they slide back on a collapse.
[Automated review]
| | INV9 | Provider/external-store identity and render-count tests | Unchanged semantic payload changes identity, or snapshot identity creates a render loop | | ||
| | INV10 | Shared test over every adopter of the announcement module | An adopter holding its own Effect, observer, timer, or listener for its own-content announcement | | ||
| | 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 (proposed) | Real-browser tests on a throttled phone profile plus a source boundary check, over a disclosure inside a boundary with participants, non-participants, a sticky header, a nested boundary, content outside, and Reduce Motion | A per-frame height or layout, a sibling walk, a moved non-participant, an added element, or travel under Reduce Motion fails | |
There was a problem hiding this comment.
This row tests only a throttled phone profile and fails only on "travel under Reduce Motion". INV12 now also requires an immediate change with a fine pointer (design:reduced-motion DR3: "MUST complete immediately, with no animated property"), and OQ3 recommends travel under every pointer, so a cross-fade or a travel with a mouse would pass. Suggest adding a fine primary pointer state and "any animated property under Reduce Motion with a fine pointer" to the failure signal, plus a fine-pointer travel case if OQ3 lands on (a).
| The public pieces are `useDisclosureBoundary(containerRef)` and, under OQ2's | ||
| (ii), `useDisclosureParticipant(elementRef)`: each takes a ref and returns | ||
| nothing, because which container scopes the motion and which app content | ||
| moves with it are the caller's intent (`spec:AST-002` FR1), and no other API | ||
| does either job. The change hook |
There was a problem hiding this comment.
spec:AST-002 FR5 asks a public-API proposal to name "the two otherwise identical cases, the different required outcomes, why the caller knows the difference, and why the component cannot derive it". This gives caller intent under FR1 (written for props) and "no other API does either job", but not the two cases or why Core cannot derive the outcome, and OQ2's own option (i) derives what moves from layout (FR2). Stating that argument for each hook would let the owner rule OQ2 on it.
|
Posted by Cindy's agent on her behalf. Cindy's decision on mixed following content (OQ2): (i), a sibling walk bounded by the container ref.
Still open: whether the glide applies under every pointer or only under touch. |
b651643 to
f1747db
Compare
|
Posted by Cindy's agent on her behalf. Cindy's decision on the name (OQ4):
The internal trigger is |
|
Posted by Cindy's agent on her behalf. Cindy's decision on pointer scope (OQ3): every pointer, mouse included. "Yes, a layout fix." This isn't a touch gesture: mouse users saw the same jump. It's recorded as a named desktop change under The changed element animates itself, and the content after it follows with matching timing and easing. |
|
Posted by Cindy's agent on her behalf. Cindy's decision on which elements move (OQ5): only elements whose position actually changed and that are visible in the area. The walk collects candidates; offscreen ones take their final place without animating. It descends into With this, every open question on
|
There was a problem hiding this comment.
This record change waits on an engineering owner's decision on INV12, including what happens when a changed element has no registered area above it, such as a Collapsible on a page that scrolls the document; an engineering owner records that decision by commenting /approve-spec <full head SHA> on this pull request, and the author may do so if they are an engineering owner. Because a new commit invalidates approval, please make these record fixes first so the approved head is final:
- Lines 190–192 cite a layout-change role from
spec:AST-094FR4, which names no such role and covers touch only; add the role through its owner first or cite the roles and tokens that exist. - Remove "are decided by
cixzhang, 2026-10-25" and "TreeList and Collapsible adopt the module first" (lines 206–210), perarchitecture:knowledge-contractsINV10. - Make the hook documentation at line 217 say "Fixed and stuck sticky elements stay put" and "motion role", matching the coverage table and the authoring guidance.
- Give rows 422–425 a fine-pointer Reduce Motion case and a theme-override case and scope the jump signal to visible followers; also carve
display: contentsout of line 182, state thespec:AST-002FR5 argument foruseAnimatedLayout, and add INV12 to the review-applicability block.
[Automated review]
| An area is any container element registered by ref: ScrollableArea | ||
| registers its scrolling element itself, and an app registers its own scroll | ||
| container or panel with `useAnimatedLayout(containerRef)`. An element whose | ||
| size changes reports the change, around its commit, to its nearest |
There was a problem hiding this comment.
What happens when the changed element has no registered area above it, such as a Collapsible on a page that scrolls the document outside any ScrollableArea? Lines 189–190 have the element play its own resize unconditionally, so if its followers then take their final place at once they overlap it on a collapse and leave a gap under it on an expand. The record should state the no-area behavior: the document as the area, or the element taking its final size at once.
| ancestor to the area: at every level, each following in-flow sibling moves | ||
| by the distance layout moved it, measured before and after. The walk visits | ||
| only the following element siblings at each ancestor level, through | ||
| `parentElement` and `nextElementSibling`, never descending into a subtree, |
There was a problem hiding this comment.
"never descending into a subtree" conflicts with the coverage row at line 249, which takes a display: contents element's children in its place. Carving that case out here keeps both sentences true.
| area's visible part, so the setup cost is bounded by what the person can | ||
| see. Which elements move is listed under Size-change coverage below. The changed element plays its own animation, entering, exiting, or | ||
| resizing, and the module moves only the content after it. Both read the | ||
| same generic theme motion role, the layout-change role of the motion scheme |
There was a problem hiding this comment.
spec:AST-094 FR4 lists no layout-change role (its table names only springs.* and timing.* roles), FR4 covers touch presentations only, and AST-094's Non-goals keep pointer timing on the theme's duration and easing tokens. Add the role through its owner first, with a measured default and source, or cite the role and token that time each pointer; the INV12 timing row (line 424) depends on it.
| a named fine-pointer change under `spec:AST-017` FR59, and its adopting pull | ||
| requests show fine-pointer evidence before and after. The container area, | ||
| the bounded walk, the changed-and-visible rule, every-pointer scope, the | ||
| shared motion role, and the name `useAnimatedLayout` are decided by |
There was a problem hiding this comment.
This attribution, with "TreeList and Collapsible adopt the module first" on lines 209–210, is decision and project history in a current record (architecture:knowledge-contracts INV10: "a record states the fact a change established, not the change"; rubric R1). Keeping only "any Core component whose size changes in place uses it rather than its own layout animation" states the durable rule.
| changes in place uses it rather than its own layout animation. | ||
| `useAnimatedLayout` is the one public piece: it takes the container's ref | ||
| and returns nothing, because which container scopes the motion is the | ||
| caller's intent (`spec:AST-002` FR1), and no other API does that job. |
There was a problem hiding this comment.
spec:AST-002 FR5 asks a public-API proposal to name "the two otherwise identical cases, the different required outcomes, why the caller knows the difference, and why the component cannot derive it", and FR2 keeps a difference internal when Core can derive it from measured or CSS layout, such as the nearest scroll container. Stating that argument here lets the owner admit useAnimatedLayout on it.
| caller's intent (`spec:AST-002` FR1), and no other API does that job. | ||
| `reportLayoutChange` stays internal. Its documentation reads: "When | ||
| something inside this area changes size, the changed element and the content | ||
| after it animate together on one shared motion token. Fixed and sticky elements stay put, Reduce Motion is |
There was a problem hiding this comment.
"Fixed and sticky elements stay put" contradicts the coverage row at line 251 (an unstuck sticky element moves) and the guidance at line 279 ("stuck sticky elements stay put"); this text also says "token" where INV12 says role, and omits the nesting and scroll-position points the guidance at lines 277–282 says the documentation states.
| | INV9 | Provider/external-store identity and render-count tests | Unchanged semantic payload changes identity, or snapshot identity creates a render loop | | ||
| | INV10 | Shared test over every adopter of the announcement module | An adopter holding its own Effect, observer, timer, or listener for its own-content announcement | | ||
| | 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 | Real-browser tests on a throttled phone profile plus a source boundary check, over a disclosure and an inserted item nested several levels inside a parent with following siblings at each level, mixed Astryx and plain elements, a fixed element, an unstuck sticky header, a nested boundary, content outside the boundary, two disclosures toggled in one frame, a 10-level by 50-sibling tree for cost, and Reduce Motion | A per-frame height or layout, a walk past the boundary, a moved out-of-flow element, a following sibling at any level that jumps, an element moved twice for one frame, an added element, or travel under Reduce Motion fails | |
There was a problem hiding this comment.
This row tests only a throttled phone profile and fails only on "travel under Reduce Motion", so a cross-fade with a mouse (design:reduced-motion DR3: "MUST complete immediately, with no animated property") or a theme override of the role that restores the glide (DR4) passes every INV12 row. "A following sibling at any level that jumps" also fails the offscreen and inline followers that lines 186–189 and 264–265 send to their final place at once; scoping it to visible, in-coverage followers fixes that.
|
Posted by Cindy's agent on her behalf. Cindy's decision on the API shape: const animatedLayout = useAnimatedLayout({containerRef});
<TreeList animatedLayout={animatedLayout}>…</TreeList>
|
…and where the logic lives; hook name is OQ4
…s; move by measured change (OQ5)
…s; move by measured change (OQ5)
…ements in the visible area
…-of-coverage list
…x order candidates, and scaled ancestors
…; additive retarget; scale correction; plain out-of-coverage list
…documentation points for the hook
…(decided), with its documentation; restore INV12's batching, INV7, Reduce Motion, and adoption text
…tself; followers match its timing via reportLayoutChange(ref, motion)
…s an AST-017 FR59 named fine-pointer change
…g; reportLayoutChange returns the motion the element plays
… element and its followers; reportLayoutChange only triggers
…f}) returns a handle passed as animatedLayout; ScrollableArea gets the ref; CodeBunny consistency
…ll position kept; no unmerged citations; OQ6 motion role, OQ7 ownership
…e names a layout role; this record owns the hook; scroll position is tested
… the module never scrolls; scope to reported changes; drop the TreeWalker note
…m spec; INV12 keeps the mechanism
…s; runtime lists AST-105/DEC-1
a96e822 to
0c0c8f4
Compare
|
/approve-spec 0c0c8f4 |
There was a problem hiding this comment.
Both records carry an engineering owner's approval of this revision, and the four record fixes from the last review are in, so this looks ready to land. When TreeList, Collapsible or another component adopts animatedLayout, that pull request owes the fine-pointer before-and-after evidence FR5 names. If you touch AST-105 again, ending FR4's Reduce Motion sentence at the design:reduced-motion DR1–DR3 reference and adding a fine-pointer Reduce Motion failure signal to its verification row would tighten it.
[Automated review]
Decided (Cindy, 2026-10-25): OQ1–OQ5 all resolved.
Exact decision needed
How does Core designate the content that moves when a disclosure changes height (OQ1)? Strategy is fixed by Cindy (16:12): measure before and after, translate on the compositor.
Existing authority searched
architecture:react-component-runtimeINV10 and INV11 (one internal module per cross-component behavior), INV3 and INV5.spec:AST-094FR5 (compositor-only motion),design:reduced-motionDR2 (cross-fade),architecture:component-style-authoringINV7 (no added elements).architecture:public-component-apiINV18–INV21 (architecture(public-component-api): four API-shape principles (INV18–INV21) #8459, approved): INV21 for any public boundary.architecture:layer-runtimeis for layers, so it is not the home; this is a runtime module like INV10/INV11.Decision
Split per knowledge-contracts INV18 (chief OK, 2026-10-25): the public contract (hook,
animatedLayoutprop, glide behavior, coverage table) is a new system spec,spec:AST-105;architecture:react-component-runtimeINV12 keeps only the mechanism (measure, walk, translate, internal reporting function), as INV10/INV11 project AST-086/AST-099. The text is unchanged apart from the move.architecture:react-component-runtimeINV12:useAnimatedLayout(containerRef). A disclosure reports to its nearest registered ancestor boundary, which moves only participants after it.Reframe: any size change (Cindy 19:14)
INV12 is a general size-change (layout-shift) module: disclosures, async content loading in, messages appearing, items inserted or removed, text expanding. Names are generic: public
useAnimatedLayout(containerRef: RefObject<HTMLElement>): void(ScrollableArea registers itself); internalreportLayoutChange(ref)(not exported).hasAnimatedChanges, feat(List): animate row inserts, removals and moves through the animatedLayout handle #8106), Textarea autosize, Skeleton → loaded content.hasAnimatedChangesFLIP (feat(List): animate row inserts, removals and moves through the animatedLayout handle #8106, not on main) shares the job and should adopt this module instead of its own.motion/rectMorph.tsis the morph INV15 names; the module reuses it.parentElementandnextElementSibling, visiting only following element siblings at each ancestor level, O(depth + following siblings), never descending into subtrees. A TreeWalker visits subtrees and text nodes unless filtered, and text nodes don't move. Lane 1 measured the manual walk 3–5× cheaper than a TreeWalker.API shape (Cindy 20:18)
useAnimatedLayout({containerRef})returns an opaque handle (no public methods or fields); size-changing components take ananimatedLayoutprop and report through it. Explicit composition replaces implicit registration; reporting is an internal method on the handle.architecture:theme-tokensINV3 (the layout role name waits for the motion scheme); decision and rollout notes removed per knowledge-contracts INV10; JSDoc says "stuck sticky" and "motion role"; fine-pointer Reduce Motion and visible-follower checks;display: contentscarve-out; INV12 in the review-applicability block.What the hook does (Cindy 19:15); name decided 19:28:
useAnimatedLayoutAll the logic (measure, walk, translate) lives inside one internal module. The public hook only marks the area where the walk stops; the internal
reportLayoutChange(ref)only triggers it. Name (OQ4), each for what it does:useSmoothLayoutShift(ref)useGlideOnResize(ref)(runner-up)useAnimatedLayout(ref)(DECIDED, Cindy 19:28: "pick the one that has the least specialized words")useAnimatedReflow(ref)useDisclosureAnimatedShift(ref)Authoring guidance now lists the points every agent was unsure about: only content after the change moves; fixed and stuck sticky stay put; Reduce Motion handled; List rows use the same module; nesting stops at the nearest area; scroll position kept.
Timing and pointer scope (decided)
reportLayoutChange(ref)only triggers the walk and lines up the start frame; Reduce Motion reaches both through the role.Element selection (Cindy 19:16)
The walk collects candidates; recommended (OQ5 a): only candidates whose layout rectangle changed between the two measurements move.
display: contentsdisplay: none, hidden, zero sizeorder, wrap, floatsMixed following content (OQ2, decided: (i))
Cindy (19:11): "I think a sibling walk on the container ref would be how we do it. It needs to account for the disclosure existing within a parent that has siblings in the container, and arbitrary nesting between the disclosure and the container."
useAnimatedLayout(containerRef: RefObject<HTMLElement>): void; ScrollableArea registers itself.useDisclosureParticipantis removed.Walk cost (lane 1, final; 4× CPU Chromium, mobile viewport, software GPU):
Fixture findings folded into the coverage table:
display: contentschildren need explicit handling; inline elements cannot take a transform (they take their final place, their block moves); flex items placed later byordermust be candidates; a scaled ancestor needs a scale correction; elements mid-animation retarget (additive composite in the implementation).Cost rule (in INV12): animate only elements whose rectangle changed and that intersect the boundary's visible area; offscreen elements take their final place at once. Setup is about 0.05 ms per animated element, so the cost is bounded by what is on screen. No per-level wrappers (rejected 19:09).
Correctness fixtures (pure sibling rule vs move-only-changed) are next from lane 1.
Perf
Headless Chromium on a server CPU; a disclosure with 400 following rows of wrapped text; forced layout timed over 60 frames, median of 15 runs (three runs agree):
On a 4–6× slower phone that is roughly 0.1–0.2 ms per frame either way, so main-thread layout is not where the approaches differ. These numbers do not capture paint and raster: a height animation repaints the moved region every frame, while a translate is composited (AST-094 FR5). That is the reason to measure once and translate.
A demo view exists for owner review.
First adopters
TreeList (groups expand inside its scroll area) and Collapsible.
Rejected alternatives
Writing rubric results
Scope
Validation
pnpm check:knowledge(CI lint)git diff --check