Popup support: wrap popup pages with addPlugins - #32
Closed
mmkal wants to merge 4 commits into
Closed
Conversation
…act gap Investigation findings: middleware plugins already support popups via a second addPlugins call (per-page dispatch with fall-through). videoMode needs work: a reused instance wipes the main timeline on beforeTest, and a fresh instance per popup collides on fixed artifact filenames in testInfo.outputDir. Spec for the intended behavior comes next. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Two tests against an auth-popup demo app. The first passes today: wrapping the popup with a second addPlugins call runs its actions through its own plugins, with timelines isolated in-memory. The second is an intended-behavior spec that fails today: after both pages finalize, each videoMode instance should own its artifacts, but both resolve the same fixed filenames in testInfo.outputDir so the last finalize clobbers the first. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
commit: |
Each videoMode() registered within a test now gets its own artifact namespace: the first instance keeps the legacy unsuffixed filenames, later ones get -2, -3, ... on metadata, raw/rendered video, player HTML, highlight/pan/fill images, dialog/final frames, .ass files, and attachment names. This lets an auth popup carry its own fresh videoMode instance without clobbering the main page's artifacts - Playwright screencasts each page separately, so separate videos per page is the natural model. Reusing one active instance on a second page now throws a clear error instead of silently wiping the first page's timeline in beforeTest. Specs: popup.spec.ts (artifact isolation now green, plus the reuse guard), popup-video.spec.ts (video: 'on' end to end - popup gets its own raw/rendered webm), shared demo app in auth-demo-app.ts. README gains a Popups section. Task moved to complete. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Used by the gitignored popup-demo spec that records the PR demo videos; the popup specs assert the same roles/text either way. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
mmkal
added a commit
that referenced
this pull request
Aug 13, 2026
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>
Contributor
Author
|
🤖 Superseded by #33, which will target main directly and contains all of this branch's commits — closing to skip the intermediate merge step. |
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>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Lets tests wrap auth-style popout windows with the same plugins as the main page:
addPluginscall on the popup was always the designed seam — now specced.videoMode()registered within a test gets its own filenames: the first keeps the existing unsuffixed names (fully back-compatible), later ones get-2,-3, … (video-rendered-2.webm,video-mode-2.json, highlight images,.assfiles, attachment names). Before, both instances wrote to the same fixed paths and whichever page finalized last silently clobbered the other. No explicit naming required — instances are indexed by registration order.videoMode()instance on a second page now throws (create a fresh videoMode() instance for each page) instead of silently wiping the first page's timeline.Specs:
spec/popup.spec.ts(middleware on popups, artifact isolation, reuse guard),spec/popup-video.spec.ts(video: "on"end to end — the popup produces its own raw + rendered webm alongside the main page's). README gains a Popups section. Full suite: 134 passed.Demo
One test, two rendered videos — Playwright screencasts each page separately, and each
videoMode()instance renders its own.Main page (
video-rendered.webm) — opens the popup, then waits for the signed-in state:popup-demo-main-page.mp4
Popup (
video-rendered-2.webm) — the approve click, recorded by the popup's own instance:popup-demo-popup.mp4
🤖 Generated with Claude Code
Session:
b7f6f792-6606-44be-9ec3-207eb762c4b6