feat(regression-test): render the suite with the new dark theme#2628
Open
balzss wants to merge 4 commits into
Open
feat(regression-test): render the suite with the new dark theme#2628balzss wants to merge 4 commits into
balzss wants to merge 4 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 (32)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 produceddrilldown.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) | ||
| } | ||
| }, []) |
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) { |
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. |
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>` |
Comment on lines
+74
to
+76
| <InstUISettingsProvider | ||
| theme={themes[themeKey]} | ||
| instanceCounterMap={new Map()} |
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
darktheme globally in the regression-test app via theInstUISettingsProviderthemeprop, so visual regression screenshots exercise the new theme instead of the default canvas theme.Test Plan
darktheme.🤖 Generated with Claude Code