Skip to content

Commit b20f3f7

Browse files
fix(cli): check warns instead of failing on a still title card (#5369)
* fix(cli): check passes a still title card whose timeline follows the seek * fix(cli): check warns on a still frame and fails only when an animation is stuck * fix(cli): count a seek clock once per sample and point the error at the registered timeline * fix(cli): judge a finished animation by its own completion state, not its end time * test(cli): pin a Web Animation paused at its end as finished * fix(cli): judge a reversed animation's end in its own direction, and read plain timelines * test(cli): pin a plain timeline held at its end as finished * test(cli): keep an unfinished plain timeline beside the finished one * fix(cli): skip a timeline whose time cannot be read instead of calling it stuck * fix(cli): skip an animation whose time is not a plain number, such as a scroll-driven one * fix(cli): a partial timeline with plain values no longer stops the browser checks
1 parent f85614e commit b20f3f7

8 files changed

Lines changed: 306 additions & 27 deletions

File tree

‎docs/packages/cli.mdx‎

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -713,11 +713,14 @@ background colours, the measured ratio against the required one, and a
713713
compliant colour you could use instead.
714714

715715
Severity is persistence-aware: a finding at a single sample demotes to info, a
716-
finding that persists gates the exit code, and a timeline that never moves on a
717-
composition of 3s or more fails with `sweep_static`. When only audio advanced
718-
and nothing on screen moved, `sweep_static` is a warning instead (`--strict`
719-
fails the run on it); add `data-no-timeline` to a composition that is meant to
720-
be still.
716+
finding that persists gates the exit code, and a composition of 3s or more
717+
fails with `sweep_static` when its frame never changes and a GSAP timeline or
718+
tween, or a CSS or Web animation, on the page held the same time without ever
719+
finishing, so the seek never drove it.
720+
Any other frame that never changes, such as a still title card, is a
721+
`sweep_static` warning ("Nothing on screen moved under seek"), and so is an
722+
audio-only advance; `--strict` fails the run on either. A composition that is
723+
meant to be still can declare `data-no-timeline` to skip the guard.
721724

722725
{/* VISUAL: a `check --snapshots` overview frame beside one finding crop, with the labelled finding box visible on both */}
723726

@@ -771,7 +774,7 @@ borders and shadows, the pixels of visible canvas/video/img elements, and video
771774
playback time all count as motion — so a playing background video keeps a scope
772775
live on its own, even a color-graded one whose picture is drawn into a canvas.
773776
Audio playback time never counts here: it shows the timeline ran, but it is not
774-
a moving picture. The frozen-sweep guard turns an audio-only advance into a
777+
a moving picture. The frozen-sweep guard reports an audio-only advance as a
775778
warning rather than a failure. Elements under
776779
`data-layout-ignore` inside the scope never count; the opt-out does not apply to
777780
the scope element itself, since naming it in an assertion outranks it. Content

‎packages/cli/src/commands/check.test.ts‎

Lines changed: 64 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@ import type {
5151
LayoutOverflow,
5252
LayoutRect,
5353
} from "../utils/layoutAudit.js";
54+
import type { SeekClock } from "../utils/checkTypes.js";
5455
import type { ProjectDir } from "../utils/project.js";
5556

5657
const PROJECT: ProjectDir = {
@@ -166,6 +167,7 @@ function fakeDriver(overrides: Partial<CheckAuditDriver> = {}): CheckAuditDriver
166167
collectLayout: vi.fn(async (_time: number, _tolerance: number) => []),
167168
collectOverlap: vi.fn(async (_time: number) => []),
168169
collectLayoutGeometry: vi.fn(async () => `geometry-${geometryCallCount++}`),
170+
collectSeekClock: vi.fn(async () => [{ id: 1, time: 0, done: false }]),
169171
collectRotationSample: vi.fn(async (_time: number) => []),
170172
collectOffPivotRotationSample: vi.fn(async (time: number) => ({ time, samples: [] })),
171173
collectGeometryCandidates: vi.fn(async () => []),
@@ -1577,11 +1579,72 @@ describe("check pipeline", () => {
15771579
finding.code === "sweep_static" &&
15781580
finding.severity === "error" &&
15791581
finding.message.includes("did not advance") &&
1580-
finding.fixHint?.includes("data-no-timeline"),
1582+
finding.fixHint?.includes("window.__timelines"),
15811583
),
15821584
).toBe(true);
15831585
});
15841586

