chore: update backstop to set themes via localstorage by mcoker · Pull Request #8279 · patternfly/patternfly

mcoker · 2026-04-04T04:58:46Z

Glass theme is returning a handful of random false positives, likely due to the size of the background image I imagine. This PR updates backstop to set themes via localStorage instead of adding classnames, which allows the page to load with the proper theme classes instead of adding them after the page renders.

The second commit updates the handful of false positives that this PR fixed.

Summary by CodeRabbit

Refactor
- Refactored theme preference handling to use localStorage-based storage instead of CSS class injection.
- Simplified dark mode detection by leveraging native color scheme emulation.
- Streamlined theme application logic in the testing infrastructure.

coderabbitai · 2026-04-04T04:59:28Z

Walkthrough

This PR refactors theme handling in Backstop's Playwright engine scripts. It removes the applyThemeClasses.js helper and shifts theme application from direct DOM manipulation to localStorage-based preferences set via an init script in onBefore, enabling themes to be applied earlier during page initialization.

Changes

Cohort / File(s)	Summary
Theme Handler Refactoring `backstop_data/engine_scripts/playwright/applyThemeClasses.js`, `backstop_data/engine_scripts/playwright/onBefore.js`, `backstop_data/engine_scripts/playwright/onReady.js`	Removed `applyThemeClasses.js` helper that applied CSS theme classes via DOM manipulation. Introduced `page.addInitScript()` in onBefore to set localStorage keys (`contrast-preference`, `theme-preference`, `theme-variant-preference`) based on CLI flags (`--felt`, `--dark`, `--glass`, `--high-contrast`). Simplified onReady to only handle `--dark` flag with `page.emulateMedia()`.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

chore: update backstop to support addl themes #8276: Modifies the same Playwright theme-handling files (applyThemeClasses.js and onReady.js) with complementary DOM class application approach.
fix(glass): remove background from page, updage dark glass bgimage #8264: Adds CSS rules for pf-v6-theme-glass class that this PR removes from runtime application.
chore(backstop): tweak parameters to reduce false positives #8041: Updates Backstop's Playwright engine scripts with theme-related initialization and script path modifications.

Suggested reviewers

srambach

🚥 Pre-merge checks | ✅ 2

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title follows conventional commit format with 'chore:' prefix and clearly describes the main change: updating backstop to set themes via localStorage instead of applying theme classes after page render.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

patternfly-build · 2026-04-04T05:02:06Z

Preview: https://pf-pr-8279.surge.sh

A11y report: https://pf-pr-8279-a11y.surge.sh

