Skip to content

Auto-wrap popups and composite their video as an overlay - #33

Closed
mmkal wants to merge 9 commits into
popup-pluginsfrom
popup-overlay
Closed

Auto-wrap popups and composite their video as an overlay#33
mmkal wants to merge 9 commits into
popup-pluginsfrom
popup-overlay

Conversation

@mmkal

@mmkal mmkal commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

Stacked on #32 (popup-plugins). Two halves, from the approved design in tasks/complete/2026-08-13-popup-overlay-video.md:

  1. Auto-wrap: pages wrapped with addPlugins automatically wrap popups they open (recursively) — spinner-waiting, error reporting etc. apply to the popup with zero test wiring. Plugins control their popup behavior via a forPopup(ctx) hook; opt out wholesale with popups: false. Wrapping an already-wrapped page now throws.
  2. Overlay video: instead of a separate -2 video, the popup renders as an overlay in the main page's video — it slides up from the bottom over the dimmed page, popup clicks and typed fill reveals are annotated inside it (focus ring included), one cursor glides between page and popup, and it slides away on close. Implemented as a two-pass render: a composite pass over the raw screencasts, then the existing annotation machinery unchanged. Popup facts land in video-mode.json under children (schema v2).
// test code stays completely ordinary:
const popupPromise = page.waitForEvent("popup");
await page.getByRole("button", { name: "Sign in" }).click();
const popup = await popupPromise; // already wrapped — no addPlugins call
await popup.getByLabel("Username").fill("mmkal");
await popup.getByRole("button", { name: "Sign in" }).click();

Demo

One composed video from spec/popup-overlay-demo.spec.ts — the sign-in popup slides up over the dimmed dashboard, the cursor types the username and password into their focused fields, clicks Sign in, the popup slides away, and the dashboard shows the signed-in state:

popup-signin-overlay-demo.mp4

Deferred (noted in the task file): popup dialog annotations; child pans degrade to plain box highlights inside the overlay.

Full suite: 142 passed. The popups: false path keeps the #32 standalone-video behavior, covered by the migrated specs.

🤖 Generated with Claude Code

Session: b7f6f792-6606-44be-9ec3-207eb762c4b6

Design settled interactively (plannotator grill): parent-owned composition,
default-on auto-wrap with a forPopup plugin hook, unified piece timeline
with source tags, 90%-fit dimmed overlay, double-wrap error instead of an
escape hatch. Full decision log in the task file.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@pkg-pr-new

pkg-pr-new Bot commented Aug 13, 2026

Copy link
Copy Markdown

Open in StackBlitz

pnpm add https://pkg.pr.new/middlewright@33

commit: e6a7948

mmkal and others added 8 commits August 13, 2026 13:35
Wrapped pages listen for the popup event and wrap the popup
automatically (recursively for nested popups). Plugins may declare
forPopup(ctx) to produce the popup's plugin - videoMode returns null for
now (its parent-bound child recorder is phase 2) - and plugins without
the hook are re-registered as-is, which is safe for the stateless ones.
Opt out with popups: false; children dispose before the parent
finalizes.

addPlugins on an already-wrapped page now throws instead of silently
replacing plugin state mid-flight - this also guards accidental double
wrapping of main pages. Manual popup wrapping (the #32 pattern) becomes
the popups: false path; specs migrated accordingly.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
videoMode.forPopup now returns a parent-bound child recorder: popup
actions record highlights on the parent clock (so child->parent time
mapping is trivial), waits merge into the parent's unified dead air, and
at finalize the popup recorder settles, closes the popup, calibrates
recordingEndedAt, and copies the popup's raw screencast to
video-raw-popup-N.webm. Facts land in a new children array in
video-mode.json (schemaVersion 2). Grandchild popups recurse into the
same flat children list. Rendering is untouched - the composite overlay
is phase 3.

The action middleware is extracted into videoModeActionMiddleware,
shared between an instance (recording onto its own state) and its child
recorders (recording onto a child state that shares the parent's clock
and dead-air spans).

Popup dialogs are not annotated yet - noted in the task file.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Two-pass render when a test recorded popups. Pass A composites each
popup screencast onto the parent's raw footage: dimmed backdrop (40%
black), scaled to fit 90% of the frame (never upscaled), centered,
alpha-faded in/out over 200ms, windowed to the popup's open/close span,
stacked newest-on-top. The composite shares the parent raw timeline
exactly, so pass B - the existing piece/hold/dead-air/cursor machinery -
runs on it unchanged; holds freeze the composite, so it never matters
which source triggered them.

