chore(claude): add fxa-pr-screenshots skill

[vpomerleau]

Because

• UI and email template PRs often ship without screenshots because locating the relevant Storybook stories and capturing them is fiddly.
• A skill should handle the whole pipeline — detection, Storybook startup, automated capture — so the user only reviews the output.

This pull request

• Adds .claude/skills/fxa-pr-screenshots/SKILL.md — a Claude Code skill with two modes:
  • Diff-driven (no argument; used by /fxa-pr-open): captures stories for UI/email files changed vs main. Produces before/after pairs for modified files via a main-baseline worktree.
  • On-demand (path argument): captures every story under the given path, regardless of diff. For docs, investigations, baselines, or sharing.
• Adds .claude/skills/fxa-pr-screenshots/capture.js — Node script driving headless Chromium (via Playwright). Reads JSON targets from stdin, writes one PNG per story.
• Registers /fxa-pr-screenshots in CLAUDE.md's Available Skills table.
• Adds .claude/screenshots/ to .gitignore.

Issue that this pull request solves

Closes: FXA-13717

Checklist

Put an x in the boxes that apply

• My commit is GPG signed.
• If applicable, I have modified or added tests which pass locally.
• I have added necessary documentation (if appropriate).
• I have verified that my changes render correctly in RTL (if appropriate).
• I have manually reviewed all AI generated code.

How to review (Optional)

• Depends on Playwright 1.44.1 (already a devDep of functional-tests, hoisted to node_modules/.bin/playwright). First-time use prompts for npx playwright install chromium (~150MB, once per machine).

Screenshots (Optional)

Please attach the screenshots of the changes made in case of change in user interface.

Other information (Optional)

Paired with /fxa-pr-open (separate PR on this ticket). Both skills land under .claude/skills/ and are shared with the team.

[Copilot]

Pull request overview

Adds a new Claude skill (fxa-pr-screenshots) that guides engineers through generating a Storybook-based screenshot checklist for UI and email-template PRs in the FxA monorepo, including guidance on when to do after-only vs. before/after captures and how to use a separate git worktree for baselines.

Changes:

• Introduces a new skill document that maps changed files (vs main) to the appropriate Storybook surface (fxa-settings, fxa-react, accounts-email-renderer).
• Provides step-by-step instructions for finding relevant *.stories.* files and extracting Storybook titles/variants to build a capture checklist.
• Documents a safe baseline approach for before/after screenshots using a separate worktree (no branch switching in the primary checkout).

Comments suppressed due to low confidence (1)

.claude/skills/fxa-pr-screenshots/SKILL.md:90

• Same as above: the baseline worktree snippet uses nx run accounts-email-renderer:storybook, but libs/accounts/email-renderer/README.md documents nx storybook accounts-email-renderer. Aligning the snippet with the documented command (or noting the alias) will make the instructions easier to follow.

git worktree add ../fxa-main-snapshot main
cd ../fxa-main-snapshot && yarn install
# UI:    yarn workspace fxa-settings storybook   (or fxa-react)
# Email: nx run accounts-email-renderer:storybook
# Capture "before" screenshots from this worktree

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 3 out of 4 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (11)

.claude/skills/fxa-pr-screenshots/SKILL.md:71

• This references Step 4, but the before/after capture mode is actually described in Step 5. As written, it sends readers to the Playwright-installation step instead of the capture instructions.

The "New?" column drives capture mode in Step 4.

.claude/skills/fxa-pr-screenshots/capture.js:44

• Using fullPage: false captures only the 1280×800 viewport, which crops tall stories. Email stories in this repo render both HTML and plaintext sections in one page (libs/accounts/email-renderer/src/storybook-email.tsx:84-92), so many template captures will omit the lower content even though the skill is intended to produce one PNG per email story.

    await page.screenshot({ path: outPath, fullPage: false });

.claude/skills/fxa-pr-screenshots/capture.js:42

• For email stories, this wait can finish on the initial placeholder instead of the rendered template. storybookEmail immediately inserts Loading email... and then replaces it asynchronously after renderUsingMJML resolves (libs/accounts/email-renderer/src/storybook-email.tsx:64-95), so a fixed 300 ms delay can produce screenshots of the loading state when rendering takes longer.

    await page.waitForFunction(
      () => {
        const root = document.querySelector('#storybook-root');
        return root && root.children.length > 0 && root.textContent.trim().length > 0;
      },
      { timeout: RENDER_TIMEOUT_MS }
    );
    await page.waitForTimeout(SETTLE_MS);

.claude/skills/fxa-pr-screenshots/SKILL.md:91

• The manifest importPath identifies the story file, not the component/template file that changed. If this matching is done against the changed file paths, a typical component edit such as .../index.tsx will not match .../index.stories.tsx, leaving the target list empty; the matching needs to use the story files discovered in Step 2.

