perf(core): a glide in a long list lays the page out once, not once per moving element

.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

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: `<div style="display: block"><div id="content" style="height: 80px; background: #fd9">Details</div><div id="a" class="row" style="margin: 0"></div></div>`,
+    },
+    {
+      name: 'a column flex box with a gap',
+      html: `<div style="display: flex; flex-direction: column; gap: 12px"><div id="content" style="height: 80px; background: #fd9">Details</div><div id="a" class="row" style="margin: 0"></div></div>`,
+    },
+    {
+      name: 'a centered column',
+      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',
+      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 class="row"></div></div>`,
+    },
+    {
+      name: 'a column that ends a flex row',
+      html: `<div style="display: flex; gap: 8px"><div style="display: flex; flex-direction: column; flex: 1"><div class="row"></div><div id="content" style="height: 80px; background: #fd9">Details</div></div><div style="width: 60px; height: 40px; background: #ddd"></div></div><div id="a" class="row"></div>`,
+    },
+  ]) {
+    test(`an exit in ${layout.name} lands the content after it exactly where it goes once hidden (FR4)`, async ({
+      page,
+    }) => {
+      await open(
+        page,
+        `<div id="area" class="area">${layout.html}<div class="row"></div></div>`,
+      );
+      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<void>(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,
   }) => {

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(
+      '<div id="area"><div id="changed"></div><div id="a"></div><div id="b"></div><div id="c"></div></div>',
+    );
+    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(
       `<div id="area"><div id="changed"></div>
@@ -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(
       () =>

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<Element, (typeof entries)[number]>();
   // 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.