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 13 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 fa0f199. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/26029314674
|
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. |
a21f47f to
bf9f3c4
Compare
|
Personally, I think |
I would love help with that, thank you! |
Without a public stroke-width API surface on the Icon component, exposing a control for it in Storybook is misleading. The control can return as a follow-up once the icon registry and PHP support stabilise (see #75550, #78332). The square.svg convention update remains. Stroke-scaling at any size continues to work via vector-effect="non-scaling-stroke" in the source SVG. Co-Authored-By: Claude Opus 4.7 <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>
|
Update:
I think this PR is ready for additional feedback! |
There was a problem hiding this comment.
The PR description mentions that we also have the the_wp_icon() function, but I can't find any changes related to it in this PR. Was it intentionally left out, or is it planned to be included in a follow-up PR?
There was a problem hiding this comment.
The PR description is outdated. Actually, the_wp_icon() function will not be added for now. See #78332 (comment)
There was a problem hiding this comment.
Good catch, I've updated the description.
| if ( ! function_exists( 'wp_get_icon' ) ) { | ||
| /** | ||
| * Returns the SVG markup for a registered icon. | ||
| * |
There was a problem hiding this comment.
Curious to know why the @since annotation is not added in new functions?
There was a problem hiding this comment.
Thank you for the feedback, this was a mistake. I've added it back now.
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>
3110eb4 to
e77376c
Compare
What?
Adds a
wp_icon()PHP function that renders any icon from the modern WordPress icon set as inline SVG, giving PHP developers the same icons available to the React<Icon>component.Why?
The modern icon set is primarily consumable via React. PHP contexts still rely on the deprecated dashicons font. The intent of this PR is to bridge the gap with a minimal API that shares the same SVG sources, so both paths produce identical output.
This approach was also informed by exploring diffent paths forward for icons, that I ultimately rejected:
mask-imagewith data URIs) offers a simple API and recolouring viacurrentColor, but the SVG is opaque to the host CSS: you lose the ability to control stroke width, do multi-colour icons, or animate individual paths. That closes the door on the future capabilities we might want to preserve.<use href="wp-icons.svg#name">) has known CORS and path-resolution fragility: it works locally but breaks behind CDNs, in subdirectory installs, or on upload. An inline SVG sprite variant avoids CORS issues but makes styling and animation awkward across<use>boundaries, and adds API surface for marginal benefit over per-call string composition.How?
I worked with Claude Code to explore this option, which is divided in three pieces:
packages/icons/lib/generate-icons-php.cjs): Reads all SVG source files andmanifest.json, generatespackages/icons/build-php/icons-data.php, a static PHP array mapping each icon slug to itsviewBoxand inner SVG content. Runs as part ofnpm run buildin the icons package.wp_icon()and(the_wp_icon()lib/icons.php): Loads the generated manifest once (static-cached), looks up the requested icon, and composes an inline<svg>string with configurablesize,class, andlabel(for accessibility). No file I/O per call, no parsing, no DOM mutation.phpunit/class-wp-icon-test.php):9tests covering default attributes, custom size/class/label, accessibility (aria-hidden vs role="img"), XSS escaping, unknown icon handling, and the.the_wp_icon()echo helperUsage
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.