From 4f17178b3eb12eb8972645a2a795e9aad6eeed6b Mon Sep 17 00:00:00 2001 From: Misha Kaletsky <15040698+mmkal@users.noreply.github.com> Date: Sat, 29 Aug 2026 17:38:44 +0100 Subject: [PATCH 1/8] =?UTF-8?q?Task:=20motion-waiter=20=E2=80=94=20don't?= =?UTF-8?q?=20click=20elements=20that=20are=20still=20sliding?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fleshed-out spec for teaching middlewright about motion: Playwright's 2-frame stability check misses timer-driven JS animation, so clicks land mid-slide (seen with a RN Animated drawer in iterate). Plan: a bounding- box-sampling motionWaiter plugin with a 1s settle budget, plus a slow- drawer demo spec producing before/after videos. Co-Authored-By: Claude Fable 5 --- tasks/motion-waiter.md | 68 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 68 insertions(+) create mode 100644 tasks/motion-waiter.md diff --git a/tasks/motion-waiter.md b/tasks/motion-waiter.md new file mode 100644 index 0000000..618c1f6 --- /dev/null +++ b/tasks/motion-waiter.md @@ -0,0 +1,68 @@ +--- +status: in-progress +size: medium +--- + +# motion-waiter: don't click elements that are still sliding + +## Status summary + +Spec fleshed out, implementation not started. Main pieces: the repro demo spec +(before video), the `motionWaiter` plugin, the after video, exports + docs. + +## 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**: default 1s `settleTimeout`; perpetual motion + (marquees, spinners that rotate the box) proceeds at the deadline with a log + line rather than failing. +- **Cheap when nothing moves**: one extra sample interval (~60ms) per action. + Only once motion is observed does the plugin require a longer quiet window + (~150ms of unchanged box) before proceeding — that defeats step cadences + slower than the sample interval. +- **Escape hatches match spinner-waiter**: `settings.run({ disabled: true })` + per block, an author-passed explicit `{ timeout }` passes straight through, + `PWDEBUG` disables the plugin. + +## Checklist + +- [ ] Repro demo: a slow (≈1.2s) timer-driven sliding drawer app + + `spec/motion-drawer-demo.spec.ts` where the un-helped click provably + lands mid-slide (the app records the drawer's offset at click time), for + the "before" video +- [ ] `src/plugins/motion-waiter.ts`: the plugin per the decisions above +- [ ] "after" demo test: same app with `motionWaiter()` — click-time offset is + the settled position +- [ ] `spec/motion-waiter.spec.ts`: fast path (static element ≈ one interval), + timer-stepped slide settles, perpetual marquee proceeds at deadline, + `disabled` + explicit-timeout passthrough +- [ ] Export from `src/plugins/index.ts` / `src/index.ts`, README section +- [ ] PR body: before/after videos (user-attachments URLs) + todo-app baseline + video per AGENTS.md From 87c6afd83f7163ba837aeedbb1edd290392a152d Mon Sep 17 00:00:00 2001 From: Misha Kaletsky <15040698+mmkal@users.noreply.github.com> Date: Sat, 29 Aug 2026 17:45:46 +0100 Subject: [PATCH 2/8] motionWaiter: sample the target's bounding box until it stops moving Playwright's stability check compares only two consecutive frames, so timer-driven JS animation (RN-web Animated, setInterval steppers) that steps coarser than the display refresh gets clicked mid-slide. The new plugin samples the target's box before pointer actions: a static element passes after one confirming sample (~60ms), observed motion demands a 150ms quiet window, and everything is capped by a 1.5s settle budget so perpetual motion proceeds with a log line instead of blocking. The drawer demo spec is the proof: the control click lands at translateX(-240.8px) on a 280px drawer (~14% open); with motionWaiter the same click lands at 0. Explicit { timeout } passes through, which also composes with spinner-waiter's fast-fail (its injected 1ms timeout skips the motion wait). Co-Authored-By: Claude Fable 5 --- README.md | 22 ++++ spec/motion-drawer-demo.spec.ts | 131 +++++++++++++++++++++ spec/motion-waiter.spec.ts | 96 ++++++++++++++++ src/plugins/index.ts | 1 + src/plugins/motion-waiter.ts | 194 ++++++++++++++++++++++++++++++++ tasks/motion-waiter.md | 55 +++++---- 6 files changed, 480 insertions(+), 19 deletions(-) create mode 100644 spec/motion-drawer-demo.spec.ts create mode 100644 spec/motion-waiter.spec.ts create mode 100644 src/plugins/motion-waiter.ts diff --git a/README.md b/README.md index d1a42f4..8679e83 100644 --- a/README.md +++ b/README.md @@ -139,6 +139,28 @@ 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: a static element passes after one confirming sample (~60ms), but once motion is observed the box must hold still for `settledFor` before the action proceeds. 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({ + settleTimeout: 1_500, // max wait for motion to stop + sampleInterval: 60, // ms between bounding-box samples + settledFor: 150, // quiet window required once motion was seen +}); +``` + +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: + +```ts +await motionWaiter.settings.run({ disabled: true }, () => + page.getByText("stock ticker").click(), +); +``` + ### 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/motion-drawer-demo.spec.ts b/spec/motion-drawer-demo.spec.ts new file mode 100644 index 0000000..20c6f7d --- /dev/null +++ b/spec/motion-drawer-demo.spec.ts @@ -0,0 +1,131 @@ +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 ~1s, 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, + plugins: [videoMode()], + }); + await page.setContent(getDrawerAppHtml()); + + await page.getByRole("button", { name: "Open menu" }).click(); + await page.getByRole("button", { name: "Notifications" }).click(); + await page.getByRole("heading", { name: "Notifications" }).waitFor(); + + // 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("with motion-waiter the click waits for the drawer to settle", async ({ + page: basePage, +}, testInfo) => { + await using page = await addPlugins({ + page: basePage, + testInfo, + plugins: [motionWaiter(), videoMode()], + }); + await page.setContent(getDrawerAppHtml()); + + await page.getByRole("button", { name: "Open menu" }).click(); + await page.getByRole("button", { name: "Notifications" }).click(); + await page.getByRole("heading", { name: "Notifications" }).waitFor(); + + // Same app, same clicks — motion-waiter held the click 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..3379a64 --- /dev/null +++ b/spec/motion-waiter.spec.ts @@ -0,0 +1,96 @@ +import { test as base, expect } from "@playwright/test"; +import { addPlugins, motionWaiter } 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 with barely any overhead", async ({ page }) => { + 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 }) => { + // 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("without the plugin the same click lands mid-slide (the gap this plugin closes)", async ({ + page, +}) => { + motionWaiter.settings.enterWith({ disabled: true }); + 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 }) => { + 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, +}) => { + 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); +}); + +function getMovingButtonHtml() { + return ` + + + +
+ + + `; +} 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..517e512 --- /dev/null +++ b/src/plugins/motion-waiter.ts @@ -0,0 +1,194 @@ +/** + * 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; + /** + * Once motion has been observed, how long the box must hold still before the + * action proceeds (ms) — 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 to skip motion checking. Default: false */ + disabled?: 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, + disabled: 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())`. + * + * 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 }, next) => { + const settings = getSettings(options); + if (settings.disabled || !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 deadline = start + settings.settleTimeout; + // 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)) { + // A static element passes on its first confirming sample (~one + // interval of overhead); once motion was seen, demand a real + // quiet window. + if (now - still.since >= (sawMotion ? settings.settledFor : 0)) { + if (sawMotion) { + settings.log( + `${locator}.${method}(...) target settled after ${now - start}ms of motion`, + ); + } + 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`, + ); + 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({ disabled: true }, () => 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/tasks/motion-waiter.md b/tasks/motion-waiter.md index 618c1f6..fdb7a13 100644 --- a/tasks/motion-waiter.md +++ b/tasks/motion-waiter.md @@ -7,8 +7,9 @@ size: medium ## Status summary -Spec fleshed out, implementation not started. Main pieces: the repro demo spec -(before video), the `motionWaiter` plugin, the after video, exports + docs. +Plugin, demo spec (before/after), unit spec, exports and README all done and +passing. Remaining: attach before/after videos + the todo-app baseline video +to the PR body. ## Problem @@ -40,29 +41,45 @@ fix it in middlewright if we care to". This is that fix. 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**: default 1s `settleTimeout`; perpetual motion - (marquees, spinners that rotate the box) proceeds at the deadline with a log - line rather than failing. +- **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. - **Cheap when nothing moves**: one extra sample interval (~60ms) per action. Only once motion is observed does the plugin require a longer quiet window - (~150ms of unchanged box) before proceeding — that defeats step cadences - slower than the sample interval. + (150ms of unchanged box) before proceeding — that defeats step cadences + slower than the sample interval. Cadences slower than `sampleInterval` can + pass the initial check (documented boundary, same as vanilla Playwright). - **Escape hatches match spinner-waiter**: `settings.run({ disabled: true })` - per block, an author-passed explicit `{ timeout }` passes straight through, - `PWDEBUG` disables the plugin. + 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 -- [ ] Repro demo: a slow (≈1.2s) timer-driven sliding drawer app + +- [x] Repro demo: a slow (1s) timer-driven sliding drawer app + `spec/motion-drawer-demo.spec.ts` where the un-helped click provably - lands mid-slide (the app records the drawer's offset at click time), for - the "before" video -- [ ] `src/plugins/motion-waiter.ts`: the plugin per the decisions above -- [ ] "after" demo test: same app with `motionWaiter()` — click-time offset is - the settled position -- [ ] `spec/motion-waiter.spec.ts`: fast path (static element ≈ one interval), - timer-stepped slide settles, perpetual marquee proceeds at deadline, - `disabled` + explicit-timeout passthrough -- [ ] Export from `src/plugins/index.ts` / `src/index.ts`, README section + lands mid-slide, for the "before" video _— control click lands at + translateX −240.8px of a 280px drawer (≈14% open); the app records the + offset at click time_ +- [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_ - [ ] PR body: before/after videos (user-attachments URLs) + todo-app baseline video per AGENTS.md + +## Implementation notes + +- Full suite: 158 passed; `spec/popup-video.spec.ts` failed once under full + parallelism but passes in isolation on this branch AND on main (main's tip + commit is itself a video-flake fix) — pre-existing flake, not this change. +- The marquee unit test steps every 40ms: a 120ms-step marquee sits inside the + documented fast-path boundary (holds longer than `sampleInterval` look + static on the first confirming sample). From 020de8453d913c1e83222a2dcf75ffdd838c89bd Mon Sep 17 00:00:00 2001 From: Misha Kaletsky <15040698+mmkal@users.noreply.github.com> Date: Sat, 29 Aug 2026 17:56:25 +0100 Subject: [PATCH 3/8] Demo legibility + always require a real stillness window MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The zero-quiet fast path would sail through an element parked a frame or two before its animation starts (RN's open → requestAnimationFrame → animate shape — the motivating bug), so the action now proceeds only after settledFor (150ms) of observed stillness — ~150-200ms per pointer action on static elements, the price of catching pause-then-slide. Demo: the drawer app freezes its slide with a pressed-item flash when a click lands, so the before video visibly strands the drawer part way out; both tests caption their videos via page.videoMode.caption. Co-Authored-By: Claude Fable 5 --- README.md | 2 +- spec/motion-drawer-demo.spec.ts | 41 +++++++++++++++++++++++---------- spec/motion-waiter.spec.ts | 2 +- src/plugins/motion-waiter.ts | 14 +++++------ tasks/motion-waiter.md | 21 ++++++++++------- 5 files changed, 51 insertions(+), 29 deletions(-) diff --git a/README.md b/README.md index 8679e83..fb435b6 100644 --- a/README.md +++ b/README.md @@ -143,7 +143,7 @@ test("a test where spinners are expected to hang", async ({ page }) => { 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: a static element passes after one confirming sample (~60ms), but once motion is observed the box must hold still for `settledFor` before the action proceeds. 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. +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). 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({ diff --git a/spec/motion-drawer-demo.spec.ts b/spec/motion-drawer-demo.spec.ts index 20c6f7d..5c9a760 100644 --- a/spec/motion-drawer-demo.spec.ts +++ b/spec/motion-drawer-demo.spec.ts @@ -6,7 +6,7 @@ test.use({ viewport: { height: 720, width: 480 }, }); -// A drawer that slides in over ~1s, stepped by a JS TIMER (the React Native +// 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. @@ -21,8 +21,12 @@ test("control: without motion-waiter the click lands on a still-sliding drawer", }); await page.setContent(getDrawerAppHtml()); - await page.getByRole("button", { name: "Open menu" }).click(); - await page.getByRole("button", { name: "Notifications" }).click(); + 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(), + ); await page.getByRole("heading", { name: "Notifications" }).waitFor(); // The app records the drawer's translateX at the moment the click landed. @@ -43,8 +47,12 @@ test("with motion-waiter the click waits for the drawer to settle", async ({ }); await page.setContent(getDrawerAppHtml()); - await page.getByRole("button", { name: "Open menu" }).click(); - await page.getByRole("button", { name: "Notifications" }).click(); + 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", () => + page.getByRole("button", { name: "Notifications" }).click(), + ); await page.getByRole("heading", { name: "Notifications" }).waitFor(); // Same app, same clicks — motion-waiter held the click until the slide @@ -100,17 +108,18 @@ function getDrawerAppHtml() { const overlay = document.getElementById("overlay"); const drawer = document.getElementById("drawer"); const DRAWER_WIDTH = 280; - const SLIDE_MS = 1000; + const SLIDE_MS = 700; const STEP_MS = 48; // JS-timer stepping, coarser than the display refresh + let slideTimer; document.querySelector("header button").addEventListener("click", () => { overlay.hidden = false; const startedAt = Date.now(); - const timer = setInterval(() => { + slideTimer = setInterval(() => { const progress = Math.min((Date.now() - startedAt) / SLIDE_MS, 1); const eased = 1 - Math.pow(1 - progress, 3); drawer.style.transform = "translateX(" + -DRAWER_WIDTH * (1 - eased) + "px)"; - if (progress >= 1) clearInterval(timer); + if (progress >= 1) clearInterval(slideTimer); }, STEP_MS); }); @@ -118,10 +127,18 @@ function getDrawerAppHtml() { item.addEventListener("click", () => { const transform = new DOMMatrixReadOnly(getComputedStyle(drawer).transform); window.__drawerXAtClick = transform.m41; - overlay.hidden = true; - drawer.style.transform = "translateX(" + -DRAWER_WIDTH + "px)"; - document.getElementById("screen").innerHTML = - "

" + item.dataset.screen + "

The " + item.dataset.screen.toLowerCase() + " screen.

"; + // A pressed-item flash before navigating, like a real app. The + // slide freezes with it — so a mid-slide click visibly strands + // the drawer part way out. + clearInterval(slideTimer); + item.style.background = "#e4e4e7"; + setTimeout(() => { + overlay.hidden = true; + drawer.style.transform = "translateX(" + -DRAWER_WIDTH + "px)"; + item.style.background = ""; + document.getElementById("screen").innerHTML = + "

" + item.dataset.screen + "

The " + item.dataset.screen.toLowerCase() + " screen.

"; + }, 700); }); } diff --git a/spec/motion-waiter.spec.ts b/spec/motion-waiter.spec.ts index 3379a64..e5facf4 100644 --- a/spec/motion-waiter.spec.ts +++ b/spec/motion-waiter.spec.ts @@ -13,7 +13,7 @@ const test = base.extend({ }, }); -test("a static element clicks with barely any overhead", async ({ page }) => { +test("a static element clicks after a short stillness check", async ({ page }) => { const start = Date.now(); await page.getByRole("button", { name: "static" }).click(); expect(Date.now() - start).toBeLessThan(700); diff --git a/src/plugins/motion-waiter.ts b/src/plugins/motion-waiter.ts index 517e512..ad664e4 100644 --- a/src/plugins/motion-waiter.ts +++ b/src/plugins/motion-waiter.ts @@ -28,9 +28,12 @@ export type MotionWaiterOptions = { /** Interval between bounding-box samples (ms). Default: 60 */ sampleInterval?: number; /** - * Once motion has been observed, how long the box must hold still before the - * action proceeds (ms) — defeats step cadences slower than the sample - * interval. Default: 150 + * 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 */ @@ -115,10 +118,7 @@ export const motionWaiter = Object.assign( still = null; sawMotion = true; } else if (still !== null && sameBox(still.box, box, settings.epsilon)) { - // A static element passes on its first confirming sample (~one - // interval of overhead); once motion was seen, demand a real - // quiet window. - if (now - still.since >= (sawMotion ? settings.settledFor : 0)) { + if (now - still.since >= settings.settledFor) { if (sawMotion) { settings.log( `${locator}.${method}(...) target settled after ${now - start}ms of motion`, diff --git a/tasks/motion-waiter.md b/tasks/motion-waiter.md index fdb7a13..27cc327 100644 --- a/tasks/motion-waiter.md +++ b/tasks/motion-waiter.md @@ -45,11 +45,14 @@ fix it in middlewright if we care to". This is that fix. 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. -- **Cheap when nothing moves**: one extra sample interval (~60ms) per action. - Only once motion is observed does the plugin require a longer quiet window - (150ms of unchanged box) before proceeding — that defeats step cadences - slower than the sample interval. Cadences slower than `sampleInterval` can - pass the initial check (documented boundary, same as vanilla Playwright). +- **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). - **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 @@ -58,11 +61,13 @@ fix it in middlewright if we care to". This is that fix. ## Checklist -- [x] Repro demo: a slow (1s) timer-driven sliding drawer app + +- [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 −240.8px of a 280px drawer (≈14% open); the app records the - offset at click time_ + 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_ From df2e6d66bd87d6a9df46d903f098fbde1d9bc7ac Mon Sep 17 00:00:00 2001 From: Misha Kaletsky <15040698+mmkal@users.noreply.github.com> Date: Sat, 29 Aug 2026 17:57:21 +0100 Subject: [PATCH 4/8] Task: videos attached to PR #42, all checklist items done Co-Authored-By: Claude Fable 5 --- tasks/motion-waiter.md | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/tasks/motion-waiter.md b/tasks/motion-waiter.md index 27cc327..7b60582 100644 --- a/tasks/motion-waiter.md +++ b/tasks/motion-waiter.md @@ -7,9 +7,8 @@ size: medium ## Status summary -Plugin, demo spec (before/after), unit spec, exports and README all done and -passing. Remaining: attach before/after videos + the todo-app baseline video -to the PR body. +Done pending review: plugin, demo spec (before/after), unit spec, exports, +README, and the PR body carries before/after + todo-app baseline videos. ## Problem @@ -77,14 +76,14 @@ fix it in middlewright if we care to". This is that fix. - [x] Export from `src/plugins/index.ts` / `src/index.ts`, README section _— README gets a motionWaiter section between spinnerWaiter and hydrationWaiter_ -- [ ] PR body: before/after videos (user-attachments URLs) + todo-app baseline - video per AGENTS.md +- [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: 158 passed; `spec/popup-video.spec.ts` failed once under full - parallelism but passes in isolation on this branch AND on main (main's tip - commit is itself a video-flake fix) — pre-existing flake, not this change. -- The marquee unit test steps every 40ms: a 120ms-step marquee sits inside the - documented fast-path boundary (holds longer than `sampleInterval` look - static on the first confirming sample). +- 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). From c114d58dcf903813d2995f03c1fa5cfff33ece3a Mon Sep 17 00:00:00 2001 From: Misha Kaletsky <15040698+mmkal@users.noreply.github.com> Date: Sat, 29 Aug 2026 20:04:18 +0100 Subject: [PATCH 5/8] Review: off by default, opt in per block via settings.run Every guarded action pays the stillness window, so motion checking now defaults to enabled: false. Opt in around known-problematic animations (motionWaiter.settings.run({ enabled: true }, () => item.click())), or pass enabled: true at registration to guard a whole suite. The demo's after-test showcases the per-block opt-in. Co-Authored-By: Claude Fable 5 --- README.md | 23 ++++++++++++++--------- spec/motion-drawer-demo.spec.ts | 8 +++++--- spec/motion-waiter.spec.ts | 9 +++++---- src/plugins/motion-waiter.ts | 23 +++++++++++++++++------ tasks/motion-waiter.md | 5 +++++ 5 files changed, 46 insertions(+), 22 deletions(-) diff --git a/README.md b/README.md index fb435b6..217b937 100644 --- a/README.md +++ b/README.md @@ -143,23 +143,28 @@ test("a test where spinners are expected to hang", async ({ page }) => { 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). 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. +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, // quiet window required once motion was seen + 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: - -```ts -await motionWaiter.settings.run({ disabled: true }, () => - page.getByText("stock ticker").click(), -); -``` +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 diff --git a/spec/motion-drawer-demo.spec.ts b/spec/motion-drawer-demo.spec.ts index 5c9a760..ef5025d 100644 --- a/spec/motion-drawer-demo.spec.ts +++ b/spec/motion-drawer-demo.spec.ts @@ -37,7 +37,7 @@ test("control: without motion-waiter the click lands on a still-sliding drawer", expect(drawerXAtClick).toBeLessThan(-40); }); -test("with motion-waiter the click waits for the drawer to settle", async ({ +test("opting in for the drawer click waits out the slide", async ({ page: basePage, }, testInfo) => { await using page = await addPlugins({ @@ -51,11 +51,13 @@ test("with motion-waiter the click waits for the drawer to settle", async ({ page.getByRole("button", { name: "Open menu" }).click(), ); await page.videoMode.caption("motion-waiter holds the click until the drawer settles", () => - page.getByRole("button", { name: "Notifications" }).click(), + motionWaiter.settings.run({ enabled: true }, () => + page.getByRole("button", { name: "Notifications" }).click(), + ), ); await page.getByRole("heading", { name: "Notifications" }).waitFor(); - // Same app, same clicks — motion-waiter held the click until the slide + // 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)`); diff --git a/spec/motion-waiter.spec.ts b/spec/motion-waiter.spec.ts index e5facf4..e6603bd 100644 --- a/spec/motion-waiter.spec.ts +++ b/spec/motion-waiter.spec.ts @@ -14,6 +14,7 @@ const test = base.extend({ }); 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); @@ -21,6 +22,7 @@ test("a static element clicks after a short stillness check", async ({ page }) = }); 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)); @@ -28,10 +30,7 @@ test("a timer-stepped slide is waited out; the click lands at rest", async ({ pa await page.getByText("clicked sliding at x=200").waitFor(); }); -test("without the plugin the same click lands mid-slide (the gap this plugin closes)", async ({ - page, -}) => { - motionWaiter.settings.enterWith({ disabled: true }); +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); @@ -40,6 +39,7 @@ test("without the plugin the same click lands mid-slide (the gap this plugin clo }); 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(); @@ -51,6 +51,7 @@ test("perpetual motion proceeds once the settle budget runs out", async ({ page 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 }); diff --git a/src/plugins/motion-waiter.ts b/src/plugins/motion-waiter.ts index ad664e4..87730a6 100644 --- a/src/plugins/motion-waiter.ts +++ b/src/plugins/motion-waiter.ts @@ -38,8 +38,14 @@ export type MotionWaiterOptions = { settledFor?: number; /** Movement smaller than this many px counts as still (subpixel jitter). Default: 0.5 */ epsilon?: number; - /** Whether to skip motion checking. Default: false */ - disabled?: boolean; + /** + * 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; }; @@ -54,7 +60,7 @@ const defaults: Required = { sampleInterval: 60, settledFor: 150, epsilon: 0.5, - disabled: false, + enabled: false, log: () => {}, }; @@ -75,7 +81,12 @@ const getSettings = (baseOptions: MotionWaiterOptions = {}) => { * * Runtime settings can be overridden per-test via * `motionWaiter.settings.enterWith({...})`, or for a single call via - * `motionWaiter.settings.run({...}, () => locator.click())`. + * `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 @@ -93,7 +104,7 @@ export const motionWaiter = Object.assign( middleware: async ({ args, locator, method }, next) => { const settings = getSettings(options); - if (settings.disabled || !pointerMethods.has(method)) return next(); + 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 @@ -144,7 +155,7 @@ export const motionWaiter = Object.assign( [ `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({ disabled: true }, () => locator.${method}(...))`, + ` await motionWaiter.settings.run({ enabled: false }, () => locator.${method}(...))`, ], "motion-waiter.ts", ); diff --git a/tasks/motion-waiter.md b/tasks/motion-waiter.md index 7b60582..405a881 100644 --- a/tasks/motion-waiter.md +++ b/tasks/motion-waiter.md @@ -52,6 +52,11 @@ fix it in middlewright if we care to". This is that fix. 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 From d1d1ce5158199360f5b24c73611122b21da66053 Mon Sep 17 00:00:00 2001 From: Misha Kaletsky <15040698+mmkal@users.noreply.github.com> Date: Sat, 29 Aug 2026 21:24:06 +0100 Subject: [PATCH 6/8] Fix the demo-video flashes: watchable spans + fade-aware demo app MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The flash at the start and end of the demo videos had two causes: 1. The motion-settle hold runs before video-mode's middleware, so video-mode recorded it as pre-action dead air and compression fast-forwarded the drawer's slide — frames of the dimmed mid-slide state leaked into the rendered intro as a flash. Fix via neutral middleware context (AGENTS.md plugin boundaries): ActionTiming gains watchableSpans, motion-waiter flags its hold there when it actually saw motion, and video-mode carves those spans out of dead air the same way popup enter/exit animations are protected. The slide now renders at full speed. 2. The demo app popped its backdrop in and out instantly (one-frame dim/undim), and navigated while the overlay was mid-fade, so the final freeze frame ghosted half-faded menu items. The overlay now fades in/out and navigation happens only after the menu is fully gone. New spec proves the carve-out: a slide-settling click must not leave a dead-air span covering the hold (fails without the video-mode change). Co-Authored-By: Claude Fable 5 --- spec/motion-drawer-demo.spec.ts | 31 +++++++++++++++++++++++-------- spec/motion-waiter.spec.ts | 24 +++++++++++++++++++++++- src/plugin-system.ts | 7 +++++++ src/plugins/motion-waiter.ts | 10 +++++++++- src/plugins/video-mode.ts | 30 +++++++++++++++++++++++++++++- 5 files changed, 91 insertions(+), 11 deletions(-) diff --git a/spec/motion-drawer-demo.spec.ts b/spec/motion-drawer-demo.spec.ts index ef5025d..d678e8e 100644 --- a/spec/motion-drawer-demo.spec.ts +++ b/spec/motion-drawer-demo.spec.ts @@ -27,7 +27,8 @@ test("control: without motion-waiter the click lands on a still-sliding drawer", await page.videoMode.caption("Playwright clicks while the drawer is still sliding", () => page.getByRole("button", { name: "Notifications" }).click(), ); - await page.getByRole("heading", { name: "Notifications" }).waitFor(); + // 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 }); // 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 — @@ -43,6 +44,9 @@ test("opting in for the drawer click waits out the slide", async ({ 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()], }); await page.setContent(getDrawerAppHtml()); @@ -55,7 +59,8 @@ test("opting in for the drawer click waits out the slide", async ({ page.getByRole("button", { name: "Notifications" }).click(), ), ); - await page.getByRole("heading", { name: "Notifications" }).waitFor(); + // 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 }); // 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). @@ -75,7 +80,11 @@ function getDrawerAppHtml() { header { display: flex; align-items: center; gap: 12px; padding: 14px 16px; background: #18181b; color: #fafafa; } header button { font-size: 18px; background: none; color: inherit; border: 1px solid #3f3f46; border-radius: 8px; padding: 6px 10px; } main { padding: 24px 16px; } - #overlay { position: fixed; inset: 0; background: rgba(0, 0, 0, 0.45); } + #overlay { + position: fixed; inset: 0; background: rgba(0, 0, 0, 0.45); + opacity: 0; transition: opacity 180ms ease; + } + #overlay.open { opacity: 1; } #drawer { position: fixed; top: 0; bottom: 0; left: 0; width: 280px; background: #ffffff; box-shadow: 4px 0 24px rgba(0, 0, 0, 0.25); @@ -116,6 +125,7 @@ function getDrawerAppHtml() { document.querySelector("header button").addEventListener("click", () => { overlay.hidden = false; + requestAnimationFrame(() => overlay.classList.add("open")); const startedAt = Date.now(); slideTimer = setInterval(() => { const progress = Math.min((Date.now() - startedAt) / SLIDE_MS, 1); @@ -135,11 +145,16 @@ function getDrawerAppHtml() { clearInterval(slideTimer); item.style.background = "#e4e4e7"; setTimeout(() => { - overlay.hidden = true; - drawer.style.transform = "translateX(" + -DRAWER_WIDTH + "px)"; - item.style.background = ""; - document.getElementById("screen").innerHTML = - "

" + item.dataset.screen + "

The " + item.dataset.screen.toLowerCase() + " screen.

"; + overlay.classList.remove("open"); + setTimeout(() => { + overlay.hidden = true; + drawer.style.transform = "translateX(" + -DRAWER_WIDTH + "px)"; + item.style.background = ""; + // Navigate only once the menu is fully gone, so the new + // screen never paints under a half-faded overlay. + document.getElementById("screen").innerHTML = + "

" + item.dataset.screen + "

The " + item.dataset.screen.toLowerCase() + " screen.

"; + }, 200); }, 700); }); } diff --git a/spec/motion-waiter.spec.ts b/spec/motion-waiter.spec.ts index e6603bd..41141ff 100644 --- a/spec/motion-waiter.spec.ts +++ b/spec/motion-waiter.spec.ts @@ -1,5 +1,5 @@ import { test as base, expect } from "@playwright/test"; -import { addPlugins, motionWaiter } from "../src/index.ts"; +import { addPlugins, motionWaiter, videoMode } from "../src/index.ts"; const test = base.extend({ page: async ({ page: basePage }, use, testInfo) => { @@ -59,6 +59,28 @@ test("an explicit timeout passes straight through, like spinner-waiter's escape 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/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/motion-waiter.ts b/src/plugins/motion-waiter.ts index 87730a6..0f8b6b1 100644 --- a/src/plugins/motion-waiter.ts +++ b/src/plugins/motion-waiter.ts @@ -102,7 +102,7 @@ export const motionWaiter = Object.assign( return { name: "motion-waiter", - middleware: async ({ args, locator, method }, next) => { + middleware: async ({ args, locator, method, timing }, next) => { const settings = getSettings(options); if (!settings.enabled || !pointerMethods.has(method)) return next(); @@ -112,7 +112,13 @@ export const motionWaiter = Object.assign( 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; @@ -134,6 +140,7 @@ export const motionWaiter = Object.assign( settings.log( `${locator}.${method}(...) target settled after ${now - start}ms of motion`, ); + flagWatchable(); } return next(); } @@ -147,6 +154,7 @@ export const motionWaiter = Object.assign( settings.log( `${locator}.${method}(...) target still moving after the ${settings.settleTimeout}ms settle budget, proceeding`, ); + if (sawMotion) flagWatchable(); try { return await next(); } catch (error) { diff --git a/src/plugins/video-mode.ts b/src/plugins/video-mode.ts index fc5dd3e..c22091b 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: {}, From caf990ef7a4f06cb9a00acd29d7f995f3bbdeae1 Mon Sep 17 00:00:00 2001 From: Misha Kaletsky <15040698+mmkal@users.noreply.github.com> Date: Sat, 29 Aug 2026 21:56:30 +0100 Subject: [PATCH 7/8] Watchable spans also cancel hold-overlap skips; smooth the demo seams MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The slide vanished from the rendered demo entirely: two pointer actions within one highlight-hold of each other trigger the overlap-skip, which jumped straight across the drawer's travel. Watchable spans now feed renderKeepSpans — the same guard that stops skips leaping over popup enter/exit animations — so flagged footage survives both dead-air compression and hold skips. The remaining end-seam artifacts (a pre-navigate Home frame and a black frameless slice leaking around the heading waitFor's highlight) go away by not highlighting the waitFor at all (skipMethods: ["waitFor"]) — footage runs continuously from the click through the pressed flash and fade. The demo app now navigates as the fade starts (the new screen is what the fade reveals), and the tests outlast the fade so the final hold freezes a settled frame. Verified frame-by-frame at 25fps: both videos play travel → slide → pressed flash → fade → settled screen with no flashes, ghosts, or black frames. Co-Authored-By: Claude Fable 5 --- spec/motion-drawer-demo.spec.ts | 20 ++++++++++++++------ src/plugins/video-mode.ts | 9 +++++++-- 2 files changed, 21 insertions(+), 8 deletions(-) diff --git a/spec/motion-drawer-demo.spec.ts b/spec/motion-drawer-demo.spec.ts index d678e8e..fb6dd0d 100644 --- a/spec/motion-drawer-demo.spec.ts +++ b/spec/motion-drawer-demo.spec.ts @@ -17,7 +17,9 @@ test("control: without motion-waiter the click lands on a still-sliding drawer", await using page = await addPlugins({ page: basePage, testInfo, - plugins: [videoMode()], + // 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()); @@ -29,6 +31,9 @@ test("control: without motion-waiter the click lands on a still-sliding drawer", ); // 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 — @@ -47,7 +52,7 @@ test("opting in for the drawer click waits out the slide", async ({ // 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()], + plugins: [motionWaiter(), videoMode({ skipMethods: ["waitFor"] })], }); await page.setContent(getDrawerAppHtml()); @@ -61,6 +66,9 @@ test("opting in for the drawer click waits out the slide", async ({ ); // 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). @@ -145,15 +153,15 @@ function getDrawerAppHtml() { clearInterval(slideTimer); item.style.background = "#e4e4e7"; setTimeout(() => { + // Navigate as the menu starts fading, so the new screen is + // what the fade reveals. + document.getElementById("screen").innerHTML = + "

" + item.dataset.screen + "

The " + item.dataset.screen.toLowerCase() + " screen.

"; overlay.classList.remove("open"); setTimeout(() => { overlay.hidden = true; drawer.style.transform = "translateX(" + -DRAWER_WIDTH + "px)"; item.style.background = ""; - // Navigate only once the menu is fully gone, so the new - // screen never paints under a half-faded overlay. - document.getElementById("screen").innerHTML = - "

" + item.dataset.screen + "

The " + item.dataset.screen.toLowerCase() + " screen.

"; }, 200); }, 700); }); diff --git a/src/plugins/video-mode.ts b/src/plugins/video-mode.ts index c22091b..059366e 100644 --- a/src/plugins/video-mode.ts +++ b/src/plugins/video-mode.ts @@ -5554,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, From 4f004fdbb4b9532db991d2a72c8ecd8e962855af Mon Sep 17 00:00:00 2001 From: Misha Kaletsky <15040698+mmkal@users.noreply.github.com> Date: Sat, 29 Aug 2026 22:44:45 +0100 Subject: [PATCH 8/8] Lint hints: point animation-shaped timeout code at motionWaiter Agents reaching for a sleep or a timeout to let a drawer finish sliding now get told about the tool built for it, in both require-timeout-comment messages. The lint-plugin spec pins the hint. Co-Authored-By: Claude Fable 5 --- spec/lint-plugin.spec.ts | 2 ++ src/lint/plugin.ts | 2 ++ 2 files changed, 4 insertions(+) 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/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}}.