diff --git a/README.md b/README.md index d1a42f4..217b937 100644 --- a/README.md +++ b/README.md @@ -139,6 +139,33 @@ test("a test where spinners are expected to hang", async ({ page }) => { }); ``` +### motionWaiter + +Waits for a *moving* target to settle before pointer actions (`click`, `dblclick`, `hover`). Playwright's own stability check only compares the target's bounding box across two consecutive frames, so timer-driven JS animation (React Native web's `Animated`, `setInterval` steppers) that steps coarser than the display refresh gets clicked mid-slide — the click lands on a half-open drawer, and video-mode's click-moment freeze bakes the clipped panel into the recording. + +motionWaiter samples the target's bounding box over a longer window: the action proceeds only once the box has been observed holding still for `settledFor` (~150ms — the per-action cost on static elements, and a real window on purpose: an element often sits parked for a frame or two before its animation starts, React Native's open → requestAnimationFrame → animate shape). + +Because every guarded action pays that stillness window, the plugin is **off by default**: register it, then opt in around the specific interactions whose animations are known to defeat Playwright's stability check — + +```ts +await motionWaiter.settings.run({ enabled: true }, () => + page.getByRole("button", { name: "Notifications" }).click(), +); +``` + +— or pass `enabled: true` at registration to guard a whole suite. The wait is budgeted — perpetual motion (marquees, rotating icons) proceeds at `settleTimeout` with a log line instead of blocking. Opacity-only fades never engage it (the box doesn't move), and step cadences slower than `sampleInterval` can pass the initial check, same as vanilla Playwright. + +```ts +motionWaiter({ + enabled: true, // guard every pointer action (default: false — opt in per block) + settleTimeout: 1_500, // max wait for motion to stop + sampleInterval: 60, // ms between bounding-box samples + settledFor: 150, // stillness window required before proceeding +}); +``` + +Register it after `spinnerWaiter` (`plugins: [spinnerWaiter(), motionWaiter()]`): spinner-waiter's fast-fail path passes an explicit 1ms timeout inward, and motionWaiter — like spinnerWaiter itself — treats an explicit `{ timeout }` as the author taking charge of timing and passes straight through. The same `settings.enterWith` / `settings.run` runtime overrides apply — `enterWith` turns it on for the rest of a test, `run` scopes it to one action. + ### hydrationWaiter Before each action, waits for `[data-hydrated="false"]` to disappear. Your app cooperates by rendering that attribute server-side and flipping it once the framework hydrates. Stops the classic "test clicked a button before React attached the handler" flake at the source. diff --git a/spec/lint-plugin.spec.ts b/spec/lint-plugin.spec.ts index d46063a..9bc52fa 100644 --- a/spec/lint-plugin.spec.ts +++ b/spec/lint-plugin.spec.ts @@ -261,6 +261,7 @@ test("reports timeout options without an explanation", async () => { const output = `${result.stdout}\n${result.stderr}`; expect(output).toContain("middlewright(require-timeout-comment)"); expect(output).toContain("remove the timeout and add loading UI for spinnerWaiter"); + expect(output).toContain("motionWaiter.settings.run({ enabled: true }"); expect(output).toContain( "https://github.com/iterate/middlewright#dont-fix-slow-tests-with-longer-timeouts", ); @@ -448,6 +449,7 @@ test("reports bare waitForTimeout sleeps", async () => { const output = `${result.stdout}\n${result.stderr}`; expect(output).toContain("middlewright(require-timeout-comment)"); expect(output).toContain("a sleep waits whether or not the app is ready"); + expect(output).toContain("motionWaiter.settings.run({ enabled: true }"); expect(await readFile(fixture.sourcePath, "utf8")).toBe(source); }); diff --git a/spec/motion-drawer-demo.spec.ts b/spec/motion-drawer-demo.spec.ts new file mode 100644 index 0000000..fb6dd0d --- /dev/null +++ b/spec/motion-drawer-demo.spec.ts @@ -0,0 +1,173 @@ +import { expect, test } from "@playwright/test"; +import { addPlugins, motionWaiter, videoMode } from "../src/index.ts"; + +test.use({ + video: "on", + viewport: { height: 720, width: 480 }, +}); + +// A drawer that slides in over 700ms, stepped by a JS TIMER (the React Native +// web Animated / legacy jQuery shape). The 48ms steps are coarser than the +// display refresh, so plenty of consecutive frame pairs are identical +// mid-slide — which defeats Playwright's own two-frame stability check. + +test("control: without motion-waiter the click lands on a still-sliding drawer", async ({ + page: basePage, +}, testInfo) => { + await using page = await addPlugins({ + page: basePage, + testInfo, + // The heading's appearance needs no highlight of its own — footage runs + // continuously from the click through the app's pressed-flash and fade. + plugins: [videoMode({ skipMethods: ["waitFor"] })], + }); + await page.setContent(getDrawerAppHtml()); + + await page.videoMode.caption("Open the menu: the drawer starts sliding in", () => + page.getByRole("button", { name: "Open menu" }).click(), + ); + await page.videoMode.caption("Playwright clicks while the drawer is still sliding", () => + page.getByRole("button", { name: "Notifications" }).click(), + ); + // timeout: the app holds a 700ms pressed flash + 200ms fade before navigating — nothing there for a spinner-waiter + await page.getByRole("heading", { name: "Notifications" }).waitFor({ timeout: 5_000 }); + // Outlast the menu's fade-out so the recording ends on a settled screen. + // timeout: fade completion, invisible to the spinner-waiter + await page.locator("#overlay").waitFor({ state: "hidden", timeout: 5_000 }); + + // The app records the drawer's translateX at the moment the click landed. + // Vanilla Playwright clicks while the drawer is still well off to the left — + // the recording freezes a half-open drawer under the click pointer. + const drawerXAtClick = await page.evaluate(() => (window as any).__drawerXAtClick); + console.log(`drawer translateX at click: ${drawerXAtClick}px (drawer is 280px wide)`); + expect(drawerXAtClick).toBeLessThan(-40); +}); + +test("opting in for the drawer click waits out the slide", async ({ + page: basePage, +}, testInfo) => { + await using page = await addPlugins({ + page: basePage, + testInfo, + // motionWaiter OUTSIDE videoMode: its settle hold completes before + // video-mode reads the action timing, so the flagged watchable span (the + // drawer's slide) renders at full speed instead of compressing away. + plugins: [motionWaiter(), videoMode({ skipMethods: ["waitFor"] })], + }); + await page.setContent(getDrawerAppHtml()); + + await page.videoMode.caption("Open the menu: the drawer starts sliding in", () => + page.getByRole("button", { name: "Open menu" }).click(), + ); + await page.videoMode.caption("motion-waiter holds the click until the drawer settles", () => + motionWaiter.settings.run({ enabled: true }, () => + page.getByRole("button", { name: "Notifications" }).click(), + ), + ); + // timeout: the app holds a 700ms pressed flash + 200ms fade before navigating — nothing there for a spinner-waiter + await page.getByRole("heading", { name: "Notifications" }).waitFor({ timeout: 5_000 }); + // Outlast the menu's fade-out so the recording ends on a settled screen. + // timeout: fade completion, invisible to the spinner-waiter + await page.locator("#overlay").waitFor({ state: "hidden", timeout: 5_000 }); + + // Same app, same clicks — the one opted-in click was held until the slide + // finished, so it landed on the drawer at rest (translateX ≈ 0). + const drawerXAtClick = await page.evaluate(() => (window as any).__drawerXAtClick); + console.log(`drawer translateX at click: ${drawerXAtClick}px (drawer is 280px wide)`); + expect(drawerXAtClick).toBeGreaterThan(-1); +}); + +function getDrawerAppHtml() { + return ` + + + + + + +
+ + Drawer demo +
+
+

Home

+

Open the menu and pick a section.

+
+ + + + + `; +} diff --git a/spec/motion-waiter.spec.ts b/spec/motion-waiter.spec.ts new file mode 100644 index 0000000..41141ff --- /dev/null +++ b/spec/motion-waiter.spec.ts @@ -0,0 +1,119 @@ +import { test as base, expect } from "@playwright/test"; +import { addPlugins, motionWaiter, videoMode } from "../src/index.ts"; + +const test = base.extend({ + page: async ({ page: basePage }, use, testInfo) => { + await using page = await addPlugins({ + page: basePage, + testInfo, + plugins: [motionWaiter()], + }); + await page.setContent(getMovingButtonHtml()); + await use(page); + }, +}); + +test("a static element clicks after a short stillness check", async ({ page }) => { + motionWaiter.settings.enterWith({ enabled: true }); + const start = Date.now(); + await page.getByRole("button", { name: "static" }).click(); + expect(Date.now() - start).toBeLessThan(700); + await page.getByText("clicked static at x=0").waitFor(); +}); + +test("a timer-stepped slide is waited out; the click lands at rest", async ({ page }) => { + motionWaiter.settings.enterWith({ enabled: true }); + // 800ms slide, stepped every 48ms — identical consecutive display frames + // mid-slide, so vanilla Playwright would click while it moves. + await page.evaluate(() => (window as any).startSlide(200, 800)); + await page.getByRole("button", { name: "sliding" }).click(); + await page.getByText("clicked sliding at x=200").waitFor(); +}); + +test("off by default: the same click lands mid-slide until a block opts in", async ({ page }) => { + await page.evaluate(() => (window as any).startSlide(200, 800)); + await page.getByRole("button", { name: "sliding" }).click(); + const clickedAt = await page.evaluate(() => (window as any).__clickedAtX); + expect(clickedAt).toBeGreaterThanOrEqual(0); + expect(clickedAt).toBeLessThan(200); +}); + +test("perpetual motion proceeds once the settle budget runs out", async ({ page }) => { + motionWaiter.settings.enterWith({ enabled: true }); + await page.evaluate(() => (window as any).startMarquee()); + const start = Date.now(); + await page.getByRole("button", { name: "sliding" }).click(); + const elapsed = Date.now() - start; + expect(elapsed).toBeGreaterThanOrEqual(motionWaiter.defaults.settleTimeout); + await page.getByText(/clicked sliding at x=\d/).waitFor(); +}); + +test("an explicit timeout passes straight through, like spinner-waiter's escape hatch", async ({ + page, +}) => { + motionWaiter.settings.enterWith({ enabled: true }); + await page.evaluate(() => (window as any).startSlide(200, 800)); + // timeout: the explicit-timeout escape hatch IS the subject under test — motion-waiter passes through, like spinner-waiter does + await page.getByRole("button", { name: "sliding" }).click({ timeout: 5_000 }); + const clickedAt = await page.evaluate(() => (window as any).__clickedAtX); + expect(clickedAt).toBeLessThan(200); +}); + +base("a motion hold is flagged watchable, so video-mode keeps the footage", async ({ + page: basePage, +}, testInfo) => { + const video = videoMode({ finalHold: 0, highlight: false }); + await using page = await addPlugins({ + page: basePage, + testInfo, + plugins: [motionWaiter(), video], + }); + await page.setContent(getMovingButtonHtml()); + motionWaiter.settings.enterWith({ enabled: true }); + + await page.evaluate(() => (window as any).startSlide(200, 800)); + await page.getByRole("button", { name: "sliding" }).click(); + + // The ~800ms settle hold precedes video-mode's middleware, which would + // normally record it as one fat dead-air span and compress the slide away. + // The watchable flag carves it out — no dead-air span may span the hold. + const { deadAir } = await video.metadata(); + expect(deadAir.filter((span) => span.end - span.start > 400)).toEqual([]); +}); + +function getMovingButtonHtml() { + return ` + + + +
+ + + `; +} diff --git a/src/lint/plugin.ts b/src/lint/plugin.ts index fb800c8..d23ee80 100644 --- a/src/lint/plugin.ts +++ b/src/lint/plugin.ts @@ -101,6 +101,7 @@ const requireTimeoutComment = { - If there's no loading UI, remove the timeout and add loading UI for spinnerWaiter. - If there's a loading UI but it takes even longer than the spinnerWaiter spinner timeout, use \`await spinnerWaiter.settings.run({ spinnerTimeout: 123_456 }, async () => ...)\` or similar to wait even longer for the spinner to complete. - If there's loading UI from a part of the code that we don't control (e.g. a library) and it isn't matched by the default spinner selectors, use \`await spinnerWaiter.settings.run({ spinnerSelectors: ["myCustomSpinnerClass"] }, async () => ...)\`. + - If the wait is really about an ANIMATION (a sliding drawer, a settling panel), don't time it — use \`await motionWaiter.settings.run({ enabled: true }, () => locator.click())\` to wait for the target to stop moving. - If it is truly impossible for there to be loading UI, add a nearby // comment matching every required pattern: {{patterns}}. - If you're in a block which has done \`await spinnerWaiter.settings.run({ disabled: true }, async () => ...)\`, you should probably *un-disable* for that block and apply the above suggestions to the inner code. @@ -109,6 +110,7 @@ const requireTimeoutComment = { sleep: dedent` Avoid waitForTimeout — a sleep waits whether or not the app is ready. Best ways to resolve: - Wait for positive UI instead: a locator wait covers readiness, and spinnerWaiter extends it while loading UI shows. + - If the sleep lets an ANIMATION finish before clicking (a sliding drawer, a settling panel), use \`await motionWaiter.settings.run({ enabled: true }, () => locator.click())\` — it waits for the target to stop moving instead of guessing a duration. - If the sleep paces a recording, let video mode pace instead (it holds popup entry and settles the recorder itself); still-needed manual pacing is a library gap worth filing. - If it is truly necessary, add a nearby // comment matching every required pattern: {{patterns}}. diff --git a/src/plugin-system.ts b/src/plugin-system.ts index 7788774..1ab7e26 100644 --- a/src/plugin-system.ts +++ b/src/plugin-system.ts @@ -85,6 +85,12 @@ export type ActionTiming = { attachedAt?: number; attachedAtStart: boolean; middlewares: ActionMiddlewareTiming[]; + /** + * `performance.now()` spans a middleware flags as watchable footage — a + * settle wait spanning a real on-screen animation, say. Video renderers + * keep these at full speed instead of compressing them as dead air. + */ + watchableSpans: { startedAt: number; endedAt: number }[]; }; /** Function that calls the next middleware or the original action */ @@ -453,6 +459,7 @@ const patchLocatorPrototype = ( attachedAt: attachedAtStart ? actionStartedAt : undefined, attachedAtStart, middlewares: [], + watchableSpans: [], }; const stopObservingAttached = attachedAtStart ? () => {} diff --git a/src/plugins/index.ts b/src/plugins/index.ts index e660004..3f82f2e 100644 --- a/src/plugins/index.ts +++ b/src/plugins/index.ts @@ -18,6 +18,7 @@ export { type VideoModeSpan, type VideoModeViewport, } from "./video-mode.ts"; +export { motionWaiter, type MotionWaiterOptions } from "./motion-waiter.ts"; export { spinnerWaiter, type SpinnerWaiterOptions, defaultSelectors } from "./spinner-waiter.ts"; export { uiErrorReporter, type UIErrorReporterOptions } from "./ui-error-reporter.ts"; export { diff --git a/src/plugins/motion-waiter.ts b/src/plugins/motion-waiter.ts new file mode 100644 index 0000000..0f8b6b1 --- /dev/null +++ b/src/plugins/motion-waiter.ts @@ -0,0 +1,213 @@ +/** + * motion-waiter: don't click elements that are still sliding. + * + * Playwright's own actionability check only requires the target's bounding box + * to be identical across TWO consecutive animation frames. Smooth CSS + * transitions move every frame, so Playwright waits those out — but + * timer-driven JS animation (React Native web's Animated, setInterval + * steppers, legacy jQuery.animate) steps coarser than the display refresh + * rate, so plenty of consecutive frame pairs are identical mid-slide. + * Playwright declares the element stable and clicks it while it's still + * moving: the click lands somewhere the user would never have aimed, and a + * recording freezes a half-open panel at the click moment. + * + * This plugin samples the target's bounding box over a longer window before + * pointer actions and proceeds only once the box holds still — a + * technique-agnostic motion detector (CSS, WAAPI, rAF and timer steppers all + * move the box; opacity-only fades deliberately don't engage it). The wait is + * budgeted: perpetual motion (marquees, rotating icons) proceeds at the + * deadline with a log line instead of blocking forever. + */ +import { AsyncLocalStorage } from "node:async_hooks"; +import type { ActionContext, Plugin } from "../plugin-system.ts"; +import { adjustError, oneArgMethods } from "../plugin-system.ts"; + +export type MotionWaiterOptions = { + /** Max time to wait for the target to stop moving (ms). Default: 1_500 */ + settleTimeout?: number; + /** Interval between bounding-box samples (ms). Default: 60 */ + sampleInterval?: number; + /** + * How long the box must be observed holding still before the action + * proceeds (ms). This is the per-action cost on static elements, and it is + * deliberately a real window rather than a single confirming sample: an + * element often sits parked for a frame or two before its animation starts + * (React Native's open → requestAnimationFrame → animate shape), and it + * also defeats step cadences slower than the sample interval. Default: 150 + */ + settledFor?: number; + /** Movement smaller than this many px counts as still (subpixel jitter). Default: 0.5 */ + epsilon?: number; + /** + * Whether motion checking is on. Default: false — every guarded action pays + * the stillness window, so the plugin is opt-in: register it, then enable + * it around the specific interactions whose animations are known to defeat + * Playwright's stability check (`motionWaiter.settings.run({ enabled: true }, + * ...)`), or pass `enabled: true` at registration for a whole suite. + */ + enabled?: boolean; + /** Debug logging function */ + log?: (message: string) => void; +}; + +/** The actions where a moving target produces a mis-click or an ugly click-moment frame. */ +const pointerMethods = new Set(["click", "dblclick", "hover"]); + +const oneArgMethodNames = new Set(oneArgMethods); + +const defaults: Required = { + settleTimeout: 1_500, + sampleInterval: 60, + settledFor: 150, + epsilon: 0.5, + enabled: false, + log: () => {}, +}; + +/** AsyncLocalStorage for runtime settings override */ +const settingsStorage = new AsyncLocalStorage>(); + +const getSettings = (baseOptions: MotionWaiterOptions = {}) => { + const runtimeOverrides = settingsStorage.getStore() || {}; + const result = { ...defaults, ...baseOptions, ...runtimeOverrides }; + if (result.settleTimeout <= result.sampleInterval) { + throw new Error("settleTimeout must be greater than sampleInterval"); + } + return result; +}; + +/** + * Creates a motion-waiter plugin. + * + * Runtime settings can be overridden per-test via + * `motionWaiter.settings.enterWith({...})`, or for a single call via + * `motionWaiter.settings.run({...}, () => locator.click())`. The plugin is + * OFF by default — the intended usage is to opt in around the interactions + * whose animations are known to be problematic: + * + * await motionWaiter.settings.run({ enabled: true }, () => + * page.getByRole("button", { name: "Notifications" }).click()); + * + * Register it INSIDE spinner-waiter (`plugins: [spinnerWaiter(), + * motionWaiter()]`): spinner-waiter's fast-fail path passes an explicit 1ms + * timeout down, which motion-waiter treats — like spinner-waiter itself does — + * as the author (or an outer middleware) taking charge of timing. + */ +export const motionWaiter = Object.assign( + (options: MotionWaiterOptions = {}): Plugin => { + if (process.env.PWDEBUG) { + return { name: "motion-waiter" }; + } + + return { + name: "motion-waiter", + + middleware: async ({ args, locator, method, timing }, next) => { + const settings = getSettings(options); + if (!settings.enabled || !pointerMethods.has(method)) return next(); + + // An explicitly passed { timeout } is the author (or an outer + // middleware like spinner-waiter's fast-fail) saying "I own the + // timing of this action" — same escape hatch as spinner-waiter. + if (explicitTimeout(method, args) !== undefined) return next(); + + const start = Date.now(); + const perfStart = performance.now(); + const deadline = start + settings.settleTimeout; + // The hold is real on-screen animation — flag it so video renderers + // keep the footage instead of compressing it as pre-action dead air. + const flagWatchable = () => { + timing.watchableSpans.push({ endedAt: performance.now(), startedAt: perfStart }); + }; + // The current run of identical samples: the box and when that run started. + let still: { box: Box; since: number } | null = null; + let sawMotion = false; + + while (Date.now() < deadline) { + // Any boundingBox failure (strict-mode violation, closed page) is + // next()'s to report with its own richer error. + const box = await boundingBox(locator, settings.sampleInterval); + const now = Date.now(); + if (box === null) { + // Not attached/visible yet. Appearance is next()'s job, but an + // element that appears and immediately slides in is exactly the + // case this plugin exists for — keep sampling until the deadline. + still = null; + sawMotion = true; + } else if (still !== null && sameBox(still.box, box, settings.epsilon)) { + if (now - still.since >= settings.settledFor) { + if (sawMotion) { + settings.log( + `${locator}.${method}(...) target settled after ${now - start}ms of motion`, + ); + flagWatchable(); + } + return next(); + } + } else { + if (still !== null) sawMotion = true; + still = { box, since: now }; + } + await sleep(settings.sampleInterval); + } + + settings.log( + `${locator}.${method}(...) target still moving after the ${settings.settleTimeout}ms settle budget, proceeding`, + ); + if (sawMotion) flagWatchable(); + try { + return await next(); + } catch (error) { + adjustError( + error as Error, + [ + `The target was still moving when the ${settings.settleTimeout}ms motion-settle budget ran out.`, + `If the motion is perpetual (a marquee, a rotating icon), disable the wait for this action:`, + ` await motionWaiter.settings.run({ enabled: false }, () => locator.${method}(...))`, + ], + "motion-waiter.ts", + ); + throw error; + } + }, + }; + }, + { + /** Runtime settings override via AsyncLocalStorage */ + settings: settingsStorage, + /** Default settings values */ + defaults, + }, +); + +type Box = { x: number; y: number; width: number; height: number }; + +async function boundingBox(locator: ActionContext["locator"], intervalMs: number): Promise { + try { + // timeout: one quick position sample — the sampling loop owns the waiting (and spinner-waiter applies inside next()), not this read + return await locator.boundingBox({ timeout: Math.max(intervalMs, 50) }); + } catch { + return null; + } +} + +function sameBox(a: Box, b: Box, epsilon: number) { + return ( + Math.abs(a.x - b.x) <= epsilon && + Math.abs(a.y - b.y) <= epsilon && + Math.abs(a.width - b.width) <= epsilon && + Math.abs(a.height - b.height) <= epsilon + ); +} + +/** The author-passed timeout option for this action, if any. */ +function explicitTimeout(method: ActionContext["method"], args: unknown[]): number | undefined { + const options = args[oneArgMethodNames.has(method) ? 1 : 0]; + if (typeof options !== "object" || options === null || Array.isArray(options)) return undefined; + const timeout = (options as Record).timeout; + return typeof timeout === "number" ? timeout : undefined; +} + +function sleep(ms: number) { + return new Promise((resolve) => setTimeout(resolve, ms)); +} diff --git a/src/plugins/video-mode.ts b/src/plugins/video-mode.ts index fc5dd3e..059366e 100644 --- a/src/plugins/video-mode.ts +++ b/src/plugins/video-mode.ts @@ -311,6 +311,8 @@ type VideoModeState = { children: VideoModeChild[]; deadAirDepth: number; deadAirSpans: VideoModeSpan[]; + /** Footage windows middlewares flagged via ActionTiming.watchableSpans — carved out of dead air. */ + watchableSpans: VideoModeSpan[]; highlights: VideoModeHighlight[]; highlightImageIndex: number; lastDialogEndedAt?: number; @@ -856,7 +858,13 @@ const metadataFor = (state: VideoModeState): VideoModeMetadata => { ...child, highlights: normalizeVideoHighlights(child.highlights), })), - deadAir: mergeVideoSpans(state.deadAirSpans), + // Watchable spans (a motion-settle hold over a sliding panel, say) are + // carved out of dead air, like popup enter/exit animations: compression + // must not fast-forward footage a middleware flagged as worth watching. + deadAir: subtractVideoSpans( + mergeVideoSpans(state.deadAirSpans), + mergeVideoSpans(state.watchableSpans), + ), highlights: normalizeVideoHighlights(state.highlights), outputs: state.outputs, schemaVersion: 2, @@ -2355,6 +2363,21 @@ const recordActionElapsedDeadAirFromTiming = ( recordDeadAirSpan(state, { end, start }); }; +const recordWatchableSpans = (state: VideoModeState, timing: ActionTiming) => { + if (state.startedAt === undefined) { + return; + } + + for (const span of timing.watchableSpans) { + const start = Math.round(span.startedAt - state.startedAt); + const end = Math.round(span.endedAt - state.startedAt); + + if (end > start) { + state.watchableSpans.push({ end, start }); + } + } +}; + const recordMiddlewareWaitBeforeVideoMode = ( state: VideoModeState, timing: ActionTiming, @@ -4865,6 +4888,7 @@ const videoModeActionMiddleware = (options: { } recordMiddlewareWaitBeforeVideoMode(state, timing); + recordWatchableSpans(state, timing); if (skipMethods.includes(method)) { try { @@ -5007,6 +5031,7 @@ export const videoMode = (options: VideoModeOptions = {}): VideoModePlugin => { children: [], deadAirDepth: 0, deadAirSpans: [], + watchableSpans: [], highlightImageIndex: 0, highlights: [], outputs: {}, @@ -5095,6 +5120,9 @@ export const videoMode = (options: VideoModeOptions = {}): VideoModePlugin => { children: [], deadAirDepth: 0, deadAirSpans: state.deadAirSpans, + // Like dead air, the child shares the parent's watchable list: spans + // land on the parent clock and protect the same timeline. + watchableSpans: state.watchableSpans, highlightImageIndex: 0, highlights: child.highlights, outputs: {}, @@ -5526,8 +5554,13 @@ export const videoMode = (options: VideoModeOptions = {}): VideoModePlugin => { // composite shares the raw timeline, so nothing downstream changes. let renderInputPath = paths.raw; // Popup enter/exit animations must reach the output even when a - // hold's overlap-skip would jump across them. - const renderKeepSpans: VideoModeSpan[] = []; + // hold's overlap-skip would jump across them. Watchable spans (a + // motion-settle hold over a sliding panel) get the same protection: + // without it, two pointer actions within one hold of each other + // skip the slide entirely. + const renderKeepSpans: VideoModeSpan[] = mergeVideoSpans(state.watchableSpans).map( + (span) => translateVideoSpan(span, timelineOffset), + ); const childLayers = await childCompositeLayers({ children: metadataBeforeVideo.children, outputDir: testInfo.outputDir, diff --git a/tasks/motion-waiter.md b/tasks/motion-waiter.md new file mode 100644 index 0000000..405a881 --- /dev/null +++ b/tasks/motion-waiter.md @@ -0,0 +1,94 @@ +--- +status: in-progress +size: medium +--- + +# motion-waiter: don't click elements that are still sliding + +## Status summary + +Done pending review: plugin, demo spec (before/after), unit spec, exports, +README, and the PR body carries before/after + todo-app baseline videos. + +## Problem + +Playwright's actionability check only requires the target's bounding box to be +identical across TWO consecutive animation frames. Smooth CSS +transitions/animations move every frame, so Playwright waits for those — but +**timer-driven JS animation** (RN-web `Animated`'s JS driver, `setInterval` +steppers, legacy `$.animate`) steps coarser than the display refresh rate, so +plenty of consecutive frame pairs are identical mid-slide. Playwright declares +the element stable and clicks it while it's still moving. + +Real-world hit (iterate repo, PR iterate/iterate#2547): a mobile drawer slides +in over ~180ms via RN Animated; the spec's click landed mid-slide, and +video-mode's click-moment freeze frame baked a half-open drawer into the +recording. The spec grew a hand-rolled motion probe (two `boundingBox()` +samples 120ms apart), which review deleted with "let the video suffer — we'll +fix it in middlewright if we care to". This is that fix. + +## Decisions (assumptions made while fleshing out) + +- **New plugin, not a spinner-waiter feature.** Motion is a distinct + cross-cutting concern (AGENTS.md plugin boundaries); spinner-waiter owns + loading UI, motion-waiter owns kinetics. +- **Detect motion by sampling the target's bounding box over time**, not by + enumerating techniques (`getAnimations()` misses JS-driven motion — the very + case that bit us). Box sampling is technique-agnostic: CSS + transitions/animations, WAAPI, rAF steppers, and timer steppers all move the + box. Opacity-only fades deliberately don't engage it (clicking a fading + element is harmless). +- **Pointer actions only** (`click`, `dblclick`, `hover`) — the actions where + a moving target produces a mis-click or an ugly click-moment frame. +- **Budgeted, never blocking**: 1.5s `settleTimeout` default (judgement call: + it's a cap only paid for perpetual motion; typical cost is + motion-duration + 150ms); perpetual motion proceeds at the deadline with a + log line, and a subsequent action failure gets a still-moving hint appended. +- **A real stillness window, always**: the action proceeds only once the box + has been observed holding still for `settledFor` (150ms) — ~150-200ms per + pointer action on static elements. Deliberately NOT a single confirming + sample: an element often sits parked a frame or two before its animation + starts (React Native's open → requestAnimationFrame → animate shape — the + motivating bug), and a zero-quiet fast path would sail right through that + parked window. Step cadences slower than `sampleInterval` can still pass + (documented boundary, same as vanilla Playwright). +- **Off by default** (review feedback): every guarded action pays the + stillness window, so the plugin opts in — register it, then + `motionWaiter.settings.run({ enabled: true }, ...)` around the specific + interactions with known-problematic animations, or `enabled: true` at + registration for a whole suite. +- **Escape hatches match spinner-waiter**: `settings.run({ disabled: true })` + per block, an author-passed explicit `{ timeout }` passes straight through + (which also makes `[spinnerWaiter(), motionWaiter()]` compose: the + fast-fail's injected 1ms timeout skips the inner motion wait), `PWDEBUG` + disables the plugin. + +## Checklist + +- [x] Repro demo: a slow (700ms) timer-driven sliding drawer app + + `spec/motion-drawer-demo.spec.ts` where the un-helped click provably + lands mid-slide, for the "before" video _— control click lands at + translateX −225px of a 280px drawer (≈20% open); the app records the + offset at click time, freezes the slide with a pressed-item flash so the + stranded drawer is visible on video, and the tests caption the videos + via `page.videoMode.caption`_ +- [x] `src/plugins/motion-waiter.ts`: the plugin per the decisions above +- [x] "after" demo test: same app with `motionWaiter()` — click-time offset is + the settled position _— translateX 0px_ +- [x] `spec/motion-waiter.spec.ts`: fast path (static click < 700ms), + timer-stepped slide settles, perpetual marquee proceeds at the deadline, + disabled + explicit-timeout passthrough _— 5 tests_ +- [x] Export from `src/plugins/index.ts` / `src/index.ts`, README section + _— README gets a motionWaiter section between spinnerWaiter and + hydrationWaiter_ +- [x] PR body: before/after videos (user-attachments URLs) + todo-app baseline + video per AGENTS.md _— all three render as inline players on PR #42_ + +## Implementation notes + +- Full suite: 159 passed on the final run (an earlier run had a one-off + `spec/popup-video.spec.ts` failure under full parallelism that also + reproduces-then-passes on main — pre-existing flake, not this change). +- The marquee unit test steps every 40ms: cadences slower than + `sampleInterval` can sit inside a hold across the stillness window + (documented boundary shared with vanilla Playwright).