1587+
function stillCard(clocks: (sample: number) => SeekClock[]) {
1588+
let sample = 0;
1589+
return fakeDriver({
1590+
getDuration: vi.fn(async () => 4),
1591+
collectLayoutGeometry: vi.fn(async () => "frozen"),
1592+
collectSeekClock: vi.fn(async () => clocks(sample++)),
1593+
});
1594+
}
1595+
1596+
function sweepOf(report: CheckReport): [string, string][] {
1597+
return report.layout.findings
1598+
.filter((finding) => finding.code === "sweep_static")
1599+
.map((finding) => [finding.severity, finding.message]);
1600+
}
1601+
1602+
it.each([
1603+
[
1604+
"whose timeline follows the seek",
1605+
(sample: number) => [{ id: 1, time: sample, done: false }],
1606+
],
1607+
["with nothing that could move", () => []],
1608+
["whose only animation already holds at its end", () => [{ id: 1, time: 4, done: true }]],
1609+
[
1610+
"whose timeline is seen, listed twice, at only one sample",
1611+
(sample: number) =>
1612+
sample === 2
1613+
? [
1614+
{ id: 1, time: 0, done: false },
1615+
{ id: 1, time: 0, done: false },
1616+
]
1617+
: [],
1618+
],
1619+
])("warns, without failing, on a still card %s", async (_case, clocks) => {
1620+
const { report } = await runScenario(stillCard(clocks));
1621+
1622+
expect(sweepOf(report)).toEqual([["warning", "Nothing on screen moved under seek."]]);
1623+
expect(report.ok).toBe(true);
1624+
});
1625+
1626+
it.each([
1627+
[
1628+
"one animation is stuck while another follows the seek",
1629+
(sample: number) => [
1630+
{ id: 1, time: 0, done: false },
1631+
{ id: 2, time: sample, done: false },
1632+
],
1633+
],
1634+
[
1635+
"a tween created mid-run is stuck",
1636+
(sample: number) => [
1637+
{ id: 1, time: sample, done: false },
1638+
...(sample >= 2 ? [{ id: 2, time: 0, done: false }] : []),
1639+
],
1640+
],
1641+
])("fails when %s", async (_case, clocks) => {
1642+
const { report } = await runScenario(stillCard(clocks));
1643+
1644+
expect(sweepOf(report).map(([severity]) => severity)).toEqual(["error"]);
1645+
expect(report.ok).toBe(false);
1646+
});
1647+
15851648
it("warns, without failing, when only the audio advanced and nothing on screen moved", async () => {
15861649
let call = 0;
15871650
const driver = fakeDriver({
Lines changed: 126 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,126 @@
1+
import { afterAll, beforeAll, describe, expect, it } from "vitest";
2+
import puppeteer, { type Browser, type Page } from "puppeteer-core";
3+
import { collectSeekClock } from "./checkBrowser.js";
4+
5+
const executablePath = process.env.PUPPETEER_EXECUTABLE_PATH;
6+
7+
// Objects with GSAP's total*() and reversed() shape; GSAP itself is not a repo dependency.
8+
const PAGE = `<body>
9+
<style>
10+
@keyframes slide { to { transform: translateX(100px) } }
11+
#forever { animation: slide 1s linear infinite paused }
12+
#scrolled { animation: slide linear both; animation-timeline: scroll(root) }
13+
#viewed { animation: slide linear both; animation-timeline: view() }
14+
</style>
15+
<div id="forever"></div>
16+
<div id="scrolled"></div>
17+
<div id="viewed"></div>
18+
<div id="reverse"></div>
19+
<div id="held"></div>
20+
<div id="parked"></div>
21+
<div id="rewound"></div>
22+
<script>
23+
const clock = (total, at, reversed = false) => {
24+
let now = at;
25+
return {
26+
totalDuration: () => total,
27+
totalTime: () => now,
28+
totalProgress: () => now / total,
29+
reversed: () => reversed,
30+
seek: (t) => (now = t),
31+
};
32+
};
33+
window.registered = clock(4, 0);
34+
window.__timelines = { main: window.registered, empty: clock(0, 0) };
35+
// A yoyo that finished: its one-iteration time() is back at 0, its totalTime() is at the end.
36+
const finishedYoyo = clock(0.2, 0.2);
37+
const finishedReversed = clock(4, 0, true);
38+
window.gsap = { globalTimeline: { getChildren: () => [finishedYoyo, finishedReversed] } };
39+
const reverse = document.getElementById("reverse").animate(
40+
[{ opacity: 0 }, { opacity: 1 }],
41+
{ duration: 9000, fill: "both" },
42+
);
43+
reverse.playbackRate = -1;
44+
reverse.finish();
45+
// Paused at its end, the way a seek leaves it: never "finished", still done.
46+
const held = document.getElementById("held").animate([{ opacity: 0 }, { opacity: 1 }], { duration: 3000, fill: "forwards" });
47+
held.pause();
48+
held.currentTime = 3000;
49+
// Reversed and paused: at its end it has not started, at 0 it is complete.
50+
const backwards = (id, at) => {
51+
const animation = document.getElementById(id).animate([{ opacity: 0 }, { opacity: 1 }], { duration: 3000, fill: "both" });
52+
animation.playbackRate = -1;
53+
animation.pause();
54+
animation.currentTime = at;
55+
};
56+
backwards("parked", 3000);
57+
backwards("rewound", 0);
58+
// A registered timeline with only the one-iteration methods, which the runtime accepts.
59+
window.__timelines.partial = { duration: () => 4, time: () => 1 };
60+
window.__timelines.partialEnded = { duration: () => 4, time: () => 4 };
61+
// Seekable but with no way to read its time: not evidence either way.
62+
window.__timelines.seekOnly = { duration: () => 4, seek: () => {} };
63+
// Plain values where GSAP has methods must not stop the read.
64+
window.__timelines.plainDuration = { duration: 4, seek: () => {} };
65+
window.__timelines.plainReversed = { totalDuration: () => 4, totalTime: () => 4, reversed: false };
66+
</script>
67+
</body>`;
68+
69+
describe.runIf(executablePath)("collectSeekClock in Chromium", () => {
70+
let browser: Browser;
71+
let page: Page;
72+
beforeAll(async () => {
73+
browser = await puppeteer.launch({ executablePath, args: ["--no-sandbox"] });
74+
});
75+
afterAll(async () => {
76+
await browser?.close();
77+
});
78+
79+
async function seekTo(time: number) {
80+
await page.evaluate((t) => {
81+
Reflect.get(window, "registered").seek(t);
82+
const forever = document.getElementById("forever")?.getAnimations()[0];
83+
if (forever) forever.currentTime = t * 1000;
84+
}, time);
85+
return collectSeekClock(page);
86+
}
87+
88+
it("marks finished clocks done, an infinite one never, and keeps ids across samples", async () => {
89+
page = await browser.newPage();
90+
await page.setContent(PAGE);
91+
92+
const first = await seekTo(1);
93+
await page.evaluate(() => {
94+
const late = { totalDuration: () => 2, totalTime: () => 0, totalProgress: () => 0 };
95+
Reflect.set(window, "__timelines", { late, ...Reflect.get(window, "__timelines") });
96+
});
97+
const second = await seekTo(2);
98+
99+
expect(first.map(({ time, done }) => [time, done])).toEqual([
100+
[1, false],
101+
[1, false],
102+
[4, true],
103+
[4, true],
104+
[0.2, true],
105+
[0, true],
106+
[1000, false],
107+
[0, true],
108+
[3000, true],
109+
[3000, false],
110+
[0, true],
111+
]);
112+
const [late, ...rest] = second;
113+
expect(rest.map(({ id }) => id)).toEqual(first.map(({ id }) => id));
114+
expect(first.map(({ id }) => id)).not.toContain(late?.id);
115+
expect(second.map(({ time }) => time)).toEqual([0, 2, 1, 4, 4, 0.2, 0, 2000, 0, 3000, 3000, 0]);
116+
await page.close();
117+
});
118+
119+
it("reports nothing on a page with no animations", async () => {
120+
page = await browser.newPage();
121+
await page.setContent("<body><h1>Hello</h1></body>");
122+
123+
expect(await collectSeekClock(page)).toEqual([]);
124+
await page.close();
125+
});
126+
});

‎packages/cli/src/utils/checkBrowser.ts‎

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@ import type {
5252
OffPivotRotationSample,
5353
RotationSample,
5454
RunAuditGrid,
55+
SeekClock,
5556
} from "./checkTypes.js";
5657
import type { ProjectDir } from "./project.js";
5758

@@ -461,6 +462,7 @@ function createPageDriver(page: Page, setTime: (time: number) => void): CheckAud
461462
collectLayout: (time, tolerance, layout) => collectLayout(page, time, tolerance, layout),
462463
collectOverlap: (time) => collectOverlap(page, time),
463464
collectLayoutGeometry: () => collectLayoutGeometry(page),
465+
collectSeekClock: () => collectSeekClock(page),
464466
collectRotationSample: (time) => collectRotationSample(page, time),
465467
collectOffPivotRotationSample: (time) => collectOffPivotRotationSample(page, time),
466468
collectGeometryCandidates: (time, request) => collectGeometryCandidates(page, time, request),
@@ -612,6 +614,58 @@ async function collectLayoutGeometry(page: Page): Promise<string> {
612614
});
613615
}
614616

617+
export async function collectSeekClock(page: Page): Promise<SeekClock[]> {
618+
// Serialized into the page; each optional GSAP read is one branch of one function.
619+
// fallow-ignore-next-line complexity
620+
return page.evaluate(() => {
621+
const callOrUndefined = (target: unknown, key: string, args: unknown[] = []): unknown => {
622+
try {
623+
return Reflect.apply(Reflect.get(Object(target), key), target, args);
624+
} catch {
625+
return undefined;
626+
}
627+
};
628+
const known: { ids: WeakMap<object, number>; next: number } = Reflect.get(
629+
window,
630+
"__hfSeekClocks",
631+
) ?? {
632+
ids: new WeakMap(),
633+
next: 0,
634+
};
635+
Reflect.set(window, "__hfSeekClocks", known);
636+
const idOf = (target: object): number => {
637+
if (!known.ids.has(target)) known.ids.set(target, ++known.next);
638+
return known.ids.get(target) ?? 0;
639+
};
640+
const clocks: SeekClock[] = [];
641+
const timelines: unknown = Reflect.get(window, "__timelines");
642+
const registered = typeof timelines === "object" && timelines ? Object.values(timelines) : [];
643+
const gsapRoot = Reflect.get(Reflect.get(window, "gsap") ?? {}, "globalTimeline");
644+
const children = callOrUndefined(gsapRoot, "getChildren", [false, true, true]);
645+
for (const timeline of [...registered, ...(Array.isArray(children) ? children : [])]) {
646+
const total = Number(
647+
callOrUndefined(timeline, "totalDuration") ?? callOrUndefined(timeline, "duration"),
648+
);
649+
if (typeof timeline !== "object" || !timeline || !(total > 0)) continue;
650+
const time = Number(
651+
callOrUndefined(timeline, "totalTime") ?? callOrUndefined(timeline, "time"),
652+
);
653+
if (!Number.isFinite(time)) continue;
654+
const progress = Number(callOrUndefined(timeline, "totalProgress") ?? time / total);
655+
const done = callOrUndefined(timeline, "reversed") === true ? progress <= 0 : progress >= 1;
656+
clocks.push({ id: idOf(timeline), time, done });
657+
}
658+
for (const animation of document.getAnimations?.() ?? []) {
659+
if (typeof animation.currentTime !== "number") continue;
660+
const time = animation.currentTime;
661+
const end = Number(animation.effect?.getComputedTiming().endTime ?? Number.POSITIVE_INFINITY);
662+
const atEnd = animation.playbackRate < 0 ? time <= 0 : time >= end;
663+
clocks.push({ id: idOf(animation), time, done: animation.playState === "finished" || atEnd });
664+
}
665+
return clocks;
666+
});
667+
}
668+
615669
/** Invoke a `window.__hyperframes*` sampler injected by layout-audit.browser.js
616670
* and return its array result (or [] when absent / non-array). Shared by the
617671
* per-frame sample collectors so the page.evaluate boilerplate lives once. */

0 commit comments

Comments
 (0)