From dc59ea5812966e0d4168d5ce9afa17747c0b0e01 Mon Sep 17 00:00:00 2001 From: Christopher Chedeau Date: Mon, 9 Nov 2026 22:23:35 -0800 Subject: [PATCH] fix(motion): a carried pending animation runs in step with its original A transient copy carries its element's animations by start time. One saved while its play was still pending had no start time yet, so the copy took only its current time and started on a frame of its own. On a loaded machine, the SideNav rail's leaving copy started the toggle's press fade 333 ms after the toggle's. The copy now takes the start its original resolves to, at once or once the original's play starts. Both callers get it: the transient copy and the layer runtime's re-placement. The rail's reduced-motion press proof read the copy's start only in frame callbacks, which on a busy page ran after the copy was gone. It also reads the start as the fade turns ready, and it keeps a frame trace. A copy fade that never started fails as frame starvation only when the trace shows no frame while the copy held it; otherwise it fails plainly. --- .../carried-pending-animation-in-step.md | 7 ++ ...eNavRailMotion.interaction.browser.spec.ts | 99 ++++++++++++++++--- packages/core/src/motion/carryAnimations.ts | 23 +++++ 3 files changed, 115 insertions(+), 14 deletions(-) create mode 100644 .changeset/carried-pending-animation-in-step.md diff --git a/.changeset/carried-pending-animation-in-step.md b/.changeset/carried-pending-animation-in-step.md new file mode 100644 index 0000000000000..4d633a2cf980b --- /dev/null +++ b/.changeset/carried-pending-animation-in-step.md @@ -0,0 +1,7 @@ +--- +'@astryxdesign/core': patch +--- + +[fix] A transient copy that carries an animation still waiting for its first frame now runs it in step with the original. On a busy page, the SideNav rail's leaving copy could start the toggle's press fade a frame or more after the toggle's own. + +@vjeux diff --git a/packages/core/src/SideNav/__tests__/SideNavRailMotion.interaction.browser.spec.ts b/packages/core/src/SideNav/__tests__/SideNavRailMotion.interaction.browser.spec.ts index 3b61feb2adb6f..e57634f99fb46 100644 --- a/packages/core/src/SideNav/__tests__/SideNavRailMotion.interaction.browser.spec.ts +++ b/packages/core/src/SideNav/__tests__/SideNavRailMotion.interaction.browser.spec.ts @@ -28,6 +28,7 @@ import { serveStorybook, type StaticServer, } from '@astryxdesign/a11y-spec/storybook'; +import {requireFrames} from '@astryxdesign/interaction-harness'; // The frame evidence is Chromium's: CI's interaction proofs run the chromium // project, and these cases have not been run in WebKit. @@ -403,12 +404,15 @@ interface MotionPart { * One press feedback animation `recordMotion` keeps apart: the toggle's own, * or on a copy of it the old state holds (`onCopy`). `name` is a CSS * animation's; `startTime` is read while it runs, since a cancelled animation - * drops it. + * drops it. `firstSeen` and `goneAt` are the record's frame times when it + * first saw the animation and first missed it. */ interface PressPart { readonly onCopy: boolean; readonly name: string | null; startTime: number | null; + readonly firstSeen: number; + goneAt: number | null; } /** The record `recordMotion` keeps on the page, for `settledMotion`. */ @@ -416,7 +420,9 @@ interface MotionRecord { readonly seen: Map; /** Press feedback, apart from the motion. */ readonly press: Map; - readonly collect: () => void; + /** The time of each frame the record sampled. */ + readonly frames: {t: number}[]; + readonly collect: (at?: number) => void; sampling: boolean; } @@ -434,11 +440,15 @@ async function recordMotion(page: Page): Promise { await toggleButton(page).evaluate(toggle => { const seen = new Map(); const press = new Map(); - const collect = () => { + const frames: {t: number}[] = []; + const collect = (at?: number) => { + const now = at ?? performance.now(); + const present = new Set(); for (const animation of document.getAnimations()) { const effect = animation.effect; const known = press.get(animation); if (known != null) { + present.add(animation); if (animation.startTime != null) { known.startTime = Number(animation.startTime); } @@ -459,7 +469,22 @@ async function recordMotion(page: Page): Promise { : null, startTime: animation.startTime == null ? null : Number(animation.startTime), + firstSeen: now, + goneAt: null, }); + present.add(animation); + // The start resolves on the frame that first draws the animation. + // On a busy page, that frame's callbacks can run after the copy + // is gone, so the start is also read as the animation turns ready. + animation.ready.then( + ready => { + const part = press.get(ready); + if (part != null && ready.startTime != null) { + part.startTime = Number(ready.startTime); + } + }, + () => undefined, + ); continue; } const properties = new Set(); @@ -480,12 +505,27 @@ async function recordMotion(page: Page): Promise { effect.target.contains(document.querySelector('[role="main"]')), }); } + for (const [animation, part] of press) { + if (part.goneAt == null && !present.has(animation)) { + part.goneAt = now; + } + } + }; + const record: MotionRecord = { + seen, + press, + frames, + collect, + sampling: true, }; - const record: MotionRecord = {seen, press, collect, sampling: true}; (window as unknown as {railMotion: MotionRecord}).railMotion = record; const sample = () => { if (record.sampling) { - collect(); + // One time for the frame and what it saw, so the frame that first + // misses an animation is not counted as holding it. + const t = performance.now(); + frames.push({t}); + collect(t); requestAnimationFrame(sample); } }; @@ -542,21 +582,36 @@ async function settledMotion(page: Page): Promise> { * settled. For each: where it ran (the toggle, or a copy of it), a CSS * animation's name, and its start time as read while it ran. */ -async function pressFeedback( - page: Page, -): Promise< - ReadonlyArray<{onCopy: boolean; name: string | null; startTime: number}> +async function pressFeedback(page: Page): Promise< + ReadonlyArray<{ + onCopy: boolean; + name: string | null; + startTime: number; + firstSeen: number; + goneAt: number | null; + }> > { return page.evaluate(() => { const record = (window as unknown as {railMotion: MotionRecord}).railMotion; - return [...record.press.values()].map(({onCopy, name, startTime}) => ({ - onCopy, - name, - startTime: startTime ?? Number.NaN, - })); + return [...record.press.values()].map( + ({onCopy, name, startTime, firstSeen, goneAt}) => ({ + onCopy, + name, + startTime: startTime ?? Number.NaN, + firstSeen, + goneAt, + }), + ); }); } +/** The time of each frame the record sampled. */ +async function recordedFrames(page: Page): Promise> { + return page.evaluate( + () => (window as unknown as {railMotion: MotionRecord}).railMotion.frames, + ); +} + test.describe('component:SideNav FR6 and component:AppShell FR17 under a finger', () => { test('a tap collapses and expands the rail with its edge on the content on every frame, no width animation, and a hidden copy gone at rest', async ({ page, @@ -727,6 +782,22 @@ test.describe('component:SideNav FR6 and component:AppShell FR17 under a finger' other => !other.onCopy && other.name === fade.name, ); expect(own, JSON.stringify(press)).toBeDefined(); + if (Number.isNaN(fade.startTime)) { + // The copy's fade never started. A new animation's start reaches + // script only after the frame that starts it. So the copy is judged + // once the page drew two frames while it held the fade. Fewer is + // starvation when the trace shows a long gap. + requireFrames(await recordedFrames(page), { + min: 2, + during: "the old state's copy held the toggle's press fade", + where: frame => + frame.t > fade.firstSeen && + (fade.goneAt == null || frame.t < fade.goneAt), + }); + throw new Error( + `the copy's press fade never started while the page drew frames: ${JSON.stringify(press)}`, + ); + } expect( Math.abs(fade.startTime - (own?.startTime ?? Number.NaN)), JSON.stringify(press), diff --git a/packages/core/src/motion/carryAnimations.ts b/packages/core/src/motion/carryAnimations.ts index e743c0c3f3b81..ca1becfd33884 100644 --- a/packages/core/src/motion/carryAnimations.ts +++ b/packages/core/src/motion/carryAnimations.ts @@ -168,6 +168,24 @@ function restoreEffect(next: KeyframeEffect, saved: SavedAnimation): void { } } +/** + * Gives `next` the start time `original` runs on, now or, while its play is + * still pending, once that play starts. A pending play has no start time + * yet: it resolves on the frame that first draws the animation. + */ +function followStart(next: Animation, original: Animation): void { + const take = () => { + if (original.startTime != null && next.playState !== 'idle') { + next.startTime = original.startTime; + } + }; + if (original.startTime != null) { + take(); + return; + } + original.ready.then(take, () => undefined); +} + /** Give `next` the state `saved` had, member by member. */ function restore(next: Animation, saved: SavedAnimation): void { const effect = next.effect as KeyframeEffect | null; @@ -214,6 +232,11 @@ function restore(next: Animation, saved: SavedAnimation): void { next.startTime = start; } else if (time != null) { next.currentTime = time; + // A running one saved while its play was pending had no start time yet. + // It takes the start its original resolves to, so the two run in step. + if (saved.playState === 'running') { + followStart(next, saved.animation); + } } }