Charts: remove the theme color props and the override bridge by adamwoodnz · Pull Request #51748 · Automattic/jetpack

adamwoodnz · 2026-08-31T03:04:25Z

Why

After CHARTS-262 (#51687) every chart color has a --a8c-charts-color-* role behind it, and the theme color props are a second way to say the same thing. Keeping both means keeping the layer that reconciles them — themeOverrideVars, withCatalogPointers, and an extra indirection on every role a field could reach. Removing the props removes the whole layer.

The fields could not simply be deleted. defaultTheme was the delivery mechanism, not a set of defaults: defaultTheme.gridStyles.stroke is literally var(--a8c-charts-color-grid, #dbdbdb), and that string is what lands in the gridline's inline style. So the chains move first, into private/catalog-pointers.ts.

Read this first — `gridColor` was not dead

The issue says to delete gridColor/gridColorDark because both are '' and read nowhere in src. True of our code, false of visx: buildChartTheme seeds the y axis line and y tick marks from config.gridColor, and the empty string was the only thing leaving the y axis unpainted — which is the design here.

But the y axis never needed gridColor. visx's ThemeConfig has first-class yAxisLineStyles/yTickLineStyles, symmetric with the x side. So this PR adds --a8c-charts-color-axis-y and --a8c-charts-color-tick-y, resolving to none: same default, but the suppression is now a documented role instead of a side effect. Once the y axis has roles, the old names were a lie — --a8c-charts-color-axis/-tick only ever painted x, so both are renamed to -axis-x/-tick-x. There is deliberately no broad --a8c-charts-color-axis above the pair, since the two sides do not share a default.

This is what keeps Calypso whole: chart-theme.ts sets gridColor, so its dashboard charts have a painted y axis today. It gets that back by declaring the two roles — see below.

Proposed changes

Take gridColor/gridColorDark off the public theme, moving the empty strings into the visx call.
Move the catalog pointers into private/catalog-pointers.ts, point every internal reader at the constant, then delete the fields and the bridge.
Give the y axis its own roles, rename the x pair to match, and add a story showing both states.
Let the stylesheet paint what it can reach — the funnel's change indicator, the leaderboard's deltas and the heatmap legend labels now take their color from their own .module.scss instead of routing a pointer through JS into an inline style.
Give LeaderboardChart, ConversionFunnelChart and BarListChart the stable root class the other charts have, and add the missing one to PieSemiCircleChart's error state. BarListChart and that error state also stop dropping a caller's own className.
Fold in the leaderboard gaps deprecated by Charts: move the leaderboard gaps and bar radius to CSS #51866, and collapse --a8c-charts-color-label-on-fill into --a8c-charts-color-label-inverse — see Removals folded in.

Why the new stable classes are needed

A role can be read by more than the chart you want to move. --a8c-charts-color-surface-secondary paints the funnel's track and GeoChart's dataless regions; the trend pair paints the funnel's change indicator and the leaderboard's deltas. Without a per-chart handle, a consumer setting one of those on the provider wrapper silently moves the other chart too — which is what happened to Premium Analytics' map in review (thanks @kangzj for catching it).

This only works for roles the stylesheet paints. A role resolved in JS at the provider wrapper — the palette, the background — still cannot be scoped per chart. That is the limit TOKENS.md records and CHARTS-255 tracks.

Two things deliberately kept

AnnotationStyles keeps its colors as the per-datum styles prop, and ChartTheme takes a colorless ThemeAnnotationStyles. --a8c-charts-color-annotation moves every annotation at once with no per-annotation selector to narrow it, so removing the datum colors would drop a capability with no replacement. The issue implied a blanket removal; this is narrower on purpose.

CompleteChartTheme stays Required< ChartTheme >. The acceptance criteria ask for a partial-overrides type, but the non-color defaults (tickLength, sparkline margin, heatmap sizing) are still guaranteed by defaultTheme and readers rely on it. What goes is the Omit/Pick juggling that existed only to keep optional color members optional.

Migration

Removed	Replacement
`backgroundColor`	`--a8c-charts-color-background`
`labelBackgroundColor`	`--a8c-charts-color-label-background`
`labelTextColor`	`--a8c-charts-color-label-inverse`
`colors[ N - 1 ]`	`--a8c-charts-color-series-N`, or `options.stroke` for one series
`gridStyles.stroke`	`--a8c-charts-color-grid`
`xAxisLineStyles.stroke`	`--a8c-charts-color-axis-x`
`xTickLineStyles.stroke`	`--a8c-charts-color-tick-x`
`legend.labelStyles.color`	`--a8c-charts-color-label`
`svgLabelSmall.fill`	`--a8c-charts-color-label-axis`
`annotationStyles.*` colors	`--a8c-charts-color-annotation`, `--a8c-charts-color-surface`, or the annotation's own `styles` prop
`geoChart.featureFillColor`	`--a8c-charts-color-surface-secondary`
`leaderboardChart.primaryColor` / `.secondaryColor`	the `primaryColor` / `secondaryColor` props, or `--a8c-charts-color-series-1` / `-2`
`leaderboardChart.deltaColors`	`--a8c-charts-color-trend-down` / `-trend-neutral` / `-trend-up`, scoped to `.leaderboard-chart` if the funnel should not follow
`leaderboardChart.rowGap` / `.columnGap`	`--a8c-charts-dimension-leaderboard-row-gap` / `-column-gap`
`conversionFunnelChart.primaryColor`	`--a8c-charts-color-series-1`
`conversionFunnelChart.backgroundColor`	`--a8c-charts-color-surface-secondary`, scoped to `.conversion-funnel-chart` if `GeoChart` should not follow
`conversionFunnelChart.positiveChangeColor` / `.negativeChangeColor`	`--a8c-charts-color-trend-up` / `-trend-down`
`heatmapChart.primaryColor`	the `primaryColor` prop, or `--a8c-charts-color-series-1`
`gridColor`	`--a8c-charts-color-axis-y` and `--a8c-charts-color-tick-y` (new). Both default to `none`
`gridColorDark`	none. Inert — `xAxisLineStyles` already overrode it
`--a8c-charts-color-label-on-fill`	`--a8c-charts-color-label-inverse`

Also renamed: --a8c-charts-color-axis → -axis-x, --a8c-charts-color-tick → -tick-x.

An override must be set inside the provider tree, or on .a8c-charts-scope itself. A rule above GlobalChartsProvider does not apply: the catalog is declared on the provider's own wrapper, and an element's own declaration beats one it inherits.

Removals folded in

Not in the issue, but this is the release for it. The changelog entry is already major, and any PR merged before the next publish rides that same bump — so a separate PR only avoids a second major if nobody publishes in between, which is not something we control.

Removed	Replacement
`AreaChart`'s `rescaleYOnLegendToggle`	`rescaleYOnVisibilityChange` — the same setting, under a name that matches when it applies
`parseRgbString()`	`normalizeColorToHex()`, which handles `rgb()` alongside every other format
`theme.leaderboardChart.rowGap` / `.columnGap`	the two dimension roles above
`--a8c-charts-color-label-on-fill`	`--a8c-charts-color-label-inverse`

The leaderboard gaps were deprecated by #51866 and never reached a release — charts is at 3.2.0, that entry is still unreleased, so 4.0.0 absorbs both. The two changelog entries are merged into one rather than shipping "deprecated" and "removed" side by side in the same version.

label-on-fill and label-inverse were one concept under two names — label text on a filled surface. label-inverse survives because it is WPDS-mapped. This is the one visible change in the PR: pie labels follow --wpds-color-foreground-interactive-neutral-strong now, so they render #f0f0f0 rather than #fff until a consumer sets the role. Premium Analytics already overrode label-on-fill to exactly that token, so its dashboard is unchanged.

Also gone: the "Migration from Legacy API" section in annotation.docs.mdx, which told readers to migrate off an annotations prop that LineChart no longer has.

smoothing is kept. One docs line called it deprecated; the types never did, and "ignored when curveType is set" is an interaction rather than a replacement. That line now says what the types say.

Docs and tests corrected

The docs told readers to set colors on the theme in 21 places across 15 files — a root "Core Concepts" section, nine copies of "the default theme has neutral colors", and a GeoChart API table listing backgroundColor / datalessRegionColor / defaultColor as "Theme value". Twelve test names said the same. Both are now stated in terms of the roles and the palette.

One of those tests was worse than misnamed: use-leaderboard-legend-items built a theme with leaderboardChart.primaryColor/.secondaryColor, passed it to the provider, and asserted only that the colors were truthy. Assigning the literal to a const first suppressed excess-property checking, so nothing flagged it. The dead theme is gone and the test is named for what it checks.

Calypso

client/dashboard/app/chart-theme.ts sets eight fields, six of them color. Every one has a route:

.dashboard {
	--a8c-charts-color-background: var(--dashboard-surface__background-color);
	--a8c-charts-color-grid: var(--dashboard-surface__border-color);
	--a8c-charts-color-axis-x: var(--dashboard-surface__border-color);
	--a8c-charts-color-tick-x: var(--dashboard-surface__border-color);
	--a8c-charts-color-axis-y: var(--dashboard-surface__border-color);
	--a8c-charts-color-tick-y: var(--dashboard-surface__border-color);
	--a8c-charts-color-label: var(--dashboard__text-muted-color);
	--a8c-charts-color-label-axis: var(--dashboard__text-muted-color);
}

The last four lines are what gridColor/gridColorDark were doing by accident. The strokeWidth fields stay on the theme prop. Nothing is lost, but the Calypso change has to land before charts is bumped there.

Premium Analytics

Migrated here, as the one in-repo consumer that set theme colors. useChartTheme keeps only the non-color fields; the colors, the leaderboard row gap and the bar radius move to a chart-roles.scss declaring roles on .a8c-charts-scope, with the two chart-scoped exceptions above. No visible change.

Changelog

One charts entry and one Premium Analytics entry, each absorbing #51866's. No plugin entries — a departure from CHARTS-202 and CHARTS-254, which each landed ten. No plugin in the monorepo sets a charts theme color, so nothing a site owner can see changes in any of them. Say so if you disagree and I will add them.

Related product discussion/links

CHARTS-263, CHARTS-227 (parent), CHARTS-262 (Charts: stop resolving paint-only colors in JS, deprecate the theme color fields [CHARTS-262] #51687, the deprecation this closes), CHARTS-267 (Charts: move the leaderboard gaps and bar radius to CSS #51866, the gaps folded in), CHARTS-255 (the JS-resolved roles this cannot scope per chart)

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

No.

Testing instructions

Run the charts Storybook: pnpm -C projects/js-packages/storybook run storybook:dev.

Open Bar / Line / Area → Fixed Dimensions, Pie, Leaderboard, Conversion Funnel, Heatmap, Geo and Line → Annotations. All match trunk except pie labels, which are now #f0f0f0 rather than #fff. The y axis has no axis line and no tick marks, only labels.
Open Bar Chart → Painted Y Axis. Left is the default; right declares --a8c-charts-color-axis-y (blue) and --a8c-charts-color-tick-y (red). The x axis is identical in both.
In DevTools, set --a8c-charts-color-grid: red on the chart's own root (the element carrying .bar-chart) — gridlines turn red. Set --a8c-charts-color-axis-x: lime on .a8c-charts-scope — the x axis turns green, the y axis stays unpainted.
Set --a8c-charts-color-surface-secondary: red on .a8c-charts-scope with Geo and Conversion Funnel both open. Both move; scoping the same declaration to .conversion-funnel-chart moves only the funnel.
Switch themeName to mixed-color-formats — the palette shows #e63946, #2a9d8f (from rgb()), #facc14 (from hsl()), so format normalization still runs through the roles. Switch to custom — gridlines pink (#ffe3e3, from the role) at stroke-width: 2 (from the theme prop).
Confirm your editor no longer offers backgroundColor, colors, labelTextColor or leaderboardChart.rowGap on theme, and still offers tickLength, gridStyles.strokeWidth, svgLabelSmall.fontFamily and leaderboardChart.labelSpacing. Nor rescaleYOnLegendToggle on AreaChart, nor parseRgbString from the package root.
Open Area Chart → Y-axis rescales when visible series change (default). Toggling a series in the legend rescales the y axis on the left chart and pins it on the right, unchanged from trunk — that story is what the removed rescaleYOnLegendToggle aliased.

Verified

pnpm -C projects/js-packages/charts run test — 85 suites, 1389 passed. pnpm -C projects/packages/premium-analytics run test — 156 suites, 3065 passed.
run typecheck on both packages, run build, eslint, stylelint — clean. run build matters: tools/assert-charts-scope-emitted.ts fails it if the catalog stops reaching dist/index.css, and this PR edits that stylesheet.
Exercised in Chrome at every surface above, reading back getComputedStyle. Grid, x axis line, x tick marks and tick labels each carry the var() chain on the element and resolve; the y pair computes none until declared, then paints, then returns to none on removal, with x untouched throughout. Pie labels #f0f0f0, moving to #00ff00 from .a8c-charts-scope and #ff00ff from .pie-chart. Annotations resolve with radius: 5 intact. Funnel background #f4f4f4, indicator #008030. Heatmap #007cba with the background chain resolving for the color-mix, and the cell fill tracking a background override at both levels. Geo #fff / #f4f4f4, both handed to Google Charts as hex. Leaderboard gaps 12px / 4px, following an override on .a8c-charts-scope.
The .conversion-funnel-chart and .leaderboard-chart scoping checked the same way — the scoped override moves only its own chart, and GeoChart no longer follows.

Rebase note

Rebased onto trunk after #51640 (tooltips render in-tree rather than in a document.body portal) and #51722 (WPDS fallbacks injected at build time). The 33 commits were squashed to one first, so each conflict was resolved against the final state of both sides rather than against intermediate ones. chart-scope.scss lost all 23 hand-written fallbacks to the build-time plugin, and chart-scope.test.ts now compares only the mapping, since the fallback is no longer in source.

🤖 Generated with Claude Code

https://claude.ai/code/session_0147vnw9VyKQkMsknWqyXNc8

github-actions · 2026-08-31T03:05:49Z

Are you an Automattician? Please test your changes on all WordPress.com environments to help mitigate accidental explosions.

To test on WoA, go to the Plugins menu on a WoA dev site. Click on the "Upload" button and follow the upgrade flow to be able to upload, install, and activate the Jetpack Beta plugin. Once the plugin is active, go to Jetpack > Jetpack Beta, select your plugin (Jetpack or WordPress.com Site Helper), and enable the charts-263-charts-remove-the-theme-color-props-and-the-override-bridge branch.
To test on Simple, run the following command on your sandbox:

bin/jetpack-downloader test jetpack charts-263-charts-remove-the-theme-color-props-and-the-override-bridge

bin/jetpack-downloader test jetpack-mu-wpcom-plugin charts-263-charts-remove-the-theme-color-props-and-the-override-bridge

Interested in more tips and information?

In your local development environment, use the jetpack rsync command to sync your changes to a WoA dev blog.
Read more about our development workflow here: PCYsg-eg0-p2
Figure out when your changes will be shipped to customers here: PCYsg-eg5-p2

github-actions · 2026-08-31T03:05:51Z

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:

Ensure all required checks appearing at the bottom of this PR are passing.
Make sure to test your changes on all platforms that it applies to. You're responsible for the quality of the code you ship.
You can use GitHub's Reviewers functionality to request a review.
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!

Jetpack plugin:

The Jetpack plugin has different release cadences depending on the platform:

WordPress.com Simple releases happen as soon as you deploy your changes after merging this PR (PCYsg-Jjm-p2).
WoA releases happen weekly.
Releases to self-hosted sites happen monthly:
- Scheduled release: September 8, 2026