Icons: self declare color#79320
Conversation
Adds `fill="currentColor"` to the outer `<svg>` of every icon in
@wordpress/icons. This makes each icon self-describe its colour intent at
the source, so consumers don't need to inject `fill: currentColor` via CSS
to get the expected rendering.
Per-file diff is mechanical and uniform:
<svg ... viewBox="0 0 24 24">
becomes
<svg ... viewBox="0 0 24 24" fill="currentColor">
330 SVGs touched. Children are unaffected; child elements that declare
their own `fill` (e.g. `fill="none"` on stroke icons) keep it.
Adds an idempotent maintenance script that normalises source SVGs in
`packages/icons/src/library` so every icon self-declares its colour via
`fill="currentColor"` on the outer `<svg>` element.
Not wired into `main()` — this is a manual one-off, intended to be invoked
on demand:
node -e "require('./packages/icons/lib/generate-library.cjs').normalizeSourceIcons()"
Skips icons that already declare `fill=` as an attribute or have `fill`
declared inside a `style` attribute on the outer `<svg>` (e.g. stroke-based
icons using `style="fill: none"` to defeat 3rd-party CSS overrides).
jasmussen
added
[Type] Enhancement
|
Size Change: +864 B (+0.01%) Total Size: 7.51 MB 📦 View Changed
|
Mechanical follow-up to the source SVG change — Jest snapshots that render `@wordpress/icons` icons now reflect the new `fill="currentColor"` attribute on the outer `<SVG>` element. 21 snapshots refreshed across 12 test suites.
jasmussen
requested review from
a team,
ajitbohra,
ellatrix and
fabiankaegy
as code owners
|
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 If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
github-actions
Bot
added
[Package] Components
|
Flaky tests detected in f7e9eb7. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/27764668864
|
This reverts commit 740e8ad.
github-actions
Bot
added
[Package] DataViews
mirka
left a comment
mirka
left a comment
There was a problem hiding this comment.
Icon component compatibility
I checked the color behavior for all icons in all three Icon components, and they all consistently render in the currentColor.
Possible breakage scenarios
The one condition that makes this possibly breaking for consumers is when they are setting a non-currentColor fill in on a wrapper element, relying on inheritance. If they're setting it directly on the Icon component, or on svg, they're safe. I've documented this in the changelog.
New guidance for customizing icon colors
The new guidance for setting custom colors on icons is to use color, not fill. This is in anticipation for the changes in #78808, where fill will no longer work consistently.
Addressing the style overrides
There are a good number of places in Gutenberg where fill is set to control the icon color. Among those places:
- There are likely
fillstyles that can be removed, now that each icon sets itself tocurrentColor. This should be addressed separately in a follow up after #78808. - I anticipate a handful of regressions that will be triggered by the changes in #78808.
- I identified only one instance where the svg changes in this PR would cause a possible regression. That instance in
DataFormis addressed in this PR.
There was a problem hiding this comment.
Color behavior is now a feature of individual icons in @wordpress/icons, so the demonstration is moved to that package's stories.
| z-index: 1; | ||
| cursor: help; | ||
| fill: var(--wpds-color-foreground-content-error-weak); | ||
| color: var(--wpds-color-foreground-content-error-weak); |
There was a problem hiding this comment.
Based on a scan of the codebase, this is the only place that breaks due to these changes. It isn't a condition that appears in Gutenberg itself, but the regression can manifest when layout is panel-dropdown, and labelPosition is none.
| Before | After |
|---|---|
 |
 |