Popup highlights project into composite coordinates and join the render
plan as plain box/pointer highlights (child-frame pixel treatments -
pans, fill reveals, screenshot stills - drop away). Child raw time maps
to the parent timeline via the settled-recorder calibration when the
popup was closed by the recorder, or the screencast start approximation
when the popup closed itself; the auth demo popup now self-closes after
Approve like a real OAuth popup.

Frame-sampling spec verifies the rendered output: frames with a dimmed
page background and the popup's bright card centered above it.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…spec

Three compositing fixes found by eyeballing the rendered demo:

- Static pages emit sparse screencast frames, and overlay emits output
  only at primary-input frame times. Resample BOTH chains to a
  continuous fps: without it the child frame that passed fade mid-ramp
  ghosts at partial alpha for the whole window, and the popup window can
  contain zero composite frames entirely (showing post-close footage).
- Run the exit fade AFTER close, using the screencast's padded final
  frame: a self-closing popup otherwise puts its own Approve click - and
  the click hold's freeze frame - inside the fade by construction.
- An instant click's source slice is a few ms wide, often between frame
  ticks, so the hold trim came up empty and bled the next piece's
  footage. Anchor the slice back from close and widen it to two frames.

spec/popup-overlay-demo.spec.ts replaces the gitignored demo spec: full
watchable treatment (pointer, captions, overlay), light assertions on
the child span and rendered output. Overlay frame assertions calibrated
against measured downscale blends.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The popup overlay now slides up from the bottom edge on enter (300ms,
quadratic ease-out) and slides back down after close, alongside the
alpha fade. Piece planning gains keepSpans: the overlapping-hold skip
used to jump source footage straight across the popup's enter/exit
animations, hard-cutting them out of the output; skips that would cross
a popup transition are cancelled. Parent highlights that start inside a
popup's exit window shift past it, so their holds can't freeze a
mid-fade ghost (the flash after the popup disappeared).

Fill reveals now work inside overlays: the typed-text reveal renders
over a frozen composite frame (popup risen, field empty, backdrop
intact) with the child screenshot's content rect scaled and positioned
through the overlay transform, synced to cursor arrival like main-page
fills. highlightCursorPoint projects fillReveal.initialRect through the
overlay transform too - the I-beam used to land offset from the field.

Child raw footage anchoring: a self-closing popup's screencast t=0 is
its first captured frame, which lags the popup event by the initial
paint, so footage played early (fields filled before the reveal). The
raw video's padded end is the better anchor: closedAt + 1s minimum
final-frame padding - duration, floored at openedAt.

Demo: the popup is now a realistic sign-in form (username/password +
Sign in) on indigo, over a teal app page so the dimmed backdrop reads;
MIDDLEWRIGHT_DEBUG_PIECES=1 dumps the render piece plan.

Includes previously staged demo/styling work reviewed via the local
video (not the GitHub PR).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The reveal base predates the fill, so the field typed with no focus
ring, and the ring then flashed in from live footage after the hold.
At reveal start the field's ring region from the post-fill screenshot
overlays the base, its text immediately covered by the pre-fill
screenshot's empty content box, and the reveal bands type over that -
ring appears when the cursor lands, letters arrive inside it, and the
post-piece footage continues the ring seamlessly.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Password inputs render one bullet per character, and the reveal only
ever shows the screenshot's dots - so there was no reason to fall back
to an instant fill. Measure bullet glyphs instead of the value's
graphemes for the reveal stops.

