Icons: Add PHP method(s) for rendering inline SVG icons from the registry#78332
Icons: Add PHP method(s) for rendering inline SVG icons from the registry#78332jasmussen wants to merge 15 commits into
Conversation
|
Size Change: 0 B Total Size: 7.96 MB ℹ️ View Unchanged
|
c40a915 to
19ea189
Compare
|
Thanks for the PR! I think this is a great suggestion. In my opinion, generating PHP for icons is redundant because the Icons Registry already holds the data for registered icons. This data should be parsable using the HTML Tag Processor, allowing for the injection of arbitrary attributes. So, the function wp_icon( $icon_name, $args = array() ) {
// Default parameters.
$args = wp_parse_args(
$args,
array(
'size' => 24,
'label' => '',
'class' => '',
)
);
// Check if an icon is registered in the Icons Registry.
$icon = WP_Icons_Registry::get_instance()->get_registered_icon( $icon_name );
if ( is_null( $icon ) ) {
return '';
}
$processor = new WP_HTML_Tag_Processor( $icon['content'] );
// Data is not considered valid if it does not contain an SVG element.
if ( ! $processor->next_tag( 'svg' ) ) {
return '';
}
// Inject the `size` attribute.
$processor->set_attribute( 'width', (string) $args['size'] );
$processor->set_attribute( 'height', (string) $args['size'] );
// Inject the `aria-label` atribute.
if ( ! empty( $args['label'] ) ) {
$processor->set_attribute( 'aria-label', $args['label'] );
}
// Inject additional CSS classes.
if ( ! empty( $args['class'] ) ) {
$processor->add_class( $args['class'] );
}
return $processor->get_updated_html();
} |
|
Solid feedback. In 0d2c4f5 the bot and I, we, whatever, pushed changes based on this, which drastically reduces the complexity of the PR. On my mind in this effort is to create a new PHP way to insert icons that mimicks as closely as possible the React based one. It seems entirely useful to use the registry, I really like the idea that the function could also reference icons registered by third party plugins. One key aspect I'd like to maintain, though, is that if you know the name of an icon, you should be able to insert it even if it hasn't been flagged "public". Feel free to clarify any nuance I'm missing in this, but the motivation really is to further the admin refresh that has been started in WordPress 7.0: to actually use this icon set in all of WordPress to unify towards a single icon set. Let me know your thoughts on the latest diff! |
|
Flaky tests detected in 1e6a9b6. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/27201882507
|
t-hamano
left a comment
There was a problem hiding this comment.
Thanks for the update. One important consideration is what to name the function.
wp_icon();
the_wp_icon();Upon investigating the core, there are no function names with the the_wp_ prefix.
My proposal is the following:
wp_get_icon();
wp_icon();|
Fantastic feedback, thank you for your time. On the function names, looks like I got it wrong in a few ways. I was following this standard, but incorrectly. In any case, should be addressed now, along with the majority of your other feedback. The last bit I'm happy to address also, but let me ask some clarifying questions just to be sure I'm reading it right.
Part of the motivation of this PR, also, is to allow us to use WordPress icons in core pages, as part of the admin in both legacy and new PHP pages. Consider the left admin menu that currently uses deprecated dashicons. I would like for the new library to be made available so that menu can be updated, so we can unify on a single icon set for WordPress. I believe this requires that at least WordPress be able to access private icons. Much of the motivation for making icons private, was specifically for frontend usage, inside the Icon block. In that case, as soon as an icon is used through that block on a website, it becomes substantially harder to edit, redesign, or remove an icon from the set. Which, it isn't something we want to do often, nevertheless a lot of icons do deserve some love now that the larger visual vocabuly of our icons has evolved. Question: if we remove this fallback, does that mean we'll only be able to use public icons in PHP pages? I don't mind breaking this up in a few PRs for ease of review and so on, but I would love for us to be able to access the full set in admin contexts, just like you can access the full set in React contexts. Let me know if I'm missing nuance! |
|
Regarding the function name, there are probably two options. wp_get_icon();
wp_icon();or get_the_icon();
the_icon();However, without the "wp" prefix, there is a high chance of conflict with functions created by consumers. Nowadays, it is common practice to include the "wp" prefix.
Yes. If my understanding is correct, the reason we introduced the "public" status for icons was to prevent users from selecting icons for dashboards within the Icon block. However, I believe this status can be removed in the future. This means that all icons in |
I think we'll probably want to maintain that status, I think it's useful for the Icon block. What I was suggesting was a more clear delineation of frontend vs. backend—that if used on the frontend we don't want it to break. However you reminded me that as soon as this becomes available to third party plugins frontend vs. backend doesn't make a difference, the support story remains the same. Good point. So let me rephrase the question instead: will there be a way for the dashboard—for WordPress core itself—to use "private" icons in PHP pages? And if yes, should that be in this PR or a separate one? |
Unfortunately, there is no such way. When creating the manifest file that serves as the basis for synchronizing icons from Gutenberg to the core, only public icons are included, excluding all others. This means that private icons are not bundled with the WordPress core itself. Therefore, the concept of a "private icon" does not exist within the WordPress core itself. To lift this restriction, we would need to modify the core and Gutenberg synchronization process itself, so we recommend considering it in a separate PR. |
|
Sounds good. If I read this right, the PR should be ready for broader feedback (thanks again for your time), and is now limited by which icons are public. I'm happy to also work on followup projects that help us further to one of the goals of making icons available to PHP pages, whether that means moving some icon-omission logic to the Icon block directly, or perhaps "graduating" more icons to being public. E.g. maybe we have most of what we need for core already, with what's public? |
| * 'core/arrow-down', 'my-plugin/custom-icon'). | ||
| * @param array $args Optional. Arguments for the icon. See wp_get_icon() for details. | ||
| */ | ||
| function wp_icon( $name, $args = array() ) { |
There was a problem hiding this comment.
Is this function really necessary? I feel like wp_get_icon() is enough, and this wrapper is completely redundant. Is this to avoid the need to deal with escaping upon echoing the icons in consumer code (considering that wp_get_icon() uses the HTML API and produces safe code)?
There was a problem hiding this comment.
From this iteration, it was to match other template functions that usually come in one flavor that gets, and one that echoes. But I'm entirely happy to start with purely the get version. We can always add more if people ask for it.
There was a problem hiding this comment.
+1, starting from purely get version.
I can’t finding now, but there’s a proposal for allowing short PHP template syntax, which makes echoing values in templates easier.
There was a problem hiding this comment.
Here's the proposal I mentioned - https://make.wordpress.org/core/2025/12/05/coding-standard-proposal-allow-the-use-of-the-php-short-echo-tag/.
Having a separate echo templates function just saves a couple of characters and is probably not worth including initially. We can add them later if we see enough requests.
There was a problem hiding this comment.
Thanks for addressing it @jasmussen 🙌
@Mamaduka I'm personally not a fan of short_open_tag, but I wouldn't mind if it were part of WPCS.
| * 'core/arrow-down', 'my-plugin/custom-icon'). | ||
| * @param array $args Optional. Arguments for the icon. See wp_get_icon() for details. | ||
| */ | ||
| function wp_icon( $name, $args = array() ) { |
There was a problem hiding this comment.
Naming question in case we chose to have this function: if we have wp_get_icon() (which makes perfect sense to me), why don't we also follow suit and name this wp_render_icon()?
There was a problem hiding this comment.
This was also to follow classic formula, e.g. the_title and get_the_title, however I guess those are more template tags than what this function is meant for, so for the moment I'm just going to keep wp_get_icon, but let me know if you have any preference otherwise.
|
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. |
| * @return string SVG markup for the icon, or empty string if not found. | ||
| */ | ||
| function wp_get_icon( $name, $args = array() ) { | ||
| $icon = WP_Icons_Registry::get_instance()->get_registered_icon( $name ); |
There was a problem hiding this comment.
What do we know about the registered icons? If this is a public API where arbitrary SVG content (that passes KSES checks) may be in the icon, things can get very complicated. One that comes to mind is if the SVG has a viewBox, then a single size argument may be tricky. Are icons required to be square? Is that enforced?
And just to confirm, anything in the registry has passed through KSES, right?
| if ( ! empty( $args['class'] ) ) { | ||
| $processor->add_class( $args['class'] ); | ||
| } |
There was a problem hiding this comment.
Is this a single class, or could it be multiple classes? The ::add_class method expects a single class, and multiple classes should be applied individually.
Something like $processor->add_class( 'wp-icon class2 class3' ) is not how the method is intended to be used.
| if ( ! empty( $args['label'] ) ) { | ||
| $processor->set_attribute( 'role', 'img' ); | ||
| $processor->set_attribute( 'aria-label', $args['label'] ); | ||
| } else { | ||
| $processor->set_attribute( 'aria-hidden', 'true' ); | ||
| } |
There was a problem hiding this comment.
Consider that the svg content may already have these attributes, so if one branch should have role and aria-label but not aria-hidden, and vice-versa for the other branch, then it's a good idea to remove the attributes from the other branch so we don't wind up with:
<svg aria-hidden="true" role="img" aria-label="whoops?" />| if ( ! empty( $args['label'] ) ) { | |
| $processor->set_attribute( 'role', 'img' ); | |
| $processor->set_attribute( 'aria-label', $args['label'] ); | |
| } else { | |
| $processor->set_attribute( 'aria-hidden', 'true' ); | |
| } | |
| if ( ! empty( $args['label'] ) ) { | |
| $processor->set_attribute( 'role', 'img' ); | |
| $processor->set_attribute( 'aria-label', $args['label'] ); | |
| $processor->remove_attribute( 'aria-hidden' ) | |
| } else { | |
| $processor->set_attribute( 'aria-hidden', 'true' ); | |
| $processor->remove_attribute( 'role' ); | |
| $processor->remove_attribute( 'aria-label' ) | |
| } |
(Do check me and test this, consider the suggestion pseudo-code)
| $processor->set_attribute( 'width', (string) $args['size'] ); | ||
| $processor->set_attribute( 'height', (string) $args['size'] ); |
There was a problem hiding this comment.
What if we get something other than int, is that possible? The string "10%", or other less sensical values: [ '9', '9' ], "nonsense".
There was a problem hiding this comment.
Yeah, came here to ask some of the same. Also, what about negative integers - should we make sure it's not the case by calling absint() on it or something?
tyxla
left a comment
There was a problem hiding this comment.
The PR description seems a bit out of date and since the PR doesn't do exactly what is suggested in the description, I wasn't able to verify the intent against the implementation. Would be nice to get an updated PR description to be sure.
Also left a few more questions and suggestions. @sirreal's questions around KSES are reasonable to be addressed too.
I generally think this is pretty close, nice work!
| $processor->set_attribute( 'width', (string) $args['size'] ); | ||
| $processor->set_attribute( 'height', (string) $args['size'] ); |
There was a problem hiding this comment.
Yeah, came here to ask some of the same. Also, what about negative integers - should we make sure it's not the case by calling absint() on it or something?
Introduce a `wp_icon()` function that lets PHP code render any icon from the modern WordPress icon set as inline SVG — same source files, same output shape in the DOM as the existing React `<Icon>` component. A build-time script reads all 331 SVG sources and generates a PHP array mapping slugs to their inner SVG content. At runtime, `wp_icon()` loads this manifest once and composes the SVG string per call with no file I/O, parsing, or DOM mutation. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Move icons.php from lib/compat/wordpress-7.0/ to lib/ since this is a new API, not a compat shim for a specific release. Remove stroke_width parameter to keep the initial API surface minimal. Fix heading casing in README to use sentence case. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The generated PHP manifest must be checked into the repo (matching the convention used by manifest.php) because the PHP runtime needs it available without a prior npm build step — wp-env mounts the repo directly and CI's PHP tests may not rebuild every package. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Remove extra padding spaces between array keys and double arrows in the generated PHP file to satisfy WordPress coding standards (PHPCS). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Rework wp_icon() to check the Icons Registry first (covers public and third-party icons), then fall back to reading the SVG file directly for non-public core icons. Uses WP_HTML_Tag_Processor for attribute manipulation, matching the pattern in the Icon block's index.php. This removes the generated icons-data.php manifest and its build script entirely, addressing review feedback. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…p.cjs Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The processor lowercases attribute names (viewBox → viewbox) and preserves trailing newlines from SVG files. Adjust assertions to match actual output. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Rename wp_icon()/the_wp_icon() to wp_get_icon()/wp_icon() per WP conventions - Move from lib/icons.php to lib/compat/wordpress-7.1/icons.php - Require explicit namespaced icon names (e.g. 'core/plus') - Remove forced fill="currentColor" so consumers can apply custom colors - Remove PHP usage section from @wordpress/icons README (not its concern) - Remove file fallback and stale test for non-public icon loading Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
wp_get_icon() already produces safe output via WP_HTML_Tag_Processor, so callers can echo it directly without needing a dedicated wrapper. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Drop phpunit/class-wp-icon-test.php. The wp_get_icon() helper in lib/compat/wordpress-7.1/icons.php remains, but this standalone test class is no longer carried on the branch. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Record the WordPress core backport (wordpress-develop#12010) tracking the icons work from Gutenberg PR #78332. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-authored-by: Marin Atanasov <8436925+tyxla@users.noreply.github.com>
- Drop the unconditional wp-icon class so output matches the icon block and React paths. - Allow size=0 to leave the SVG's intrinsic dimensions untouched. - Split the class argument on whitespace so multi-class strings work with WP_HTML_Tag_Processor::add_class(). - Add a wp_icon_html filter so the markup can be modified by plugins. - Refactor render_block_core_icon() to call wp_get_icon() for the SVG + aria/label/focusable/class handling; the block only layers style engine CSS on top now. - Restore the WP_Icon_Test unit tests, updated for the new behavior plus coverage for the filter, multi-class, and size=0. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
4fbfe65 to
fe8ff6f
Compare
|
Thanks for the review, @tyxla. With the help of Claude—as this is getting into somewhat complex territory for myself, so buyer beware—I tried my best to address your feedback, and update the PR description. The feedback from @sirreal I haven't addressed yet, as I'm still not sure whether that should happen in this PR or not. Let me know if things are on a right track, and also feel free to push changes directly. Thanks for all the help. |
Yes, sorry for posting two places! I don't have a strong opinion on where it should happen. A lot of my feedback were genuine questions that need some thought, I don't necessarily have answers 🙂 |
Same for my feedback, FWIW. Sorry if it felt like I was rushing you to go ahead and just do it. There are genuinely many pieces here that need to be carefully thought out and reconciled. |
|
Much appreciated. I've tried to keep each change in separate PRs, and rebased along the way, so ideally we can extricate easily changes that were mostly proposals or thoughts to discuss whether to add or not. |
| * @param string $name The namespaced icon name. | ||
| * @param array $args The arguments passed to wp_get_icon(). | ||
| */ | ||
| return apply_filters( 'wp_icon_html', $html, $name, $args ); |
There was a problem hiding this comment.
| return apply_filters( 'wp_icon_html', $html, $name, $args ); | |
| return apply_filters( 'wp_get_icon', $html, $name, $args ); |
In Core conventions, it seems to be common to use the same filter name as the function name when the function applies a filter to the data it ultimately returns.
| * @type int $size Width and height in pixels. Pass 0 to leave the | ||
| * SVG's intrinsic dimensions untouched. Default 24. |
There was a problem hiding this comment.
| * @type int $size Width and height in pixels. Pass 0 to leave the | |
| * SVG's intrinsic dimensions untouched. Default 24. | |
| * @type int|int[] $size Width and height in pixels. Accepts a single integer | |
| * for a square icon, or an array of array( $width, $height ) | |
| * for non-square dimensions. Pass 0 to leave the SVG's | |
| * intrinsic dimensions untouched. Default 24. |
This is not mandatory, but we might also support an array of width and height in addition to integer values. I believe this is similar to the core implementation.
wp_get_icon(
'core/plus',
array(
'size' => 32,
'label' => 'Add item',
)
);
wp_get_icon(
'core/plus',
array(
'size' => array( 24, 48 ),
'label' => 'Add item',
)
);
What
Adds a
wp_get_icon()PHP helper that returns the SVG markup for any icon in the WordPress icon registry, with optional sizing, classes, an accessible label, and awp_icon_htmlfilter.The
core/iconblock's server-side renderer is updated to usewp_get_icon()instead of duplicating the same aria/label/class logic inline.Why
There's no PHP-side equivalent to the React
<Icon>component. Themes, blocks, and plugins that render in PHP have to either re-fetch SVG files from disk or hand-roll their own icon-rendering logic, which leads to inconsistent output (missing aria attributes, nofocusable="false"on decorative icons, etc.).A single helper makes it cheap to render the same icon set consistently from PHP and React, and gives
render_block_core_icon()and any other core/plugin PHP path one place to evolve.How
wp_get_icon( $name, $args ):WP_Icons_Registry.WP_HTML_Tag_Processorto setwidth/height(or skip whensizeis0), add CSS classes, and apply accessibility attributes:label:role="img"+aria-label.label:aria-hidden="true"+focusable="false"(matchesrender_block_core_icon()and the React<Icon>component).wp_icon_htmlfilter so plugins can wrap or annotate the output.render_block_core_icon()is refactored to callwp_get_icon()for the core SVG + aria/class handling, then layers in the block's color, border, spacing, and dimensions styles via the style engine.Testing instructions
From any PHP context (a theme template, a custom block, etc.), call:
Confirm:
<svg ... width="24" height="24" aria-hidden="true" focusable="false">…</svg>.label, output usesrole="img"+aria-labelinstead.size, width/height match.class, the class is added; a space-separated string adds each class individually.Add a filter:
Confirm the filter modifies the output.
Run the unit tests:
Regression check on the
core/iconblock (now refactored to usewp_get_icon()): insert the block, toggleariaLabel, apply color/border/width — confirm rendered output matchestrunk.Backport
WordPress core PR: WordPress/wordpress-develop#12010
Use of AI Tools
This PR was authored with Claude Code (Claude Opus 4.6). I tested and reviewed the code. Claude Code is credited via Co-Authored-By in the commit messages.