(This is possibly not a condition that anyone will ever hit, but we can fix it anyway.)
There was a problem hiding this comment.
I'm honestly also confused by the previous state of the code: the svg had fill: currentColor, but the outer element also specified a hardcoded fill?
|
I would appreciate a technical review from @WordPress/gutenberg-components. |
mirka
added
[Type] Breaking Change
ciampo
left a comment
ciampo
left a comment
There was a problem hiding this comment.
I scanned the codebase further and found another potential candidate for regression:
There are also a few other ancestor `fill` rules that should be carefully assessed as potential regressions:
gutenberg/packages/components/src/custom-select-control-v2/styles.ts
Lines 210 to 214 in 84dd3ad
I also investigated some potential cleanup opportunities (that can be done as follow-ups to keep the PR focused):
`fill: currentColor` cleanup
gutenberg/packages/global-styles-ui/src/style.scss
Lines 96 to 98 in 84dd3ad
gutenberg/packages/global-styles-ui/src/icon-with-current-color.tsx
Lines 20 to 28 in 84dd3ad
gutenberg/packages/media-fields/src/media_thumbnail/style.scss
Lines 47 to 49 in 84dd3ad
gutenberg/packages/components/src/tabs/styles.ts
Lines 218 to 222 in 84dd3ad
gutenberg/packages/global-styles-ui/src/style.scss
Lines 246 to 248 in 84dd3ad
`fill` -> `color` cleanup/migration
gutenberg/packages/dataviews/src/components/dataviews-filters/style.scss
Lines 127 to 157 in 84dd3ad
gutenberg/packages/dataviews/src/components/dataviews-filters/style.scss
Lines 302 to 309 in 84dd3ad
gutenberg/packages/edit-site/src/components/site-hub/style.scss
Lines 83 to 89 in 84dd3ad
gutenberg/packages/edit-site/src/components/add-new-template/style.scss
Lines 111 to 116 in 84dd3ad
gutenberg/routes/template-list/add-new-template/style.scss
Lines 111 to 116 in 84dd3ad
| z-index: 1; | ||
| cursor: help; | ||
| fill: var(--wpds-color-foreground-content-error-weak); | ||
| color: var(--wpds-color-foreground-content-error-weak); |
There was a problem hiding this comment.
I'm honestly also confused by the previous state of the code: the svg had fill: currentColor, but the outer element also specified a hardcoded fill?
mirka
force-pushed
the
update/icons-self-declare-color
branch
from
84dd3ad to
db7b33b
Compare
mirka
force-pushed
the
update/icons-self-declare-color
branch
from
db7b33b to
af778a4
Compare
I looked at this one too and I think it is safe to leave as-is. The rule targets the In this block manager context, that inherited color appears to be the same normal UI text color as
Considered 👍 |
mirka
added
has dev note
| * External dependencies | ||
| */ | ||
| import type { ReactElement } from 'react'; | ||
| import type { StoryFn } from '@storybook/react-vite'; |
There was a problem hiding this comment.
Can we please add "@storybook/react-vite": "^10.4.3" to devDependencies in packages/icons/package.json?
There was a problem hiding this comment.
@manzoorwanijk Is this not caught by one of your recent linting improvements for dependencies?
Digging into this a bit, I see we disable import/no-extraneous-dependencies for a bunch of files, including Storybook files like this one. I'm not sure the full context behind that, but my instinct is that feels wrong, and we could be catching these issues that way.
There was a problem hiding this comment.
Is this not caught by one of your recent linting improvements for dependencies?
This is a dev only import, not part of the bundle. That tool catches the missing dependencies in published bundles.
These types of issues will be caught automatically once we adopt #76195.
There was a problem hiding this comment.
I wish Node 24 upgrade would not block such things and we adopt #72143 to unblock the new npm features, but that will probably remain a wish forever 😄
There was a problem hiding this comment.
I still think those ESLint overrides are misguided and should be improved, and that would also help identify these issues.
I did a bit of digging and found that it stems back to #16969, with a comment "disabled [...] files for now [...] but we should revisit it really soon" but never happened, and likely expanded from there to exclude more files, maybe compounded by previous issues from #72136, and general apathy toward strict testing for "development" files.
5 participants
What?
Adds
fill="currentColor"to the outer<svg>tag of all 330 icons in @wordpress/icons, so every icon self-declares its colour intent at the source.As commit number two, to keep or delete as we decide, it adds a small
normalizeSourceIconshelper to packages/icons/lib/generate-library.cjs that performs this normalisation. The helper exists as a manual maintenance tool, to be invoked on demand when new icons are added.Why?
Follow-up to review feedback.
Delivered as a separate PR for ease of review. It's a big diff because it touches all icons, but it's largely mechanical, but large.
How?
The script skips any SVGs whose outer already declares fill= as an attribute, or declares fill inside a style attribute (e.g. stroke-based icons that use style="fill: none"). Children are untouched.
To re-run after adding new icons:
Testing Instructions
This PR was co-authored with help of Claude Opus 4.8.
✍️ Dev note for WP 7.1
Icon colors should be customized with
colorSince
@wordpress/iconsversion 15.0.0, each individual icon in the library now defines how they inheritcurrentColorinternally. By default, every icon renders using the current text color.If your code tints icons by setting
fill, switch tocolorinstead:This is more reliable because icons may use
fill,stroke, or a mix of both internally. Going forward,coloris the recommended way to tint icons.