diff --git a/.changeset/animated-layout-exit-reads.md b/.changeset/animated-layout-exit-reads.md new file mode 100644 index 0000000000000..e46c2fc95fccf --- /dev/null +++ b/.changeset/animated-layout-exit-reads.md @@ -0,0 +1,7 @@ +--- +'@astryxdesign/core': patch +--- + +[perf] With an `animatedLayout` handle, a Collapsible or TreeList change in a long list reads every row's place before any glide starts, so the page is laid out once rather than once per gliding row. + +@cixzhang diff --git a/packages/core/src/motion/__tests__/animatedLayout.interaction.browser.spec.ts b/packages/core/src/motion/__tests__/animatedLayout.interaction.browser.spec.ts index c7870455d8d6d..861ae64b1b43b 100644 --- a/packages/core/src/motion/__tests__/animatedLayout.interaction.browser.spec.ts +++ b/packages/core/src/motion/__tests__/animatedLayout.interaction.browser.spec.ts @@ -1136,6 +1136,73 @@ test.describe('spec:AST-105 FR2–FR4', () => { expect(follower[0].duration).toBeLessThan(1200); }); + for (const layout of [ + { + name: 'block flow', + html: `
Details
`, + }, + { + name: 'a column flex box with a gap', + html: `
Details
`, + }, + { + name: 'a centered column', + html: `
Details
`, + }, + { + name: 'a space-between column', + html: `
Details
`, + }, + { + name: 'a column that ends a flex row', + html: `
Details
`, + }, + ]) { + test(`an exit in ${layout.name} lands the content after it exactly where it goes once hidden (FR4)`, async ({ + page, + }) => { + await open( + page, + `
${layout.html}
`, + ); + const result = await page.evaluate(async () => { + const area = document.querySelector('#area'); + const content = document.querySelector('#content') as HTMLElement; + const a = document.querySelector('#a') as HTMLElement; + const handle = window.AnimatedLayout.createAnimatedLayout(() => area); + const report = window.AnimatedLayout.reportLayoutChange( + handle, + () => content, + ); + report.measureBefore(); + report.animate(); + // Where the content after it lands once the element is gone. + content.style.display = 'none'; + const target = a.getBoundingClientRect().top; + content.style.display = ''; + const held = window.AnimatedLayout.holdLayoutExit( + handle, + content, + () => {}, + ); + await new Promise(resolve => { + queueMicrotask(resolve); + }); + a.getAnimations().forEach(animation => animation.finish()); + const landed = a.getBoundingClientRect().top; + // When the element hides, nothing after it moves again. + content.style.display = 'none'; + window.AnimatedLayout.releaseLayoutExit(content); + const hidden = a.getBoundingClientRect().top; + return {held, landed, hidden, target}; + }); + if (result.held) { + expect(Math.abs(result.landed - result.target)).toBeLessThan(0.5); + } + expect(Math.abs(result.hidden - result.target)).toBeLessThan(0.5); + }); + } + test('a closing element exits on the same motion while the content after it glides up at once (FR4)', async ({ page, }) => { diff --git a/packages/core/src/motion/animatedLayout.test.ts b/packages/core/src/motion/animatedLayout.test.ts index 07551008b4674..4428f4e93b163 100644 --- a/packages/core/src/motion/animatedLayout.test.ts +++ b/packages/core/src/motion/animatedLayout.test.ts @@ -194,6 +194,43 @@ describe('the walk (spec:AST-105 FR3, Coverage)', () => { } }); + it('reads every follower (place, transform, scale) before it starts any glide, so a long page is laid out once', async () => { + const {area, changed, phase} = setUp( + '
', + ); + for (const [id, y] of [ + ['a', 0], + ['b', 40], + ['c', 80], + ] as const) { + placeAt( + document.getElementById(id) as HTMLElement, + {before: [0, y, 400, 40], after: [0, y + 80, 400, 40]}, + phase, + ); + } + // 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( + (element: Element, pseudo?: string | null) => { + if (played.some(p => p.keyframes[0].translate != null)) { + readsAfterAGlide.push((element as HTMLElement).id); + } + return computed(element, pseudo); + }, + ); + await runChange( + createAnimatedLayout(() => area), + changed, + phase, + ); + expect(played.filter(p => p.keyframes[0].translate != null)).toHaveLength( + 3, + ); + expect(readsAfterAGlide).toEqual([]); + }); + it('takes the children of a display: contents wrapper and skips absolute, fixed, hidden and display: none elements', async () => { const {area, changed, phase} = setUp( `
@@ -523,6 +560,16 @@ describe('the changed element exits (spec:AST-105 FR4)', () => { expect(content.style.pointerEvents).toBe(''); }); + it('guesses, then corrects, where a negative margin would collapse with its end margin', () => { + const {content, after, layout} = exitSetUp(); + after.style.marginTop = '-4px'; + const reads = vi.mocked(after.getBoundingClientRect); + reads.mockClear(); + expect(holdLayoutExit(layout, content, vi.fn())).toBe(true); + expect(reads.mock.calls.length).toBeGreaterThan(2); + expect(content.style.getPropertyValue('margin-block-end')).toBe('-88px'); + }); + it('stops an exit when the content opens again', () => { const {content, layout} = exitSetUp(); const done = vi.fn(); @@ -545,7 +592,8 @@ describe('the changed element exits (spec:AST-105 FR4)', () => { }); it('lands at once where no end margin frees its space (a grid track)', () => { - const {content, after, layout} = exitSetUp(); + const {area, content, after, layout} = exitSetUp(); + area.style.display = 'grid'; // The row after it ignores the margin: it moves only when the content // is hidden. vi.spyOn(after, 'getBoundingClientRect').mockImplementation( @@ -567,7 +615,9 @@ describe('the changed element exits (spec:AST-105 FR4)', () => { }); it("a refused hold keeps the element's own end margin and lets the content after it take its place at once", async () => { - const {content, after, layout} = exitSetUp(); + const {area, content, after, layout} = exitSetUp(); + // A grid track, which no end margin frees. + area.style.display = 'grid'; content.style.setProperty('margin-block-end', '12px'); vi.spyOn(after, 'getBoundingClientRect').mockImplementation( () => diff --git a/packages/core/src/motion/animatedLayout.ts b/packages/core/src/motion/animatedLayout.ts index eb352ea370ec9..4d49e98787003 100644 --- a/packages/core/src/motion/animatedLayout.ts +++ b/packages/core/src/motion/animatedLayout.ts @@ -728,6 +728,8 @@ function flush(batch: Batch): void { after?: DOMRect; rawX?: number; rawY?: number; + isMovedByApp?: boolean; + scale?: {x: number; y: number}; }[] = []; for (const element of followers) { const before = batch.before.get(element); @@ -756,6 +758,9 @@ function flush(batch: Batch): void { } entries.push({element, before, carried, carriedMs}); } + // Every reading (places, transforms, scales) before any glide starts: a + // glide started between two readings would make the next one lay the page + // out again, once per element (on a long page, the whole cost). const byElement = new Map(); // An ancestor of the change, and a sibling before it, move by their layout // change alone: when scroll anchoring re-scrolls the area, an outer @@ -781,6 +786,13 @@ function flush(batch: Batch): void { entry.rawY = isAncestor ? nearer(rawY, scrolledY) : rawY; byElement.set(entry.element, entry); } + for (const entry of entries) { + const after = entry.after as DOMRect; + if (intersects(entry.before, visible) || intersects(after, visible)) { + entry.isMovedByApp = movedByApp(entry.element); + entry.scale = scaleOf(entry.element, after); + } + } for (const entry of entries) { const {element, before, carried, carriedMs} = entry; const after = entry.after as DOMRect; @@ -814,14 +826,14 @@ function flush(batch: Batch): void { }); continue; } - if (movedByApp(element)) { + if (entry.isMovedByApp === true) { // The app moves it itself: it takes its final place at once // (`spec:AST-105` Coverage). continue; } // Where it shows now, against its new place, in its own px; the part the // carried glide does not cover is the new move. - const scale = scaleOf(element, after); + const scale = entry.scale ?? {x: 1, y: 1}; const now = carried(0); const moveX = dx / scale.x - now.x; const moveY = dy / scale.y - now.y; @@ -1093,6 +1105,9 @@ function giveUpSpace(element: HTMLElement, area: Element): boolean { ? null : nextInFlow(element.parentElement, area); const checked = afterParent == null ? [next] : [next, afterParent]; + // The element's scale, read in the layout the caller's visibility check + // already laid out. + const scale = scaleOf(element, element.getBoundingClientRect()); // Where the content after it goes once it is hidden, read with it taken // out of flow (not `display: none`, which would restart the CSS animations // inside it). @@ -1106,14 +1121,14 @@ function giveUpSpace(element: HTMLElement, area: Element): boolean { } else { element.style.setProperty('position', position, positionPriority); } + // The reading is in screen px, the margin in the element's own: under a + // scaled ancestor a distance is divided by the scale on this axis. + const perPx = isRow ? scale.x : scale.y; // A first guess (its box), then one correction from the measured result: // a gap, a collapsed margin or the element's own margins. element.style.setProperty(property, `${-size}px`); const guess = placeOf(next, rtl)[axis]; - // The reading is in screen px, the margin in the element's own: under a - // scaled ancestor the correction is divided by the scale on this axis. - const scale = scaleOf(element, element.getBoundingClientRect()); - const correction = (target[axis] - guess) / (isRow ? scale.x : scale.y); + const correction = (target[axis] - guess) / perPx; element.style.setProperty(property, `${-size + correction}px`); // Everything checked must land on both axes: a flex-row item that sets // its row's height frees no block space with an inline margin.