The manifest lists every story's `id` (slugified path, e.g. `components-boxbutton--default`), `title`, `name`, and `importPath`. Match each story's `importPath` against the changed files from Step 2 to build the capture target list.

.claude/skills/fxa-pr-screenshots/SKILL.md:40

• Diff-driven detection omits libs/accounts/email-renderer/src/partials/**, even though email partials have their own Storybook stories (for example libs/accounts/email-renderer/src/partials/userInfo/index.stories.ts) and are included by templates. A PR that changes a shared partial will be told no screenshots are needed, despite changing rendered email output.

  | Email templates / layouts | `libs/accounts/email-renderer/src/{templates,layouts}/**/*.{mjml,ts,txt,ftl}` | `accounts-email-renderer` — port 4400 |

.claude/skills/fxa-pr-screenshots/SKILL.md:113

• This shell form is fragile because the JSON is wrapped in single quotes; any single quote in a generated value (for example, a branch-derived outPath) will break the command before the script sees valid JSON. A heredoc or writing the JSON directly to stdin avoids depending on shell quoting.

echo '<targets-json>' | node .claude/skills/fxa-pr-screenshots/capture.js

.claude/skills/fxa-pr-screenshots/SKILL.md:123

• The baseline worktree path is fixed, so concurrent runs (or a stale worktree from a previous interrupted run) will collide. This is especially risky given the hard rule calls out parallel Claude sessions; one run could fail to add the worktree or later remove ../fxa-main-snapshot while another run is using it. Use a unique per-run/per-branch worktree path instead.

1. Print, confirm, then run: `git worktree add ../fxa-main-snapshot main` followed by `(cd ../fxa-main-snapshot && yarn install)` — `yarn install` in a fresh worktree is slow, so this needs explicit user approval.
2. From the baseline worktree, start the same Storybook(s) on the same ports — there will be a port conflict, so stop the branch-side Storybook first or use different ports per run.
3. Capture the modified-file stories there as `before-*.png` (e.g. `<surface>/before/<story-id>.png`).
4. Clean up: `git worktree remove ../fxa-main-snapshot`.

.claude/skills/fxa-pr-screenshots/SKILL.md:42

• This row marks content-server changes as screenshot-relevant, but the rest of the skill only defines a Storybook-based target pipeline. There are no commands or URL-discovery steps for the “capture from running app” path, so a content-server PR can enter the screenshot flow without an executable capture procedure.

  | Content-server pages (legacy) | `packages/fxa-content-server/**` | no Storybook — capture from running app; note as legacy |

.claude/skills/fxa-pr-screenshots/SKILL.md:39

• The UI globs exclude image assets, so visual-only changes such as packages/fxa-react/components/LoadingSpinner/spinner.svg are treated as “no screenshots needed” even though they change rendered UI. Include the asset extensions used by these components, or otherwise resolve imported assets from changed stories.

  | Settings UI | `packages/fxa-settings/**/*.{tsx,scss,css}` | `fxa-settings` — port 6008 |
  | Shared React components | `packages/fxa-react/**/*.{tsx,scss,css}` | `fxa-react` — port 6007 |

.claude/skills/fxa-pr-screenshots/SKILL.md:84

• The skill starts Storybook processes in the background but never tells the user to stop them after capture. Those processes will keep ports 6007/6008/4400 occupied and can cause later runs (or baseline captures on the same ports) to fail unless cleanup is added to the workflow.

Background-start pattern: `<cmd> > /tmp/storybook-<surface>.log 2>&1 &`. Poll until ready:

until curl -sf http://localhost:/iframe.html > /dev/null; do sleep 2; done

**.claude/skills/fxa-pr-screenshots/SKILL.md:123**
* If the capture is run from the baseline worktree, the relative `.claude/screenshots/.../before` paths are written inside `../fxa-main-snapshot`; the subsequent `git worktree remove` deletes those before-images, and the markdown emitted from the primary worktree will point at files that do not exist. The workflow needs to write baseline captures back to the primary checkout (or run the capture command from the primary checkout against the baseline Storybook URL).

2. From the baseline worktree, start the same Storybook(s) on the same ports — there will be a port conflict, so stop the branch-side Storybook first or use different ports per run.
3. Capture the modified-file stories there as before-*.png (e.g. <surface>/before/<story-id>.png).
4. Clean up: git worktree remove ../fxa-main-snapshot.

</details>

[dschom]

This ports can shift I think.

[dschom]

Looks pretty good, but do we have to install chromium? Won't the default firefox for playwright be fine?

[dschom]

I find the last part of this quite confusing. "The only acceptable way to get a main baseline is a separate worktree (git worktree add ../<name> main)." Later on you have a nice clean diff command that wouldn't require a checkout. Just curious why this is even a rule at all?

[vpomerleau]

This was to get before/after screenshots from different branches.