Repository navigation
perf(core): a glide in a long list lays the page out once, not once per moving element - #8639
Conversation
…t around a size change When something inside a registered area changes size, the content after it glides from its old place to its new one with a compositor-only translate instead of jumping (architecture:react-component-runtime INV12, proposed in #8463). - motion/animatedLayout.ts: one batched measurement before and after the change; a bounded parentElement/nextElementSibling walk up to the area (no TreeWalker), descending only through display: contents; absolute and fixed elements skipped; only elements whose rectangle changed and that are visible move; scale-corrected; Web Animations translate with composite: 'add', feature-tested per call with fallbacks; offscreen elements land silently; Reduce Motion cross-fades with no travel; every pointer. - useAnimatedLayout(ref) (public) registers an area; reportLayoutChange stays internal. ScrollableArea registers its viewport itself. - Collapsible and TreeList adopt it: the shown content plays its own entrance, and the chevron turns, on the same placeholder layout-change role as the glide. - Storybook: Core/Animated layout, with a nested tree, a Collapsible, a field error, load more, wrapping tags, plain divs, a display: contents wrapper, a sticky header and an absolute badge. Draft for trying it out; tests, docs and a changeset follow.
…ollapsible and TreeList take as animatedLayout
The API shape the owner set: the hook takes {containerRef} and returns an
opaque handle; Collapsible and TreeList accept an animatedLayout prop and
report their size changes through it, with no context registration.
Without the prop, a component keeps today's behavior, its chevron
included. ScrollableArea no longer registers itself: an app passes the
ScrollableArea's ref as containerRef. The story uses exactly this shape.
Adds the changeset.
…subpixel rounding A real-Chromium check showed unscaled followers moving by 79.92 px for an 80 px shift: subpixel rounding made rect.width / offsetWidth read a hair off 1. A scale within 1% of 1 is now treated as none.
…+ Add tag wraps and its followers move
…tion by pointer, visible-part rule for the changed element, top-layer and hidden skipped, chevron on the spring)
- One shared motion role: the glide, the changed element's entrance and the disclosure chevron all play springs.settle, resolved through the internal motion-role registry (a theme's --motion-springs-settle reaches all three); the glide's keyframes are sampled from the spring and played linearly. - Reduce Motion by pointer (design:reduced-motion DR1-DR3): a cross-fade under a coarse primary pointer, an immediate change otherwise. - The changed element outside the area's visible part lands without animation; hidden and top-layer elements are skipped. - Collapsible and TreeList take the chevron from their Icon's icon target and set its transition before the change commits; no handle leaves every inline value untouched. - Tests: the walk, motion and handle in jsdom; Collapsible and TreeList with and without a handle; one real-browser case per AST-105 Coverage row, plus FR2-FR5. - Docs: the useAnimatedLayout hook doc (FR7 text), the animatedLayout prop in the Collapsible and TreeList docs and records, and a Before and after story.
… registry only Motion roles are internal (spec:AST-094 DEC-11): the glide, the changed element and the chevron share the registry's one value, and a scope's --motion-springs-settle changes nothing. The tests assert that instead of an override.
… and compare scrolling with and without a handle Run in real Chromium (all 20 cases, three repeats): the page helper's microtask wait is plain JavaScript; the out-of-view and nested-area cases wait for the first change's glide to finish before the second; and the FR3 scroll case compares the area's scroll frame by frame with and without a handle, since the browser's own scroll anchoring moves it either way.
…ed ones and retargets on the compositor - A single-mode CollapsibleGroup measures every item a toggle changes (the one that closes as well as the one that opens), so the items between glide with the content after the group. Items holding a handle register their measurement with the group. - A measurement no commit claims by the next frame (a controlled owner refused the toggle) is dropped, so it never moves unchanged rows later. A change nothing measured (an owner set isOpen directly) lands at once, entrance included. - The follower glide is one replace-composited translate, which the compositor runs (architecture:react-component-runtime INV12). An element already gliding is retargeted: its glide stops and the new one starts from where it is on screen, so it keeps moving without a jump. - INV12's source boundary check (no layout property animated, the walk only through parentElement / nextElementSibling, nothing public, only the hook and the adopters use it) and a real-browser case on a throttled phone over a 10-level by 50-sibling tree with two changes in one frame.
spec:AST-105's Coverage row: an element already moving keeps its running motion and adds the new move. The new glide now samples the sum of the running glide's remaining curve and the new move into one replace-composited translate, so the element keeps its speed through the change instead of restarting from rest. Every animation of a change starts on the document timeline's current time, so a replaced glide moves in the very frame it is replaced and the changed element and its followers share one start. Tests: the carried speed (jsdom) and, in a real browser, no jump and no lost speed across the change.
…e morph once Core has it, its own compositor translate until then
…st adopt it in their own pull request The module, the hook, their tests and docs, and the INV12 record line stay here. Collapsible's and TreeList's adoption (their animatedLayout prop, docs and records), the CollapsibleGroup measurement, the adopter tests and the Storybook story move to a stacked pull request, so each lands and reverts on its own. The changeset now describes the hook alone.
Restores the adoption from the module pull request onto it: the animatedLayout prop on Collapsible and TreeList (docs and records), the CollapsibleGroup measuring every item a toggle changes, the adopter tests, and the Storybook story. The story keeps only adopting components and plain following content; its demos that drove the internal reporting functions directly are gone.
…motion spec:AST-105 FR4: the changed element plays its own animation when it exits too. holdLayoutExit keeps a closing element on screen but out of flow (absolutely positioned where it is), so the content after it glides up at once, and conceals it upward from its bottom edge on springs.settle (a fade under Reduce Motion with a coarse pointer, nothing with a fine one); the component hides it when the exit finishes, and releaseLayoutExit puts it back, including when it opens again mid-exit. The throttled-tree browser case now reads its animations in the same task, so a slow round trip can't outlast the glide.
…'s exit spec:AST-105 FR4: the changed element animates when it exits too. A measured close keeps the Collapsible's content (or the TreeList row's child group) rendered, inert and hidden from assistive technology, while holdLayoutExit holds it out of flow and conceals it on springs.settle; the rows after it glide up at once, and the content hides (or the group unmounts) when the exit ends. Opening again mid-exit ends it. Rows of an exiting group leave the tree's keyboard order at once. An unmeasured close, no handle, or an exit that lands at once hides it in the same commit.
…es with its row; the exit stays in flow - This pull request is now the internal module only: the public hook, its docs and the chevron's transition move to the adoption pull request, so nothing public appears before a component can take the handle. - A changed element that an earlier change in the same frame displaced moves with its row, by its total change (AST-105 FR3), while it plays its own entrance or exit; an element not rendered before the change has no old place and does not move. - The exit keeps the element in flow where it is and gives up its space with a negative end margin as large as its box, so it scrolls and clips with its container and keeps its width (no offset-parent positioning). - Browser proofs: the sticky row uses a stuck header after the change, the flex row a wrapped item and a row-direction neighbor whose rectangle does not change; new cases for the displaced changed element and the exit in a scrolled area.
… chevron move here The module pull request is now internal only, so this one carries the public face with the components that take it: useAnimatedLayout, its doc page and export (and its docsite route), applyLayoutChangeTransition and its tests, and the boundary test's list of users (the hook, Collapsible and TreeList). The adopters follow the module's in-flow exit: their tests check the exit's end margin instead of absolute positioning. The changeset describes the hook and the two adopters together.
…e app moves are left alone - holdLayoutExit measures the layout after the element with it hidden and sets the end margin that puts that layout there, so a gap or collapsing margins come out exact and nothing jumps when the element hides; where no margin can free the space (a grid track and its gap) the exit lands at once instead. - The held element is inert while it conceals (no focus, no pointer, nothing read); the module sets and releases it. - A follower with its own transform, translate, rotate or scale, or with a running transform animation the module did not start, takes its final place at once, as AST-105's Coverage lists for elements the app moves. - Tests: the exact space with a gap (jsdom), the grid fallback, app-moved followers; real-browser cases for a flex gap and collapsing margins, and for app-moved followers.
… left alone) into the adoption The module now makes a held element inert, so Collapsible's content and TreeList's child group drop their own inert and aria-hidden while exiting (TreeList keeps the marker that takes the group's rows out of the keyboard order).
…red close, and restores what it replaced - giveUpSpace measures the first element after the closing one in the area (its next sibling, or an ancestor's below the area), not the parent's edge, which a collapsed negative margin never moves: a Collapsible-shaped close (the last child of a plain wrapper) now plays its exit. - holdLayoutExit holds only a close a change in the area measured, and its documentation asks for the call after animate() reports it measured. - releaseLayoutExit puts back the element's own inert and inline values. - AST-105's out-of-coverage list names a closing grid item whose track or gap changes when it hides (owner-approved): it hides at once, without its exit. - Tests: an unmeasured close holds nothing; prior values restored; a real- browser case for the plain-wrapper shape.
…d holds only a measured close) into the adoption
…al next box and checks what follows its container - The flush stops every running glide and reads every new place before starting any glide, and an element inside another moving element moves only by its own change: nested changes compose whichever is reported first (AST-105 FR3). - The exit's search for the next box enters a display: contents wrapper, as the follower walk does. - The exit checks the content after its container on both axes, so a flex-row item that sets its row's height is refused instead of leaving the content below to jump. - A refused exit puts back the element's own end margin and drops its change, so the content after it takes its final place at once. - AST-105's out-of-coverage line names both layouts no margin frees (owner- approved). - Tests: a refused hold keeps its own margin and glides nothing; real- browser cases for nested changes reported inner-first, a display: contents next box, and a tall flex-row item.
…ext box and container checks) into the adoption
…d both ways, and skips positioned ancestors CodeBunny on #8634: - Every report re-walked every change already in the frame, so 100 changes in one frame read 592,325 rectangles. A frame's walks now share one cache of rectangles and boxes, the visible part is read once, and the band at least doubles when the frame's changes outgrow it, so earlier changes are walked again only a few times. Test: 100 changes over 200 rows read each row at most once (17 s and failing on the previous head). - Each level's walk now stops at the first sibling outside the band in either direction, as INV12 says, so a change outside the band walks no further than its neighbours. Test: a change at row 900 of 1,000 reads at most 3 elements. - An absolutely positioned, fixed or top-layer ancestor of the change takes its final place, as the Coverage rows say (they glided 80 px). Chromium cases for an absolute and a fixed ancestor. - The Coverage row's order case gets a Chromium test beside the centered, space-between and end-aligned rows.
…it lands CodeBunny on #8639: the direct-margin path read where the content after a closing element goes once and set the margin from it, but its test (endMarginMovesLinearly) accepted any column flex box and looked only at the parent and the next box. In a centered or space-between column, or under a disclosure that ends a column inside a flex row, the content after the element stopped short (20, 49 and 100 px) and then jumped when the element hid. The path is gone: every exit takes the guess and the measured correction, and falls back when anything checked misses. Chromium cases: in block flow, a gap column, a centered column, a space-between column and a column that ends a flex row, the content after an exit lands where it goes once the element hides, and does not move when it does.
There was a problem hiding this comment.
Incremental review of 6ab36a8..c128ce2; prior findings carried: none
This looks ready to approve: with the direct-margin shortcut removed, every held exit guesses, corrects and checks as before, and in Chromium the content after a closing element lands exactly where the base lands it, in the Core/Animated layout › Centered column story and in every other layout checked, while the read-order test still shows the flush takes all its readings before any glide starts. Two non-blocking follow-ups:
- Please refresh the description, since its second fix, its jsdom and Chromium evidence bullets and its timing table still describe the removed shortcut, which by your own estimate saved about 20 ms per collapse at 10,000 rows.
- The space-between browser case cannot fail as written, because
#ais the column's last item andspace-betweenkeeps it at the end whatever margin the exit sets, so a row after#awould make it exercise the refused hold instead.
[Automated review]
| }, | ||
| { | ||
| name: 'a space-between column', | ||
| html: `<div style="display: flex; flex-direction: column; justify-content: space-between; min-height: 400px"><div class="row"></div><div id="content" style="height: 80px; background: #fd9">Details</div><div id="a" class="row"></div></div>`, |
There was a problem hiding this comment.
This case can't fail as written: #a is the column's last item, and space-between keeps the last item at the column's end whatever margin the exit sets, so #a never moves (it also passes against the earlier direct-margin build, which set 0 px here). With another row after #a, the margin can no longer land it and the hold is refused, which would exercise that path; otherwise the case could go.
…it; inside a positioned ancestor only later content moves CodeBunny on #8634, with the owner's decision (a, scoped): - With the walk stopping at the first sibling outside the band, a change above the band never reached the rows in view, which shift and jumped where the scroller opts out of scroll anchoring (0 of 14 glided). There, the walk after a change above the band goes on to the band; with default anchoring the rows in view stay put and the walk still stops at once. INV12 states the cost. Chromium cases for both. - Inside a positioned or top-layer ancestor that takes its final place, the siblings before the change no longer glide (a Collapsible docked at the bottom had its trigger glide under the revealing content). Chromium case. - A Changeset names the earlier-sibling glide, and the adopters' Changeset says the content around a change glides, not only after it. Both new Chromium cases fail on the previous head.
There was a problem hiding this comment.
Incremental review of c128ce2..0744d42; prior findings carried: context, user_impact, evidence_by_spec_claim
This still looks ready to approve: the base-branch merge is clean and leaves the reordered flush and the held exit as they were, the read-order test still fails against the base module and passes here, and in Chromium the content after a closing element lands exactly where the base lands it, the Core/Animated layout › Centered column story included. Two non-blocking follow-ups remain:
- Please refresh the description, since its second fix, its jsdom and Chromium evidence bullets and its timing table still describe the removed exit shortcut, which by your own estimate saved about 20 ms per collapse at 10,000 rows.
- The space-between browser case cannot fail as written, because
#ais the column's last item andspace-betweenkeeps it at the end whatever margin the exit sets; a row after#ainside the column would make it exercise the refused hold instead.
[Automated review]
… content after the change moves CodeBunny on #8634: - A stuck sticky footer around an opening Collapsible glided off the area's bottom edge, while the Sticky row and FR7 keep stuck elements put. A sticky ancestor now takes its final place like a positioned one. - Inside such an ancestor the siblings before the change stayed but the wrapper around it glided, opening a gap under a header in a docked panel. One rule now applies to ancestors and siblings alike: inside a positioned, sticky or top-layer ancestor only the content after the change moves; that ancestor, the wrappers inside it and the content before the change take their final place. A new AST-105 Coverage row states it; the Changeset says the same. Chromium cases: a header and a card around the change in a bottom-docked panel, and a bottom-stuck sticky footer; both fail on the previous head.
…'s scroll; only a stuck sticky ancestor stays put CodeBunny on #8634: - When a change lands above the visible part, scroll anchoring re-scrolls the area, and every recorded ancestor moved by that scroll, so rows in view inside a wrapper slid 51 px (or faded, under Reduce Motion on a touch screen). The area's scroll change is now taken out of an ancestor's move. Chromium: the wrapped scroll-anchoring case, with and without Reduce Motion; the plain one fails on the previous head. - Every sticky ancestor took its final place, stuck or not, so in a centered column the row before an unstuck sticky card glided across it. Only a stuck sticky ancestor (resting against the area's edge at its inset) stays put now; an unstuck one is in flow. The Coverage row and the Changeset say 'stuck sticky'. Chromium case; it fails on the previous head.
…ck reads the scrollport; a column-flow level walks through CodeBunny on #8634: - Only the area's own scroll was netted out of an ancestor's move, so in a panel the page scrolls, the page's scroll anchoring made the ancestors glide and drew the rows in view 51 px off. An ancestor now moves by its place against the area's scrolled content, which nets out a scroll of the area, an outer scroller or the page. - A sticky element was judged stuck against the area's edges; it sticks to its scrollport (the nearest scrolling ancestor, or the viewport), so a footer stuck to the viewport bottom glided 51 px below it. It is judged against its scrollport now. - In a multi-column list (or a wrapping column flex box, or a column-flow grid), DOM order does not run down the page, so the band stop left a later column's rows to jump. A level laid out across the block direction now runs through its siblings; INV12 states that cost. Chromium cases: a page-scrolled panel with a wrapper, a viewport-stuck footer in a page-scrolled panel, and a 120-row two-column list. All three fail on the previous head.
…rea moves; the body is no scroller of its own CodeBunny on #8634: - When the area itself moves as its content grows (a dialog re-centered, a panel the page centers or docks), the card around the change took its final place while the siblings before the change glided by the area's move, drawing a trigger over the opening content. The siblings before the change now net out the same move as the ancestors around them. - With a non-visible overflow on body (overflow-x: hidden), the body read as a sticky footer's scroller, so a footer stuck to the viewport glided past the edge. The body's overflow applies to the viewport while the root's is visible, so the viewport is its scrollport then. Chromium cases for both; both fail on the previous head.
…maller; grid placement from past the band is out of coverage CodeBunny on #8634: - With two changes in one frame (a group closing an item above the view and opening one in view), the scroll anchoring one change caused was added to the siblings before the other change, so rows the browser kept in place glided from 344 px. An ancestor or an earlier sibling now takes the scroll and area correction only where it makes its move smaller. Chromium case; it fails on the previous head. - A grid item that grid placement brings into view from past the band in DOM order is not reached, as with order. AST-105's out-of-coverage list names grid placement beside order (owner approval needed).
|
/approve-spec 8f177d5 |
There was a problem hiding this comment.
Changes requested: the reordered flush looks right, since the new read-order test fails against the base module and passes here, a change in a long list no longer lays the page out again for every gliding row, and exits land exactly where they did, but this pull request changes @astryxdesign/core source without a Changeset, so the required lint check fails its changeset coverage step. Please add a patch Changeset naming @astryxdesign/core (for example pnpm changeset:new --packages @astryxdesign/core --category perf), or edit one of the pending .changeset/animated-layout*.md entries.
- The description's second fix, evidence bullets and timing table still describe the removed direct-margin exit, and its stacking note and
Changeset: noneline are out of date. - The space-between browser case cannot fail as written, because
#ais the column's last item and stays at the column's end whatever margin the exit sets; a row after#ainside the column would make it exercise the refused hold.
[Automated review]
…etween exit case has a row after the landing one CodeBunny on #8639: core source changed without a Changeset (the lint check's coverage step). A [perf] patch Changeset names it. The space-between browser case could not fail with the landing row last in the column; a row after it makes it exercise the exit's landing.
|
/approve-spec 9012c84 |
There was a problem hiding this comment.
This looks ready to approve: reading every follower's transform and scale before any glide starts removes the per-follower layouts, while the same rows still glide by the same amounts and every exit sets the same end margin as before. Two optional test fixes would make the new evidence hold up better:
- The space-between browser case can't fail, because its hold is refused and the remaining check reads
hiddenin the same state astarget; asserting the refusal, or dropping the case, would fix that. - The read-order unit test only watches
getComputedStyle, so movingscaleOfback into the glide loop would bring the layouts back without failing it; also watchingoffsetWidthandoffsetHeightreads after the first glide would guard both halves.
[Automated review]
| html: `<div style="display: flex; flex-direction: column; justify-content: center; min-height: 400px"><div class="row"></div><div id="content" style="height: 80px; background: #fd9">Details</div><div id="a" class="row"></div></div>`, | ||
| }, | ||
| { | ||
| name: 'a space-between column', |
There was a problem hiding this comment.
This case can't fail as written: in Chromium the hold is refused here, on the base as well, since one correction moves #a to 207.8 px against a 178 px target. So if (result.held) below skips the landing check, and hidden is read with #content at display: none, the same state target was read in. expect(result.held).toBe(false) plus a check that #a has no glide would pin the refusal; otherwise the case could go.
| // A style or layout read after a glide started lays the page out again. | ||
| const readsAfterAGlide: string[] = []; | ||
| const computed = window.getComputedStyle.bind(window); | ||
| vi.spyOn(window, 'getComputedStyle').mockImplementation( |
There was a problem hiding this comment.
This spy only sees the transform read. With entry.scale = scaleOf(...) moved back into the glide loop, a change above 10,000 rows in Chromium is back to 15 layouts, yet this suite still passes, because jsdom's offsetWidth is 0 and nothing watches it. Recording offsetWidth and offsetHeight reads after the first glide too, for example with vi.spyOn(HTMLElement.prototype, 'offsetWidth', 'get'), would guard both halves of the fix.
# Conflicts: # docs/architecture/react-component-runtime.md # docs/specs/AST-105-animated-layout/spec.md # packages/core/src/motion/__tests__/animatedLayout.interaction.browser.spec.ts # packages/core/src/motion/animatedLayout.boundary.test.ts # packages/core/src/motion/animatedLayout.test.ts # packages/core/src/motion/animatedLayout.ts
…e fix: a real conflict in animatedLayout.ts # Conflicts: # packages/core/src/motion/animatedLayout.ts
Primary intent
A perf fix in the animated-layout module: opening or closing an item in a long, fully rendered list no longer stalls the page before the glide starts.
User or maintainer impact
On the perf page (#8629), a 10,000-row list with every row rendered:
The lower figures are opening; a collapse runs about 20 ms higher at 10k rows (see below).
"Without a handle" includes the browser's own layout of the list (the perf page times it in both modes). What remains beyond it is the glide reading where things landed, plus its few extra reads.
Problem and solution fit
A CPU profile of the 10k toggle put about two thirds of the time in computed-style reads, mostly reads between writes. The flush read each follower's transform and scale (
movedByApp,scaleOf) right before starting that follower's glide. Every glide it started dirtied style, so the next reading laid the whole page out again, once per visible follower. The flush now reads every place, transform and scale first, then starts the glides.An earlier revision also let a held exit set its end margin from a single reading. That path is gone: it misjudged centered and space-between columns, and a column that ends a flex row, so the content after a closing element stopped short and jumped when it hid. Every exit keeps the guess and the measured correction, which costs about 20 ms more on a collapse at 10k rows.
No behavior changes: the same elements move by the same amounts, and exits land where they go once the element hides.
Authority
spec:AST-105(unchanged),architecture:react-component-runtimeINV12 (unchanged), and the perf ask behind #8629.Change and evidence
Scope
Testing
pnpm -F @astryxdesign/core typecheck, eslint.animatedLayout.interaction.browser.spec.tsin real Chromium.Changeset:
[perf]patch for@astryxdesign/core.