The reveal base now rewinds a single frame (a longer rewind could cross
the previous fill's completion and wipe its value from the frozen
frame), and the pre-fill empty content box covers the field from t=0 so
anchor imprecision can't leak early-typed text.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
mmkal added a commit to iterate/iterate that referenced this pull request Aug 14, 2026
…meout hacks

middlewright PR 33 auto-wraps popups opened by wrapped pages, so the
spinner/hydration waiters cover the OAuth popup with zero test wiring. The
chat-titles spec drops its two popup-action timeouts (which existed only
because the popup was unwrapped); the timeouts justified by the auth app's
missing loading UI stay. Dependency is sha-pinned to the PR build
(pkg.pr.new/middlewright@eb34bdc) — swap to the npm release once
iterate/middlewright#33 lands.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
mmkal added a commit to iterate/iterate that referenced this pull request Aug 14, 2026
Two spec-only changes to `specs/mobile/chat-titles.spec.ts`
([#2490](#2490)); no product
changes.

### 1. Drop the picker tap the app already skips

The spec failed every preview e2e run it appeared in (seen twice +
retries on [#2498](#2498)):

```
strict mode violation: getByText('mobile-chat-titles-…') resolved to 2 elements:
  1) <h1 …>mobile-chat-titles-…</h1>            ← chat list header
  2) <div …>→ /notes in mobile-chat-titles-…</div> ← note composer caption
```

Test identities auto-continue project selection and the app auto-opens
the account's only project, so the spec's
`page.getByText(projectSlug).click()` picker tap ran when the chat list
was already up — where the slug appears twice. Deterministic, not flake.
Fix: align the signup helper with `notes.spec.ts`'s proven flow —
consent's "Allow access", then wait for "New chat".

### 2. middlewright#33: popups auto-wrap — popup timeout hacks removed


[iterate/middlewright#33](iterate/middlewright#33)
auto-wraps popups opened by wrapped pages, so the spinner/hydration
waiters now cover the OAuth popup with zero test wiring. The spec drops
its two popup-action `{ timeout: 15_000 }` escapes (which existed only
because the popup was unwrapped). Timeouts justified by the auth app's
missing loading UI are unchanged.

**Note for merge:** `middlewright` is sha-pinned to the PR build
(`https://pkg.pr.new/middlewright@eb34bdc`). Swap to the npm release
once middlewright#33 lands — or merge as-is and bump later; the sha URL
is immutable.

Verified locally: 2/2 passes of the spec against a local dev server with
the wrapped popup.

### 3. The real fix for the popup slowness: auth app loading states

The popup timeouts all pointed at one product gap — the auth app goes
loading-UI-less exactly where it's slowest. On a cold preview, "Allow
access" is >1s away while the consent skeleton shows unmarked gray
boxes, and the redirect transitions flip their pending labels
("Authorizing...", "Signing in...") back to idle text mid-navigation.
Fixed in the auth app:

- Both loading skeletons (consent, project-access) are now
`role="status" aria-label="Loading"` — announced to assistive tech, and
the e2e spinner-waiter extends its budget while they show.
- `redirectAndStayPending` is now shared: consent and OTP sign-in keep
their pending labels showing until the browser actually leaves ("Get
started" already did this).
- `email-otp-signup.ts` drops every manual timeout and its
disabled-spinner-waiter block — the signup flow rides real product
loading UI end to end.

middlewright is repinned to the
[#34](iterate/middlewright#34) build
(`pkg.pr.new/middlewright@2f47229`, stacked on #33): explicit action
timeouts pass through spinner-waiter (a genuine middlewright gap, kept),
and the readiness-goal fix carried as `patches/middlewright.patch` is
upstreamed — the patch is deleted here.

### Risk map

- Riskiest: `redirectAndStayPending` in consent/OTP sign-in — a
never-resolving promise held until unload. Same pattern project-access
has shipped for a while; the mutation can no longer "succeed" visibly,
which is the point.
- Behavior change on merge: sign-in flows show persistent pending labels
through redirects (previously flickered back to idle); loading skeletons
are announced as status to screen readers.
- Review order: `apps/auth/src/routes/_auth/consent.tsx` + `login.tsx`
(redirect semantics), `project-access.tsx` (skeleton mark + helper
extraction), `specs/test-support/email-otp-signup.ts` (timeout escapes
deleted), `chat-titles.spec.ts`, generated/lockfile churn last.

### Video

The spec end-to-end — including middlewright#33's popup overlay
compositing (the OAuth popup renders as an overlay in the main page's
video):


https://github.com/user-attachments/assets/36135e5c-3a96-466b-83b9-0a426705910c

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Session id: `7b51d95e-5871-4a8f-b4c9-996a84e1beea`

<!-- CURSOR_SUMMARY -->
---

> [!NOTE]
> **Medium Risk**
> Changes OAuth consent/login/project flows and pins middlewright to a
PR tarball; behavior is UX-only but on the authentication path and
affects CI e2e timing.
> 
> **Overview**
> Keeps auth OAuth/login flows visibly loading until navigation actually
starts, and updates Playwright helpers to rely on **middlewright**
instead of manual timeouts.
> 
> **Auth app:** Introduces shared `redirectAndStayPending` so
react-query mutations stay pending after `window.location` is
set—buttons keep **"Authorizing..."**, **"Signing in..."**, and
**"Creating..."** and stay disabled through slow server round trips
(consent, login OTP success, project-access continue/create paths).
Loading skeletons on consent and project-access now use `role="status"`
and `aria-label="Loading"` so assistive tech and the e2e spinner-waiter
can extend waits.
> 
> **Tooling:** Replaces the local `middlewright` patch with
`https://pkg.pr.new/middlewright@2f47229` and removes
`patches/middlewright.patch` (upstream spinner-waiter fixes). Specs drop
redundant popup timeouts and the disabled spinner-waiter block in email
OTP signup; **chat-titles** stops tapping the project slug on the chat
list (aligns with auto-open + avoids strict-mode duplicate slug
matches).
> 
> <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit
68f46b3. Bugbot is set up for automated
code reviews on this repo. Configure
[here](https://www.cursor.com/dashboard/bugbot).</sup>
<!-- /CURSOR_SUMMARY -->

<!-- CLOUDFLARE_PREVIEW -->
## Environment Config Lease [Login
↗](https://auth.iterate-preview-8.com/test-login?email=pr2500%2Btest%40nustom.com&project=pr2500&return_to=https%3A%2F%2Fos.iterate-preview-8.com%2Fapi%2Fiterate-auth%2Flogin)

<!-- CLOUDFLARE_PREVIEW_STATE -->
<!--
{
  "apps": {
    "os": {
      "appDisplayName": "OS",
      "appSlug": "os",
      "status": "deployed",
      "updatedAt": "2026-08-14T12:21:20.488Z",
      "deployedAt": "2026-08-14T12:17:17.737Z",
      "headSha": "68f46b3fc30b3ecca749de6aa1636512cf85dd70",
      "message": null,
      "publicUrl": "https://os.iterate-preview-8.com",
"runUrl":
"https://github.com/iterate/iterate/actions/runs/274798385393991",
      "shortSha": "68f46b3",
      "deployDurationMs": 60928,
      "deployConfigDurationMs": 2,
      "deployCommandDurationMs": 58862,
      "deployReadinessDurationMs": 2062,
      "deployedWorkerName": "os-preview-8",
      "deployedWorkerVersion": "80dbc953-a6ea-48f7-9a3f-095a5cb31f61",
      "testDurationMs": 235458,
      "testRetries": null,
      "workerSizeKib": 20782.88,
      "workerGzipKib": 5231.5,
      "deployedFingerprint": null,
      "mainWorkerGzipKib": 5242.34
    },
    "docs": {
      "appDisplayName": "Docs",
      "appSlug": "docs",
      "status": "deployed",
      "updatedAt": "2026-08-14T12:17:27.225Z",
      "deployedAt": "2026-08-14T12:16:31.864Z",
      "headSha": "68f46b3fc30b3ecca749de6aa1636512cf85dd70",
      "message": null,
"publicUrl": "https://docs-preview-8.iterate-dev-preview.workers.dev",
"runUrl":
"https://github.com/iterate/iterate/actions/runs/274798385393991",
      "shortSha": "68f46b3",
      "deployDurationMs": 13217,
      "deployConfigDurationMs": 3,
      "deployCommandDurationMs": 12987,
      "deployReadinessDurationMs": 226,
      "deployedWorkerName": "docs-preview-8",
      "deployedWorkerVersion": "926de3ad-a003-4153-851d-12ef25bdb654",
      "testDurationMs": 2195,
      "testRetries": null,
      "workerSizeKib": 3637.46,
      "workerGzipKib": 866.22,
      "deployedFingerprint": null,
      "mainWorkerGzipKib": null
    },
    "semaphore": {
      "appDisplayName": "Semaphore",
      "appSlug": "semaphore",
      "status": "deployed",
      "updatedAt": "2026-08-14T12:18:26.478Z",
      "deployedAt": "2026-08-14T12:16:34.827Z",
      "headSha": "68f46b3fc30b3ecca749de6aa1636512cf85dd70",
      "message": null,
      "publicUrl": "https://semaphore.iterate-preview-8.com",
"runUrl":
"https://github.com/iterate/iterate/actions/runs/274798385393991",
      "shortSha": "68f46b3",
      "deployDurationMs": 16135,
      "deployConfigDurationMs": 4,
      "deployCommandDurationMs": 15949,
      "deployReadinessDurationMs": 182,
      "deployedWorkerName": "semaphore-preview-8",
      "deployedWorkerVersion": "df07fe49-0679-4386-b26c-78810794b776",
      "testDurationMs": 61448,
      "testRetries": null,
      "workerSizeKib": 1915.87,
      "workerGzipKib": 441.25,
      "deployedFingerprint": null,
      "mainWorkerGzipKib": null
    },
    "auth": {
      "appDisplayName": "Auth",
      "appSlug": "auth",
      "status": "deployed",
      "updatedAt": "2026-08-14T12:17:43.196Z",
      "deployedAt": "2026-08-14T12:16:43.501Z",
      "headSha": "68f46b3fc30b3ecca749de6aa1636512cf85dd70",
      "message": null,
      "publicUrl": "https://auth.iterate-preview-8.com",
"runUrl":
"https://github.com/iterate/iterate/actions/runs/274798385393991",
      "shortSha": "68f46b3",
      "deployDurationMs": 24895,
      "deployConfigDurationMs": 5,
      "deployCommandDurationMs": 24622,
      "deployReadinessDurationMs": 268,
      "deployedWorkerName": "auth-preview-8",
      "deployedWorkerVersion": "5db76562-bdc8-4862-a2dd-af4ad64751b5",
      "testDurationMs": 18166,
      "testRetries": null,
      "workerSizeKib": 3989.97,
      "workerGzipKib": 817.41,
      "deployedFingerprint": null,
      "mainWorkerGzipKib": null
    },
    "streams-example-app": {
      "appDisplayName": "Streams Example App",
      "appSlug": "streams-example-app",
      "status": "deployed",
      "updatedAt": "2026-08-14T12:18:57.867Z",
      "deployedAt": "2026-08-14T12:16:34.587Z",
      "headSha": "68f46b3fc30b3ecca749de6aa1636512cf85dd70",
      "message": null,
      "publicUrl": "https://streams.iterate-preview-8.com",
"runUrl":
"https://github.com/iterate/iterate/actions/runs/274798385393991",
      "shortSha": "68f46b3",
      "deployDurationMs": 37553,
      "deployConfigDurationMs": 5,
      "deployCommandDurationMs": 15707,
      "deployReadinessDurationMs": 21841,
      "deployedWorkerName": "streams-example-app-preview-8",
      "deployedWorkerVersion": "7ad3af65-b802-440f-92d6-fdd3f4eaccc8",
      "testDurationMs": 92834,
      "testRetries": null,
      "workerSizeKib": 5529.86,
      "workerGzipKib": 1562.85,
      "deployedFingerprint": null,
      "mainWorkerGzipKib": null
    },
    "dummy-petshop": {
      "appDisplayName": "Dummy Petshop",
      "appSlug": "dummy-petshop",
      "status": "deployed",
      "updatedAt": "2026-08-14T12:18:14.007Z",
      "deployedAt": "2026-08-14T12:16:37.899Z",
      "headSha": "68f46b3fc30b3ecca749de6aa1636512cf85dd70",
      "message": null,
      "publicUrl": "https://dummy-petshop.iterate-preview-8.com",
"runUrl":
"https://github.com/iterate/iterate/actions/runs/274798385393991",
      "shortSha": "68f46b3",
      "deployDurationMs": 6014,
      "deployConfigDurationMs": 0,
      "deployCommandDurationMs": 5805,
      "deployReadinessDurationMs": 206,
      "deployedWorkerName": "dummy-petshop-preview-8",
      "deployedWorkerVersion": "6144d66f-98b6-496c-9845-9abcfcbfb19f",
      "testDurationMs": 48975,
      "testRetries": null,
      "workerSizeKib": 1115.4,
      "workerGzipKib": 198.29,
"deployedFingerprint":
"b717e505c46472150438e97f994eb4a2c283e0f8+bfab28a2c1794a35118c88bad09eba3f5105f3d7",
      "mainWorkerGzipKib": null
    }
  },
  "environmentConfigLease": {
    "dopplerConfig": "preview_8",
    "slug": "preview-8"
  },
  "notice": null
}
-->
<!-- /CLOUDFLARE_PREVIEW_STATE -->

<details>
<summary>Slot: preview-8 | Doppler config: preview_8</summary>

| app | status | commit | preview | size (gzip) | deploy duration | test
duration | retries | cleanup duration | workflow run | updated | summary
|
| --- | --- | --- | --- | --- | --- | --- | --- | --- | --- | --- | ---
|
| Auth | deployed | `68f46b3` |
[https://auth.iterate-preview-8.com](https://auth.iterate-preview-8.com)
| 817.4 KiB | 24.9s | 18.2s | | | [Workflow
run](https://github.com/iterate/iterate/actions/runs/274798385393991) |
2026-08-14T12:17:43.196Z | |
| Docs | deployed | `68f46b3` |
[https://docs-preview-8.iterate-dev-preview.workers.dev](https://docs-preview-8.iterate-dev-preview.workers.dev)
| 866.2 KiB | 13.2s | 2.2s | | | [Workflow
run](https://github.com/iterate/iterate/actions/runs/274798385393991) |
2026-08-14T12:17:27.225Z | |
| Dummy Petshop | deployed | `68f46b3` |
[https://dummy-petshop.iterate-preview-8.com](https://dummy-petshop.iterate-preview-8.com)
| 198.3 KiB | 6.0s | 49.0s | | | [Workflow
run](https://github.com/iterate/iterate/actions/runs/274798385393991) |
2026-08-14T12:18:14.007Z | |
| OS | deployed | `68f46b3` |
[https://os.iterate-preview-8.com](https://os.iterate-preview-8.com) |
5.11 MiB (-10.8 KiB vs main) | 60.9s | 235.5s | | | [Workflow
run](https://github.com/iterate/iterate/actions/runs/274798385393991) |
2026-08-14T12:21:20.488Z | |
| Semaphore | deployed | `68f46b3` |
[https://semaphore.iterate-preview-8.com](https://semaphore.iterate-preview-8.com)
| 441.3 KiB | 16.1s | 61.4s | | | [Workflow
run](https://github.com/iterate/iterate/actions/runs/274798385393991) |
2026-08-14T12:18:26.478Z | |
| Streams Example App | deployed | `68f46b3` |
[https://streams.iterate-preview-8.com](https://streams.iterate-preview-8.com)
| 1.53 MiB | 37.6s | 92.8s | | | [Workflow
run](https://github.com/iterate/iterate/actions/runs/274798385393991) |
2026-08-14T12:18:57.867Z | |

</details>
<!-- /CLOUDFLARE_PREVIEW -->

<!-- loc-report -->
| Group | Lines | Significant |
| --- | ---: | ---: |
| Product | +39 -20 | +17 -11 🟩⬜⬜⬜⬜ |
| Tests | +5 -4 | +2 -2 🟩⬜⬜⬜⬜ |
| Config | +1 -2 | +1 -2 🟥⬜⬜⬜⬜ |
| Other | +13 -241 | +7 -228 🟥🟥🟥🟥⬜ |
| Generated | +56 -280 | +56 -264 🟩🟥🟥🟥🟥 |
| **Total** | **+114 -547** | **+83 -507** 🟩🟥🟥🟥🟥 |

<sub>Lines counts every changed line; Significant ignores blank lines,
JS comments, and TypeScript lines with no runtime output. Between
d90becf and 68f46b3, bucketed first-match-wins into the groups defined
in `scripts/ci/loc-report.ts`.</sub>
<!-- /loc-report -->

---------

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
@mmkal
mmkal deleted the branch popup-plugins August 14, 2026 13:05
@mmkal mmkal closed this Aug 14, 2026
@mmkal

mmkal commented Aug 14, 2026

Copy link
Copy Markdown
Contributor Author

🤖 Replaced by #36 (same branch, targeting main directly) — GitHub's stack tooling wouldn't allow retargeting this PR's base after #32 closed.

mmkal added a commit that referenced this pull request Aug 14, 2026
Replaces #33 (stack tooling wouldn't let its base move off the
now-closed #32). Complete popup support, from the approved design in
`tasks/complete/2026-08-13-popup-overlay-video.md` — includes what was
#32 (popup wrapping + per-instance videoMode artifact namespacing) plus:

1. **Auto-wrap**: pages wrapped with `addPlugins` automatically wrap
popups they open (recursively) — spinner-waiting, error reporting etc.
apply to the popup with zero test wiring. Plugins control their popup
behavior via a `forPopup(ctx)` hook; opt out wholesale with `popups:
false`. Wrapping an already-wrapped page now throws.
2. **Overlay video**: instead of a separate `-2` video, the popup
renders as an overlay **in the main page's video** — it slides up from
the bottom over the dimmed page, popup clicks and typed fill reveals are
annotated inside it (focus ring included), one cursor glides between
page and popup, and it slides away on close. Implemented as a two-pass
render: a composite pass over the raw screencasts, then the existing
annotation machinery unchanged. Popup facts land in `video-mode.json`
under `children` (schema v2).

```ts
// test code stays completely ordinary:
const popupPromise = page.waitForEvent("popup");
await page.getByRole("button", { name: "Sign in" }).click();
const popup = await popupPromise; // already wrapped — no addPlugins call
await popup.getByLabel("Username").fill("mmkal");
await popup.getByRole("button", { name: "Sign in" }).click();
```

## Demo

One composed video from `spec/popup-overlay-demo.spec.ts` — the sign-in
popup slides up over the dimmed dashboard, the cursor types the username
and password into their focused fields, clicks Sign in, the popup slides
away, and the dashboard shows the signed-in state:


https://github.com/user-attachments/assets/2e5fc8eb-b313-4cf8-8c71-f45364a07617

Deferred (noted in the task file): popup dialog annotations; child pans
degrade to plain box highlights inside the overlay.

Full suite: 142 passed. The `popups: false` path keeps the #32
standalone-video behavior, covered by the migrated specs.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Session: `b7f6f792-6606-44be-9ec3-207eb762c4b6`

---------

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
mmkal added a commit that referenced this pull request Aug 14, 2026
Follow-up to #36 (`popup-overlay`) — its popup auto-wrap exposed the
explicit-timeout bug fixed here.

### 1. Explicit timeouts are the author’s budget — stop overriding them
with the 1ms fast-fail

`iterate/iterate`’s preview e2e went fully red on the first run with
#33’s build: every mobile spec’s OAuth popup action failed with
`TimeoutError: locator.click: Timeout 1ms exceeded`. Those specs pass `{
timeout: 15_000 }` on popup actions precisely because the auth pages
render no spinner-visible loading UI — and popups used to be raw pages
where that timeout was honored. Once popups auto-wrapped,
spinner-waiter’s no-spinner fast-fail replaced the author’s 15s budget
with 1ms:

```ts
await popup.getByRole("button", { name: "Allow access" }).click({ timeout: 15_000 });
// → TimeoutError: locator.click: Timeout 1ms exceeded.
```

Now an explicitly passed `timeout` passes straight through — the
per-action equivalent of `settings.run({ disabled: true })`.

### 2. Leave disappearance waits to Playwright

`waitFor({ state: "detached" | "hidden" })` drives toward the target
leaving the page. Spinner-waiter’s appear-oriented model does not apply,
so these waits now pass through unchanged: satisfied waits resolve
normally and failures use Playwright’s configured action timeout rather
than the 1ms fast-fail.

### Risk map

- Explicit-timeout actions skip all spinner-waiter behavior. That is
deliberate: the author owns the supplied budget.
- Disappearance waits use vanilla Playwright behavior rather than
spinner detection.
- Review order: `src/plugins/spinner-waiter.ts`, then the three
regression specs in `spec/spinner-waiter.spec.ts`.

Spinner-waiter suite green locally (13 passed).

### Video


https://github.com/user-attachments/assets/24ccb234-7927-4cc8-a2d7-3f9e51debfe9

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Session id: `7b51d95e-5871-4a8f-b4c9-996a84e1beea`

<!-- CURSOR_SUMMARY -->
---

> [!NOTE]
> **Medium Risk**
> Middleware pass-through changes when explicit timeouts are set skip
all spinner extension for those actions; disappearance waits bypass
fast-fail—behavioral change in a widely used plugin path but aligned
with author intent and downstream iterate usage.
> 
> **Overview**
> **Spinner-waiter** no longer overrides actions when the test author
sets an explicit `{ timeout }` or uses `waitFor({ state: "hidden" |
"detached" })`.
> 
> For **explicit timeouts**, middleware now passes straight to
Playwright instead of applying the no-spinner **1ms fast-fail**. That
restores per-action budgets (e.g. OAuth popup clicks with `{ timeout:
15_000 }`) that broke once popup auto-wrap routed those actions through
spinner-waiter.
> 
> For **disappearance waits**, the appear-oriented spinner logic is
skipped so those calls get normal Playwright timeouts—satisfied waits
resolve, failures use the configured action timeout rather than 1ms.
> 
> `spec/spinner-waiter.spec.ts` adds three tests covering honored
explicit timeout, exceeded explicit timeout messaging, and disappearance
wait pass-through.
> 
> <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit
be26f5a. Bugbot is set up for automated
code reviews on this repo. Configure
[here](https://www.cursor.com/dashboard/bugbot).</sup>
<!-- /CURSOR_SUMMARY -->

---------

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant