Skip to content

Popup support: wrap popup pages with addPlugins - #32

Closed
mmkal wants to merge 4 commits into
mainfrom
popup-plugins
Closed

Popup support: wrap popup pages with addPlugins#32
mmkal wants to merge 4 commits into
mainfrom
popup-plugins

Conversation

@mmkal

@mmkal mmkal commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

Lets tests wrap auth-style popout windows with the same plugins as the main page:

const popupPromise = page.waitForEvent("popup");
await page.getByRole("button", { name: "Sign in" }).click();
await using popup = await addPlugins({
  page: await popupPromise,
  testInfo,
  plugins: [spinnerWaiter(), videoMode()], // fresh instances for the popup
});
await popup.getByRole("button", { name: "Approve" }).click();
  • Middleware plugins needed no code change. Locator dispatch is per-page with fall-through for unwrapped pages, so a second addPlugins call on the popup was always the designed seam — now specced.
  • Video mode artifacts are namespaced per instance. Each 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, .ass files, 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.
  • Reusing one active 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

mmkal and others added 2 commits August 13, 2026 12:34
…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>
@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@32

commit: 7fe19b3

mmkal and others added 2 commits August 13, 2026 12:47
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>
@mmkal

mmkal commented Aug 14, 2026

Copy link
Copy Markdown
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 mmkal closed this Aug 14, 2026
@mmkal
mmkal deleted the popup-plugins branch August 14, 2026 13:05
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>
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