Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions .changeset/carried-pending-animation-in-step.md
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -403,20 +404,25 @@ 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`. */
interface MotionRecord {
readonly seen: Map<Animation, MotionPart>;
/** Press feedback, apart from the motion. */
readonly press: Map<Animation, PressPart>;
readonly collect: () => void;
/** The time of each frame the record sampled. */
readonly frames: {t: number}[];
readonly collect: (at?: number) => void;
sampling: boolean;
}

Expand All @@ -434,11 +440,15 @@ async function recordMotion(page: Page): Promise<void> {
await toggleButton(page).evaluate(toggle => {
const seen = new Map<Animation, MotionPart>();
const press = new Map<Animation, PressPart>();
const collect = () => {
const frames: {t: number}[] = [];
const collect = (at?: number) => {
const now = at ?? performance.now();
const present = new Set<Animation>();
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);
}
Expand All @@ -459,7 +469,22 @@ async function recordMotion(page: Page): Promise<void> {
: 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<string>();
Expand All @@ -480,12 +505,27 @@ async function recordMotion(page: Page): Promise<void> {
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);
}
};
Expand Down Expand Up @@ -542,21 +582,36 @@ async function settledMotion(page: Page): Promise<ReadonlyArray<MotionPart>> {
* 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<ReadonlyArray<{t: number}>> {
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,
Expand Down Expand Up @@ -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),
Expand Down
23 changes: 23 additions & 0 deletions packages/core/src/motion/carryAnimations.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Comment on lines +172 to +174

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Optional: the first sentence restates the two branches below. Keeping only the quirk is enough, for example: "A pending play has no start time. Its start resolves on the frame that first draws the animation." The comment in restore() can then keep only its reason, that the two run in step.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Kept for this head, which the spec owner has stamped. The trim to the pending-start quirk goes in the next change to this file.

*/
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;
Expand Down Expand Up @@ -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);
}
}
}

Expand Down
Loading