Skip to content

Commit 0382bd5

Browse files
committed
docs: prefer committed visual snapshots; split videos into their own lane
1 parent 4401e58 commit 0382bd5

3 files changed

Lines changed: 44 additions & 7 deletions

File tree

.github/pull_request_template.md

Lines changed: 24 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -38,14 +38,35 @@ Detail the steps to test your changes. This helps reviewers verify your work.
3838
- [ ] **Scope**: My changes are focused on the linked issue (one major feature/fix per PR).
3939
- [ ] **Self-Review**: I have performed a thorough self-review of my code.
4040
- [ ] **Testing**: New and/or updated tests have been added to cover my changes (if applicable).
41+
- [ ] **Visual Snapshot** (UI changes only): If a user would notice this change at a glance (layout, theme tokens, brand elements, empty/error states), I've added or updated a `*.visual.tsx` snapshot in `webview-ui/`. See `webview-ui/AGENTS.md` → "When a UI change needs a snapshot".
4142
- [ ] **Documentation Impact**: I have considered if my changes require documentation updates (see "Documentation Updates" section below).
4243
- [ ] **Contribution Guidelines**: I have read and agree to the [Contributor Guidelines](/CONTRIBUTING.md).
4344

44-
### Screenshots / Videos
45+
### Visual Snapshots
4546

4647
<!--
47-
For UI changes, please provide before-and-after screenshots or a short video of the *actual results*.
48-
This greatly helps in understanding the visual impact of your changes.
48+
For UI changes to static rendered state, the primary artifact is a committed
49+
Playwright CT snapshot (`*.visual.tsx` in `webview-ui/`) — that baseline
50+
becomes durable regression coverage for the surface. See
51+
`webview-ui/AGENTS.md` for what deserves a snapshot.
52+
53+
Before/after screenshots pasted here are welcome as a review aid but are not a
54+
substitute for the committed baseline. If a snapshot is possible, prefer the
55+
snapshot.
56+
-->
57+
58+
### Videos (interaction / animation only)
59+
60+
<!--
61+
Snapshots cannot capture motion or multi-step flows. Attach a short screen
62+
recording here when reviewers need to see:
63+
- A new interactive flow (dropdown, form, dialog progression)
64+
- Animation, transition, or timing behavior
65+
- A regression that only manifests during interaction
66+
67+
Videos are a review aid, not regression coverage. If the *result* of the
68+
interaction has a distinct rendered state worth protecting, still commit a
69+
`*.visual.tsx` snapshot of that end-state alongside the video.
4970
-->
5071

5172
### Documentation Updates

webview-ui/AGENTS.md

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,26 @@ Codecov tracks `webview-ui` coverage under the `webview-ui` flag.
4242

4343
## Visual Tests (Playwright CT)
4444

45-
- Add Playwright screenshot tests selectively for components where layout, styling, VS Code theme variables, or real web-component rendering are part of the behavior under test.
45+
### When a UI change needs a snapshot
46+
47+
If your PR changes anything a user would notice at a glance — layout, spacing, theme tokens, brand elements, gradients, mask/blur effects, hover/empty/error states — **add a `*.visual.tsx` snapshot to the same PR**. Do not attach screenshots to the PR description as evidence; commit the baseline instead so future PRs get regression coverage automatically.
48+
49+
Visual regression is screen-based, not line-based — the goal is a small set of durable "pixel receipts" for the surfaces users see first, not blanket coverage. Prefer covering:
50+
51+
- Onboarding / first-run surfaces (welcome view, hero, unconfigured state)
52+
- Empty and error states (no history, provider misconfig, degraded modes)
53+
- Theme-critical layouts that rely heavily on `--vscode-*` tokens or CSS masks/gradients
54+
- One representative snapshot per user-facing screen — not per component
55+
56+
Skip a visual test when the change is behavior-only (state transitions, handler wiring, validation) — those belong in Vitest. Visual tests are for what JSDOM cannot verify.
57+
58+
### Where the two coverage flags fit
59+
60+
- `webview-ui` (Vitest + JSDOM) — broad line coverage over component logic, hooks, and state. This is your main coverage gate.
61+
- `webview-ui-ct` (Playwright CT) — narrow pixel-regression signal over a small set of critical screens. Low absolute % is expected and fine; the flag is not a coverage-to-hit target, it's a "did the surface still render the same" check.
62+
63+
### Authoring rules
64+
4665
- Keep behavioral assertions in Vitest. A `*.visual.tsx` test should establish a deterministic state and make a focused screenshot assertion.
4766
- Run visual comparisons with `pnpm test:visual:docker` from `webview-ui/`.
4867
- Update intentional baselines with `pnpm test:visual:docker:update` and commit the resulting `__screenshots__` files with the UI change.

webview-ui/playwright/monocart-reporter.d.ts

Lines changed: 0 additions & 3 deletions
This file was deleted.

0 commit comments

Comments
 (0)