Skip to content

CHARTS-287: Storybook: Remove the admin color override - #53340

Open
adamwoodnz wants to merge 1 commit into
renovate/wordpress-monorepofrom
charts-287-remove-the-storybook-admin-color-override-once-base-styles
Open

adamwoodnz wants to merge 1 commit into
renovate/wordpress-monorepofrom
charts-287-remove-the-storybook-admin-color-override-once-base-styles

Conversation

@adamwoodnz

@adamwoodnz adamwoodnz commented Oct 8, 2026 •

Copy link
Copy Markdown
Contributor

Fixes https://linear.app/a8c/issue/CHARTS-287

Why

No visible change in Storybook. Stories already render with the modern admin color (blueberry). The WordPress packages now give that color by default, so the workaround that forced it is dead code that would hide a regression if the default changed again.

Important

Stacked on #53004 (Renovate: Update wordpress monorepo), which bumps storybook to @wordpress/base-styles 13.3.0, block-editor 18.1.0 and block-library 11.2.0. Without that bump, removing the override brings back the legacy #007cba. Merge #53004 first; this PR then rebases onto trunk.

Proposed changes

  • Remove the :root { @include mixins.admin-scheme(#3858e9); } override, its comment, and the now-unused @wordpress/base-styles/mixins import from projects/js-packages/storybook/storybook/style.scss.
  • Add a js-packages/storybook changelog entry.

Related product discussion/links

Does this pull request change what data or activity we track or use?

No.

Testing instructions

  1. Check out this branch, run pnpm install, then pnpm --dir projects/js-packages/storybook storybook:dev.
  2. Open Charts › Bar Chart › Default and Charts › Pie Chart › Default. In the preview iframe DevTools, getComputedStyle(document.documentElement).getPropertyValue('--wp-admin-theme-color') is #3858e9.
  3. Set the adminColorScheme control to midnight or light: the chart scope ([data-testid=charts-scope]) resolves to #cf4339 or #007cba.
  4. Set themeName to custom with an accent color: the chart scope resolves to that accent.

Verification

Ran locally with Playwright against the Storybook from this branch:

Story :root Chart scope
Bar › Default #3858e9 #3858e9
Pie › Default #3858e9 #3858e9
Bar, adminColorScheme: midnight #3858e9 #cf4339
Pie, adminColorScheme: light #3858e9 #007cba
Bar, themeName: custom, accent #ff0000 #3858e9 #f00

stylelint passes on the changed file. No screenshots, because the rendered output is the same as before.

🤖 Generated with Claude Code

@github-actions

github-actions Bot commented Oct 8, 2026

Copy link
Copy Markdown
Contributor

Thank you for your PR!

When contributing to Jetpack, we have a few suggestions that can help us test and review your patch:

  • ✅ Include a description of your PR changes.
  • ✅ Add a "[Status]" label (In Progress, Needs Review, ...).
  • ✅ Add testing instructions.
  • ✅ Specify whether this PR includes any changes to data or privacy.
  • ✅ Add changelog entries to affected projects

This comment will be updated as you work on your PR and make changes. If you think that some of those checks are not needed for your PR, please explain why you think so. Thanks for cooperation 🤖


Follow this PR Review Process:

  1. Ensure all required checks appearing at the bottom of this PR are passing.
  2. Make sure to test your changes on all platforms that it applies to. You're responsible for the quality of the code you ship.
  3. You can use GitHub's Reviewers functionality to request a review.
  4. When it's reviewed and merged, you will be pinged in Slack to deploy the changes to WordPress.com simple once the build is done.

If you have questions about anything, reach out in #jetpack-developers for guidance!

@adamwoodnz

This comment has been minimized.

@adamwoodnz

Copy link
Copy Markdown
Contributor Author

@claude please review this PR. Get the change set with a single gh pr diff 53340, then read only the changed source files it touches. Skip changelog entries and lockfiles. Use at most about 20 tool calls, and post your review before you reach that, even if it is incomplete.

@claude

This comment has been minimized.

@adamwoodnz

Copy link
Copy Markdown
Contributor Author

On Claude's non-blocking notes:

  1. Merge order guard: I will not add one here. The base of this PR is Update wordpress monorepo #53004, so GitHub retargets it to trunk only after Update wordpress monorepo #53004 merges. If someone reverts Update wordpress monorepo #53004 later, this PR still applies cleanly, and the result is the legacy color in Storybook only. No product code is affected. A visual check for one Storybook value is not worth its cost.
  2. Rebase target: trunk is correct. gh repo view Automattic/jetpack --json defaultBranchRef returns trunk.

@adamwoodnz adamwoodnz added [Status] Needs Review This PR is ready for review. and removed [Status] In Progress labels Oct 8, 2026
@adamwoodnz
adamwoodnz marked this pull request as ready for review October 8, 2026 04:17
@adamwoodnz

Copy link
Copy Markdown
Contributor Author

Review cycle summary

  • Round reviewer: Codex, 1 round. No P1 to P3 findings, so the round phase ended early.
  • Final reviewer: Claude. No blocking issues. I declined its two non-blocking notes (a guard for merge order, and a question about which branch to rebase onto) and gave the reasons above.
  • CI: green (38 pass, 26 skipped).
  • Resolved: I collapsed 2 review comments. There were no inline threads.
  • Unaddressed human comments: none.

Merge order: merge #53004 first. GitHub then retargets this PR to trunk.

Status: clean

@anomiex
anomiex force-pushed the renovate/wordpress-monorepo branch from 91c1c95 to 5ae6d4e Compare October 8, 2026 14:57
@anomiex
anomiex requested review from a team as code owners October 8, 2026 14:57
@anomiex
anomiex force-pushed the renovate/wordpress-monorepo branch from 5ae6d4e to ace4253 Compare October 8, 2026 14:59

@LiamSarsfield LiamSarsfield left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, I checked the CSS from the pinned packages, and all seven --wp-admin-* values on :root come out the same with and without the override. With trunk's versions it drops back to #007cba, so the change is neutral once #53004 is in.

Regarding the rebasing:

  • Renovate rebased #53004 this afternoon, but this branch still has the old copy, so it shows as conflicting and the Files tab has the whole bump in it.
  • Rebasing onto the current renovate/wordpress-monorepo should go through cleanly. The description also links a private claude.ai session, which probably shouldn't be on a public PR.

The :root override existed because block-editor and block-library
reset --wp-admin-theme-color to the legacy #007cba. base-styles 13.3.0
(WordPress/gutenberg#83749) defaults it to #3858e9, and the
block-editor 18.1.0 and block-library 11.2.0 builds carry that
default, so the override no longer changes anything.
@adamwoodnz
adamwoodnz force-pushed the charts-287-remove-the-storybook-admin-color-override-once-base-styles branch from 8247c2b to bfd0e9e Compare October 8, 2026 19:13
@adamwoodnz

Copy link
Copy Markdown
Contributor Author

Thanks for checking the CSS against both sets of versions.

  • I rebased onto the current renovate/wordpress-monorepo (ace4253d0a9). The change is now the single commit bfd0e9e0c75, and the Files tab shows only the stylesheet and the changelog.
  • I removed the session link from the description.

@adamwoodnz
adamwoodnz removed the request for review from a team October 8, 2026 19:18
@adamwoodnz adamwoodnz added the DO NOT MERGE don't merge it! label Oct 8, 2026

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants