Theme: Omit color properties when neither provided nor inherited

[aduth]

What?

Updates ThemeProvider to avoid assigning theme color CSS properties when seed values are not provided and when there is no inherited seed values.

Why?

This fixes a regression introduced by #79523 and flagged by @mirka in #79523 (comment), where editor notice action buttons no longer follow the user's selected color scheme.

How?

Updating the logic to avoid assigning theme color CSS properties when seed values are not provided and when there is no inherited seed values is based on a few assumptions:

• Default color values are expected to be available via one or both of (a) loading the design tokens stylesheet or (b) relying on fallback values embedded in component stylesheets by virtue of fallback token value build plugins.
• It's still necessary to retain the existing behavior when seed values are not provided but there's an inherited ThemeProvider settings, since relying on the cascade of CSS properties in DOM hierarchy does not account for configuration that should extend across DOM boundaries in React trees (e.g. portaled content, like overlays). For this reason, the revised implementation only applies to "total default" scenarios, where there is no ancestor, and there is no seed values provided.

Testing Instructions

Verify that the color scheme applies in editor notice action buttons:

1. (Prerequisite) Configure a non-default color scheme under Users > Profile
2. Go to Posts > New
3. Insert snippet below in your browser's DevTools console to emulate an error message
4. Observe that the notice retains the user color scheme

wp.data.dispatch( 'core/notices' ).createSuccessNotice( 'Post published.', {
	actions: [
		{ label: 'View post', url: '#' },
		{ label: 'Primary action', onClick() {}, variant: 'primary' },
		{ label: 'Secondary action', onClick() {} },
	],
} );

Screenshots or screencast

Before	After

Use of AI Tools

Used Cursor IDE + Auto (likely Composer) agent to research and implement, with manual revisions after review on my part to simplify implementation, comment phrasing, and collapse tests.

[aduth]

The changes in this file aren't strictly needed and were authored by AI, but I think their inclusion and standardization helps drive home the point that (a) this was already a documented expectation and (b) the behavior should be consistent with other customization props.

[github-actions]

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: aduth <aduth@git.wordpress.org>
Co-authored-by: mirka <0mirka00@git.wordpress.org>
Co-authored-by: ciampo <mciampini@git.wordpress.org>
Co-authored-by: habiburdev <habiburdev@git.wordpress.org>
Co-authored-by: t-hamano <wildworks@git.wordpress.org>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

[github-actions]

Size Change: +42 B (0%)

Total Size: 7.81 MB

📦 View Changed

Filename	Size	Change
build/scripts/theme/index.min.js	26.2 kB	+42 B (+0.16%)

_{compressed-size-action}

[aduth]

One could argue this change lessens coverage of a "default" scenario absent any options provided. We could add an explicit wrapper instead. My interpretation of this test (also evidenced by its name) is in guaranteeing color contrast, and the coverage here applying to default colors, regardless how they take effect. Resolved color settings behavior is covered in the ThemeProvider tests.

[ciampo]

I think this change is reasonable, although it now validates runtime-generated tokens from the default seeds. It does not validate that design-tokens.css (or build fallbacks) deliver those defaults.

Nitty, but should we have a separate artifact/parity test?

[aduth]

I think I follow what you're saying. The coverage of this specific test doesn't change, since it was always using the DEFAULT_SEED_COLORS values, it's just that it happened before implicitly through defaulting in ThemeProvider. But now in cases where we're not generating color properties, we're relying on the values that come through the CSS file or build fallbacks. And we don't (never did) have any coverage that those values align with what's in DEFAULT_SEED_COLORS.

My instinct is that we should merge this now, since (a) the coverage doesn't actually change and (b) since this is planned to be backported, there's both an urgency to merge sooner and smaller changes.

In parallel, I can open a pull request to add coverage for the scenario you're describing. In practice that might look like asserting that values in e.g. design-token-fallbacks.mjs overlap with values in DEFAULT_SEED_COLORS? Open to suggestions though.

[aduth]

In parallel, I can open a pull request to add coverage for the scenario you're describing. In practice that might look like asserting that values in e.g. design-token-fallbacks.mjs overlap with values in DEFAULT_SEED_COLORS? Open to suggestions though.

Exploring that test coverage separately in #81161.

[github-actions]

Flaky tests detected in 7eea78a.
Some tests passed with failed attempts. The failures may not be related to this commit but are still reported for visibility. See the documentation for more information.

🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/30030453254
📝 Reported issues:

• [Flaky Test] two users concurrently move list items (RTC WebSocket) #79951 in /test/e2e/specs/editor/collaboration/collaboration-stress.spec.ts

[mirka]

This makes sense to me, but it'd be nice to get a thumbs up from @ciampo as well.

[mirka]

Nit: The "otherwise" is ambiguous as to what it's contrasting with. It could be clearer that we're contrasting with the parent value being inherited, not the value being omitted. For example:

Suggested change

	* When omitted, inherits from the parent `ThemeProvider`, otherwise
	* the prebuilt default applies.
	* When omitted, inherits from the parent `ThemeProvider`. If there is no parent value to inherit, the prebuilt default applies.

[aduth]

That's a good point. I updated this in beb53f0 (and 7eea78a after some internal debate on including the word "value").

[habiburdev]

Thanks for working on this fix!

I tested the notice action buttons against the testing instructions:

Testing Steps & Results

1. Configured a non-default color scheme under Users > Profile.
2. Created a new Post (Posts > Add New Post).
3. Executed the wp.data.dispatch( 'core/notices' ).createSuccessNotice(...) snippet in the browser DevTools console to generate the notice with primary and secondary action buttons.

Observation

• Verified that the notice action buttons correctly inherit and display the custom user admin color scheme in the editor.
• Primary and secondary action buttons match the expected contrast and styles.

Everything works as expected (see attached screenshot below). LGTM! 🚀

[t-hamano]

This makes sense to me, but it'd be nice to get a thumbs up from @ciampo as well.

@ciampo, would you have time to review this PR so that it can be backported to 7.1?

[ciampo]

@ciampo, would you have time to review this PR so that it can be backported to 7.1?

Sorry for the delay, I had a couple of busy weeks that prevented me from being active in Gutenberg. I'm back now, i'll try to catch up with anything urgent

LGTM 🚀

[ciampo]

I think this change is reasonable, although it now validates runtime-generated tokens from the default seeds. It does not validate that design-tokens.css (or build fallbacks) deliver those defaults.

Nitty, but should we have a separate artifact/parity test?

[github-actions]

There was a conflict while trying to cherry-pick the commit to the wp/7.1 branch. Please resolve the conflict manually and create a PR to the wp/7.1 branch.

PRs to wp/7.1 are similar to PRs to trunk, but you should base your PR on the wp/7.1 branch instead of trunk.

# Checkout the wp/7.1 branch instead of trunk.
git checkout wp/7.1

# Create a new branch for your PR.
git checkout -b my-branch

# Cherry-pick the commit.
git cherry-pick c6b1373629fe58e7c6211e16b7c3371574db0624

# Check which files have conflicts.
git status

# Resolve the conflict...
# Add the resolved files to the staging area.
git status
git add .
git cherry-pick --continue

# Push the branch to the repository
git push origin my-branch

# Create a PR and set the base to the wp/7.1 branch.
# See https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/proposing-changes-to-your-work-with-pull-requests/changing-the-base-branch-of-a-pull-request.

[aduth]

Manual backport PR at: #81172

Aside: Would it be reasonable for the backport automation to exclude changes to CHANGELOG.md to minimize the potential for merge conflicts?

[t-hamano]

Aside: Would it be reasonable for the backport automation to exclude changes to CHANGELOG.md to minimize the potential for merge conflicts?

Indeed, merge conflicts are a hassle, especially since more packages have been updating their CHANGELOGs recently. It's likely that the wp/X.X branch is not aligned with the npm packages, so we can probably exclude changes to the CHANGELOG.