feat(regression-test): render the suite with the new dark theme#2628
Open
balzss wants to merge 11 commits into
Open
feat(regression-test): render the suite with the new dark theme#2628balzss wants to merge 11 commits into
balzss wants to merge 11 commits into
Conversation
Apply the new theming system's `dark` theme globally via the InstUISettingsProvider `theme` prop so the visual regression screenshots exercise the new theme instead of the default canvas theme. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Visual regression report
Diff images (33)alert.png — baseline no longer producedavatar.png — baseline no longer producedbadge.png — baseline no longer producedbillboard.png — baseline no longer producedbreadcrumb.png — baseline no longer producedbutton-and-derivatives.png — baseline no longer producedbyline.png — baseline no longer producedcalendar.png — baseline no longer producedcheckbox.png — baseline no longer producedcheckboxgroup.png — baseline no longer producedcolorpicker.png — baseline no longer producedcontextview.png — baseline no longer producedcustom-and-lucide-icons.png — baseline no longer produceddateinput-dateinput2.png — baseline no longer produceddatetimeinput.png — baseline no longer produceddiff-demo.png — 6324 pixels differdrilldown.png — baseline no longer producedfiledrop.png — baseline no longer producedform-errors.png — baseline no longer producedheading.png — baseline no longer producedimg.png — baseline no longer producedlink.png — baseline no longer producedmenu.png — baseline no longer producedmetric-pill-tag-timeselect-text.png — baseline no longer producedoptions.png — baseline no longer producedpagination.png — baseline no longer producedprogressbar.png — baseline no longer producedselect-simpleselect.png — baseline no longer producedtable.png — baseline no longer producedtabs.png — baseline no longer producedtooltip.png — baseline no longer producedtreebrowser.png — baseline no longer producedview.png — baseline no longer producedBaselines come from the |
An axe violation throws in `cy.checkA11y`, which failed the Cypress test and triggered two problems in the visual regression pipeline: the deterministic afterEach screenshot was skipped (the page got a blank/missing baseline), and Cypress' automatic failure screenshot (the error overlay) leaked into the diff as a spurious "new" image because it has a different filename. Always take the afterEach screenshot even when the test failed, and disable Cypress' failure screenshots. The a11y assertion still fails the run as a gate, but the visual baselines are now clean and complete regardless of a11y state. This bug was latent from the start; it only surfaced once a change introduced an actual a11y violation. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…dark themes Render each component page under multiple themes and screenshot each one, so the visual regression suite compares canvas, light, and dark side by side. - layout.tsx reads the `?theme=` query param and applies the matching theme via InstUISettingsProvider. It renders `canvas` for the first paint and switches in an effect to avoid a hydration mismatch, exposing `data-theme` on <html> so the spec can wait for the theme to be applied before screenshotting. - spec.cy.ts is now data-driven: a PAGES list (slug/title/wait/a11y-skip) times a THEMES list. Each page is screenshotted as `<slug>-<theme>` before the axe check so an a11y failure can never drop a visual baseline. Axe runs per theme with skipFailures and the totals are asserted once at the end, keeping a11y a gate across all themes without aborting the other themes' screenshots. - Screenshot capture moved out of the support afterEach into the spec. Screenshot filenames change from `<title>` to `<slug>-<theme>`, so the first run re-baselines every image. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…r to the visual-diff report Make the report easier to read for the multi-theme regression suite: - The Diff image now renders the actual screenshot with a bright outline (and faint fill) around each changed region, grouped from the changed-pixel mask via a coarse-grid connected-components pass. This replaces pixelmatch's faded ghost + scattered red pixels, which was hard to interpret — especially for large or anti-aliased changes. Diff PNGs are now generated only for changed pairs. - A new --facets flag renders a row of one-click filter chips (e.g. canvas/light/dark) that filter screenshots by the "<name>-<facet>" suffix, alongside the existing status filter and name search. Generic; no theme names are hardcoded in the tool. - The visual regression workflow passes --facets canvas,light,dark. Adds unit tests for the region-boxing logic. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Updates the visual regression testing setup to support capturing screenshots across multiple InstUI themes (via ?theme=) and enhances the visual diff report to better highlight where changes occurred and to optionally filter results by “facet” (e.g., theme).
Changes:
- Add query-param-driven theming to the regression-test app layout and expose the applied theme via
data-theme. - Refactor the Cypress suite to generate visits/screenshots/meta per page and per theme, and disable Cypress’ automatic failure screenshots.
- Improve
ui-scripts visual-diffreport output with changed-region highlighting and optional facet filter chips (wired into the workflow).
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| regression-test/src/app/layout.tsx | Select InstUI theme via ?theme= and expose data-theme for test synchronization |
| regression-test/README.md | Document per-theme screenshot capture and new page-spec workflow |
| regression-test/cypress/support/e2e.ts | Remove global screenshot/meta afterEach (now driven by spec) |
| regression-test/cypress/e2e/spec.cy.ts | Generate per-page, per-theme visits/screenshots/a11y checks from data arrays |
| regression-test/cypress.config.ts | Disable failure screenshots to avoid polluting visual baselines |
| packages/ui-scripts/lib/commands/visual-diff.ts | Produce highlighted “diff” images + facet filter UI in report |
| packages/ui-scripts/lib/node_tests/visual-diff.test.ts | Add unit tests for changed-mask → bounding-box extraction |
| .github/workflows/visual-regression.yml | Pass facet list to visual-diff so the report can filter by theme |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+60
to
+65
| useEffect(() => { | ||
| const requested = new URLSearchParams(window.location.search).get('theme') | ||
| if (requested && requested in themes) { | ||
| setThemeKey(requested as ThemeKey) | ||
| } | ||
| }, []) |
Contributor
Author
There was a problem hiding this comment.
Correct in principle — in walks the prototype chain, so ?theme=toString would resolve themeKey to a non-theme value. Leaving as-is for now: in this internal harness the param is driven solely by the fixed THEMES list in the Cypress spec, never untrusted input, so a prototype key can't occur in practice and would at worst produce an obviously-broken screenshot. Happy to guard with Object.hasOwn(themes, requested) if you'd prefer the belt-and-suspenders.
Comment on lines
+170
to
+175
| THEMES.forEach((theme) => { | ||
| cy.visit(`${BASE_URL}/${slug}?theme=${theme}`) | ||
| // Wait until the requested theme has actually been applied before doing | ||
| // anything else (layout.tsx sets data-theme in an effect after mount). | ||
| cy.get(`html[data-theme="${theme}"]`) | ||
| if (wait) { |
Contributor
Author
There was a problem hiding this comment.
Fixed in bab17fd. The stub was replaced on every cy.visit, and since each test now visits once per theme, the afterEach only saw the final (dark) visit — console errors in the canvas/light renders were dropped. The outgoing stub's callCount is now folded into a running total on each window:before:load, reset in a beforeEach, and the afterEach asserts the accumulated total. A console.error in any theme now fails the test.
Comment on lines
+33
to
+36
| // Never capture Cypress' automatic failure screenshots (the error overlay). | ||
| // Visual regression baselines come solely from the deterministic afterEach | ||
| // `cy.screenshot`; a failure screenshot has a different filename and would | ||
| // otherwise leak into the diff as a spurious "new" image. |
Contributor
There was a problem hiding this comment.
This, as well as the "Build & capture." section (line 15) in docs/contributing/testing/visual-regression.md, could be updated to reflect the current workflow.
Contributor
Author
There was a problem hiding this comment.
Fixed in bab17fd — comment updated. Screenshots now come from the cy.screenshot calls in the spec (per theme), not an afterEach.
Contributor
Author
There was a problem hiding this comment.
Good catch — fixed in bab17fd. Updated both the cypress.config comment and the "Build & capture" step in docs/contributing/testing/visual-regression.md to reflect that screenshots are taken directly in the spec, once per theme.
Comment on lines
+392
to
+396
| <div class="filter" id="facet-filter"> | ||
| <span style="font-size:12px;color:#666;">Theme:</span> | ||
| <button data-facet="all" class="active">All</button> | ||
| ${facets.map((f) => `<button data-facet="${f}">${f}</button>`).join('')} | ||
| </div>` |
Contributor
Author
There was a problem hiding this comment.
Leaving as-is. This would only matter if facets came from untrusted input; they're hardcoded in the workflow (--facets canvas,light,dark), and the whole report is a trusted template that interpolates many values this way. No untrusted path exists, so escaping just these two would be inconsistent without a real benefit. Can revisit if facets ever become user-supplied.
Comment on lines
+74
to
+76
| <InstUISettingsProvider | ||
| theme={themes[themeKey]} | ||
| instanceCounterMap={new Map()} |
Contributor
Author
There was a problem hiding this comment.
Updated the PR description to match the current behavior — defaults to canvas and captures canvas/light/dark via ?theme=, one screenshot per page per theme.
Contributor
|
regression-test/README.md contains an outdated link: |
git-nandor
requested changes
Jul 16, 2026
Comment on lines
+33
to
+36
| // Never capture Cypress' automatic failure screenshots (the error overlay). | ||
| // Visual regression baselines come solely from the deterministic afterEach | ||
| // `cy.screenshot`; a failure screenshot has a different filename and would | ||
| // otherwise leak into the diff as a spurious "new" image. |
Contributor
There was a problem hiding this comment.
This, as well as the "Build & capture." section (line 15) in docs/contributing/testing/visual-regression.md, could be updated to reflect the current workflow.
Each test now visits the page once per theme, and every cy.visit re-stubs console.error. The afterEach only saw the final visit's stub, so console errors during the canvas or light render were silently dropped. Accumulate the call count across visits and assert the total. Also refresh stale docs: correct the visual-regression guide link in the regression-test README, and update the cypress.config comment and the guide's capture step to reflect screenshots being taken directly in the spec (per theme) rather than in an afterEach hook. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
Author
|
@git-nandor fixed in bab17fd — the regression-test README now links to |
| - **Detect unexpected console errors** — the spec's `afterEach` hook asserts `console.error` was not called. | ||
|
|
||
| See the [visual regression testing guide](../docs/testing/visual-regression.md) for the full CI pipeline, the diff report UI, and tuning notes. | ||
| Each page is captured once **per theme** (`canvas`, `light`, `dark`), so screenshots are named `<slug>-<theme>.png`. The theme is selected with the `?theme=<key>` query param, which `src/app/layout.tsx` reads and applies via `InstUISettingsProvider`. Add or remove themes with the `THEMES` array in `cypress/e2e/spec.cy.ts` (this multiplies the screenshot/baseline count). |
| On every PR: | ||
|
|
||
| 1. **Build & capture.** GitHub Actions builds InstUI, builds the `regression-test` Next.js app as a static site, and serves it. Cypress visits every page under `regression-test/src/app/*`, waits for it to render, and calls `cy.screenshot()` in an `afterEach` hook, producing one PNG per test case. | ||
| 1. **Build & capture.** GitHub Actions builds InstUI, builds the `regression-test` Next.js app as a static site, and serves it. Cypress visits every page under `regression-test/src/app/*` once per theme (via the `?theme=` query param), waits for each to render, and calls `cy.screenshot()` directly in the spec, producing one PNG per page/theme combination. |
Paint the page background from the active theme's background.page semantic token so every theme is screenshotted on the surface it's actually used on. Most visibly, the dark theme now renders on its near-black surface instead of white. Canvas stays white; light shifts to its faint-grey page surface. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The lightbox gains an HTML mode alongside Baseline/Actual/Diff/Slider that iframes the live rendered page, so a reviewer can inspect the real DOM, computed styles, and text next to the pixels. The page URL is derived from the screenshot name via the existing meta map and facet suffixes (appUrlFor), and shown only when the page still exists and the app was published. To back it, CI rebuilds the regression-test app as a static export with a basePath matching its published location and drops it next to the report; visual-diff is passed --app-path app. The basePath is env-driven (REGRESSION_APP_BASE_PATH) so local dev and the Cypress capture build are unaffected. Published reports previously accumulated on gh-pages forever, and shipping the app bundle per PR makes that worse, so the workflow now also removes a PR's report directory when the PR closes. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…Page screenshots capture it The background was set only on <html>. Cypress capture: 'fullPage' stitches the body onto its own canvas and does not reliably paint the <html> background into that image, so the dark surface showed in a real browser (and the report's HTML view) but not in the captured screenshots. Set it on <body> too, with min-height: 100vh so short pages still fill the frame. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The HTTP+SSE endpoint (mcp.atlassian.com/v1/sse) is deprecated after 2026-06-30. Switch to the Streamable HTTP transport (type: http, /v1/mcp). 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…its factory newTheme.semantics is a factory (primitives) => tokens, not a resolved object, so reading `.semantics.color.background.page` off it was always undefined and the background fell back to transparent (white) for every theme. Call the factory with newTheme.primitives — the same way emotion's useComputedTheme resolves it. Verified against the compiled themes: dark #10141A, light #F2F4F5, canvas #ffffff. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
git-nandor
approved these changes
Jul 21, 2026
HerrTopi
approved these changes
Jul 22, 2026
Land three report improvements: - Default the status filter to "Changed" (falling back to "All" when nothing changed) so reviewers see regressions first, and apply filters on load. - Close the lightbox on Escape and navigate with the arrow keys. - Rework the diff image to a Chromatic-style pixel highlight: dim and desaturate the actual screenshot into a faint backdrop and paint the exact changed pixels (dilated for visibility) in the accent color, replacing the coarse bounding boxes. Extracts a tested dilateMask helper. Also add a TEMP diff-demo fixture and a workflow step that injects it so the published report always has one "changed" row to preview the new rendering (this PR's -<theme> screenshot renames leave zero real "changed" rows). Both are marked for removal before merge. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
hajnaldo
approved these changes
Jul 23, 2026
hajnaldo
left a comment
hajnaldo
left a comment
Collaborator
There was a problem hiding this comment.
All looks good for now - maybe later when we use it we'll have suggestions.
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.
Summary
regression-testapp selects an InstUI theme from the?theme=query param (defaulting tocanvas) and exposes the applied theme viadata-theme; the Cypress spec drives one screenshot per page per theme (canvas,light,dark), so baselines are named<slug>-<theme>.png.background.pagesemantic token, so each theme is captured on the surface it's actually used on — the dark theme renders on its near-black surface rather than on white.ui-scripts visual-diffreport with changed-region highlighting and a theme facet filter.gh-pageswhen the PR closes.Jira
Test Plan
visual-regression/pr-2628/directory should be removed fromgh-pages.🤖 Generated with Claude Code