coderabbitai

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@backstop_data/engine_scripts/playwright/onBefore.js`:
- Around line 6-9: Detect and handle conflicting contrast flags passed via
process.argv by checking the boolean flags isGlass and isHighContrast together
and failing fast or normalizing them before any suffix or theme selection logic
runs: if both are present either throw an error (with a clear message) or pick a
single canonical preference and clear the other boolean, and then ensure the
same normalized value is used when building the rendered theme/path suffix (the
code that reads isGlass/isHighContrast to create suffix parts) so you never
produce a suffix like "_glass_hc" that won't be rendered.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI (base), Organization UI (inherited)

Review profile: CHILL

Plan: Pro

Run ID: 4a592e1c-9c12-42e1-94da-f2888fae9fe0

📥 Commits

Reviewing files that changed from the base of the PR and between 359353d and 7c4c8e3.

⛔ Files ignored due to path filters (12)

backstop_data/bitmaps_reference_glass/pf-core__components_clipboard-copy_html_inline-compact-with-additional-action_0_document_1_tablet.png is excluded by !**/*.png
backstop_data/bitmaps_reference_glass/pf-core__components_description-list_html_term-help-text_0_document_2_desktop.png is excluded by !**/*.png
backstop_data/bitmaps_reference_glass/pf-core__components_label_html_vertical-label-group-with-removable-category_0_document_1_tablet.png is excluded by !**/*.png
backstop_data/bitmaps_reference_glass/pf-core__components_navigation_html-demos_horizontal-nav-with-horizontal-subnav_0_document_0_mobile.png is excluded by !**/*.png
backstop_data/bitmaps_reference_glass/pf-core__components_navigation_html-demos_horizontal-subnav_0_document_0_mobile.png is excluded by !**/*.png
backstop_data/bitmaps_reference_glass/pf-core__components_navigation_html_grouped-nav_0_document_0_mobile.png is excluded by !**/*.png
backstop_data/bitmaps_reference_glass/pf-core__components_navigation_html_horizontal-subnav_0_document_1_tablet.png is excluded by !**/*.png
backstop_data/bitmaps_reference_glass/pf-core__components_navigation_html_horizontal-subnav_0_document_2_desktop.png is excluded by !**/*.png
backstop_data/bitmaps_reference_glass/pf-core__components_navigation_html_nav-item-icons_0_document_1_tablet.png is excluded by !**/*.png
backstop_data/bitmaps_reference_glass/pf-core__components_navigation_html_nav-item-icons_0_document_2_desktop.png is excluded by !**/*.png
backstop_data/bitmaps_reference_glass/pf-core__components_tooltip_html_top_0_document_1_tablet.png is excluded by !**/*.png
backstop_data/bitmaps_reference_glass/pf-core__components_tooltip_html_top_0_document_2_desktop.png is excluded by !**/*.png

📒 Files selected for processing (3)

backstop_data/engine_scripts/playwright/applyThemeClasses.js
backstop_data/engine_scripts/playwright/onBefore.js
backstop_data/engine_scripts/playwright/onReady.js

💤 Files with no reviewable changes (1)

backstop_data/engine_scripts/playwright/applyThemeClasses.js

coderabbitai · 2026-04-04T05:12:20Z

+  const isFelt = process.argv.includes('--felt');
+  const isDark = process.argv.includes('--dark');
+  const isGlass = process.argv.includes('--glass');
+  const isHighContrast = process.argv.includes('--high-contrast');


⚠️ Potential issue | 🟠 Major

Handle conflicting contrast flags explicitly.

On Line 6-9 and Line 14-18, --glass and --high-contrast can both be passed, but only one contrast preference is applied (contrast-high wins). Meanwhile backstop.js can still generate a combined suffix (e.g., _glass_hc), which risks writing/reading baselines under a theme label that is never actually rendered.

Please fail fast (or normalize flags in one shared place) to keep rendered theme and path suffix semantics aligned.

Suggested fix

const isFelt = process.argv.includes('--felt'); const isDark = process.argv.includes('--dark'); const isGlass = process.argv.includes('--glass'); const isHighContrast = process.argv.includes('--high-contrast'); + + if (isGlass && isHighContrast) { + throw new Error('`--glass` and `--high-contrast` are mutually exclusive.'); + }

Also applies to: 14-18

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed. In `@backstop_data/engine_scripts/playwright/onBefore.js` around lines 6 - 9, Detect and handle conflicting contrast flags passed via process.argv by checking the boolean flags isGlass and isHighContrast together and failing fast or normalizing them before any suffix or theme selection logic runs: if both are present either throw an error (with a clear message) or pick a single canonical preference and clear the other boolean, and then ensure the same normalized value is used when building the rendered theme/path suffix (the code that reads isGlass/isHighContrast to create suffix parts) so you never produce a suffix like "_glass_hc" that won't be rendered.

patternfly-build · 2026-04-06T15:12:04Z

🎉 This PR is included in version 6.5.0-prerelease.65 🎉

The release is available on:

Your semantic-release bot 📦🚀

mcoker added 2 commits April 3, 2026 23:54

chore: update backstop to set themes via localstorage

7b41c9e

chore: add updated screenshots

7c4c8e3

coderabbitai Bot reviewed Apr 4, 2026

View reviewed changes

mcoker merged commit 9fc1a6b into patternfly:main Apr 4, 2026
5 checks passed

mcoker deleted the backstop-switch-to-localStorage branch April 4, 2026 05:17

patternfly-build added the released on @prerelease label Apr 6, 2026

This was referenced Apr 29, 2026

Update "redhat" theme to "project felt" #8346

Closed

Add baseline screenshots for all of the themes #8363

Closed

coderabbitai Bot mentioned this pull request May 8, 2026

Bug - Code editor - Background color should match monaco background in dark theme #8382

Open

3 tasks

This was referenced Jul 22, 2026

The automated release is failing 🚨 #8519

Closed

The automated release is failing 🚨 #8533

Closed

Navigation Menu

Uh oh!

chore: update backstop to set themes via localstorage - #8279

chore: update backstop to set themes via localstorage#8279
mcoker merged 2 commits into
patternfly:mainfrom
mcoker:backstop-switch-to-localStorage

mcoker commented Apr 4, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Apr 4, 2026 •

edited

Loading

Uh oh!

patternfly-build commented Apr 4, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot left a comment

Uh oh!

coderabbitai Bot Apr 4, 2026

Uh oh!

Uh oh!

patternfly-build commented Apr 6, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

mcoker commented Apr 4, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Apr 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

Suggested reviewers

Uh oh!

patternfly-build commented Apr 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

coderabbitai Bot Apr 4, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

patternfly-build commented Apr 6, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

mcoker commented Apr 4, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Apr 4, 2026 •

edited

Loading

patternfly-build commented Apr 4, 2026 •

edited

Loading