-
Notifications
You must be signed in to change notification settings - Fork 4.6k
SVG Icon registration API #72215
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: trunk
Are you sure you want to change the base?
SVG Icon registration API #72215
Conversation
| $icons_directory = __DIR__ . '/../packages/icons/src/library/'; | ||
| if ( ! is_dir( $icons_directory ) ) { | ||
| return; | ||
| } | ||
|
|
||
| $svg_files = glob( $icons_directory . '*.svg' ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In WordPress core, the problem is how to handle this. For example, it might be necessary to synchronize the SVG icon of packages/icons/src/library with wp-includes/icons/.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
First step would be #72299, after which we can open a Trac ticket extending the copy logic in tools/webpack/packages.js.
| * } | ||
| * @return bool True if the icon was registered with success and false otherwise. | ||
| */ | ||
| private function register( $icon_name, $icon_properties ) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Currently, we do not allow external icon registration, which means that even if a developer runs the following code, an error will occur:
WP_Icons_Registry_Gutenberg::get_instance()->register();| * @return string The sanitized icon SVG content. | ||
| */ | ||
| private function sanitize_icon_content( $icon_content ) { | ||
| $allowed_tags = array( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Borrowed the logic from the Twenty Twenty theme.
| 'name' => $icon_name, | ||
| 'content' => $svg_content, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For actual use in the Icon Block, we may also need the label field.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One outstanding question for me is whether we'd like to properly annotate SVGs with metadata like title/label and, maybe, keywords (e.g. to match "edit" to the pencil icon). That would be better than programmatically adjusting the case and spacing.
And, independently of the above, all the labels (and possibly keywords) would need to exist statically in the code in order for i18n to be possible. In its simplest form, a build step in wpdev might, for instance, generate a PHP file with these strings. There is also JSON-based i18n (developed for block.json), in case we'd like to settle on defining metadata via JSON, although I'm less familiar with how that works.
| if ( current_user_can( 'edit_posts' ) ) { | ||
| return true; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is possible to expose icons externally, but for now this is restricted. External icons are rendered by the WP_Icons_Registry class.
lib/rest-api.php
Outdated
| function gutenberg_register_icon_controller_endpoints() { | ||
| $icons_registry = new WP_REST_Icon_Controller_Gutenberg(); | ||
| $icons_registry->register_routes(); | ||
| } | ||
| add_action( 'rest_api_init', 'gutenberg_register_icon_controller_endpoints' ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is to override the endpoints of the WP_REST_Icon_Controller class that will be implemented in core, allowing us to continue developing the endpoints on the Gutenberg plugin.
|
Flaky tests detected in 62d53f4. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/18378781319
|
|
Ok, except for preparing the core backports PR, I think this PR is ready for review. |
|
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. |
mcsf
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@t-hamano: this looks pretty good for a first PR! It's been a few months since we last discussed this. Do you feel it's missing anything?
| 'gutenberg-experiments', | ||
| 'gutenberg_experiments_section', | ||
| array( | ||
| 'label' => __( 'Enables the SVG Icon registarion API.', 'gutenberg' ), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| 'label' => __( 'Enables the SVG Icon registarion API.', 'gutenberg' ), | |
| 'label' => __( 'Enables the SVG Icon registration API.', 'gutenberg' ), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggestion aside, I'm not totally sure we need the experiment. I suppose it depends on the initial scope of the registry. The more locked down it is, the less the need for an experiment gate, and vice versa.
Edit: OK, I think it's fair to start conservatively with the experiment, as long as we don't forget to remove it as soon as possible. :)
| $icons_directory = __DIR__ . '/../../packages/icons/src/library/'; | ||
| if ( ! is_dir( $icons_directory ) ) { | ||
| return; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Preliminary thoughts on making this work reliably across Gutenberg and Core:
- Write a build step in wordpress-develop to copy icons from the Icons package to somewhere in wp-includes (I have a Git stash back from October that I need to resume and share).
- In the Core backport, replace the hardcoded path (
packages/icons/src/library) with a hardcoded path underwp-includes. The problem here is that users who will be simultaneously using 7.0 and Gutenberg won't see new icons added by the plugin.- This could be solved using a filter (e.g.
wp_icons_registry_directory), but the problem there is that the filter is unavoidably "public" and can be abused. - Also using a filter, but rescinding even more control, we could do as in
parse_blocks:$registry_class = apply_filters( 'icons_registry_class', 'WP_Icons_Registry' ); new $registry_class(); - This sounds awful, but I'll share for the sake of completeness: we could hardcode a Gutenberg dependency in the Core class, e.g.
if Gutenberg is active then path := wp-content/gutenberg/... else path := wp-include/.... - We could do some creative "hook juggling" so that Gutenberg can define the class before Core, but I always worry about the fragility of these setups (and, if I remember correctly, we had some issues in the early 5.0 days).
- Am I missing a better solution? If not, at the moment I believe the
wp_icons_registry_directoryfilter is the best one.
- This could be solved using a filter (e.g.
| $allowed_tags = array( | ||
| 'svg' => array( | ||
| 'class' => true, | ||
| 'xmlns' => true, | ||
| 'width' => true, | ||
| 'height' => true, | ||
| 'viewbox' => true, | ||
| 'aria-hidden' => true, | ||
| 'role' => true, | ||
| 'focusable' => true, | ||
| ), | ||
| 'path' => array( | ||
| 'fill' => true, | ||
| 'fill-rule' => true, | ||
| 'd' => true, | ||
| 'transform' => true, | ||
| ), | ||
| 'polygon' => array( | ||
| 'fill' => true, | ||
| 'fill-rule' => true, | ||
| 'points' => true, | ||
| 'transform' => true, | ||
| 'focusable' => true, | ||
| ), | ||
| ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How did you arrive at this ruleset? By inspecting current icons? How confident can we be about it?
Edit: ah, I see from #72215 (comment) that it's from Twenty Twenty. We should add a comment to make that explicit.
| 'properties' => array( | ||
| 'name' => array( | ||
| 'description' => __( 'The icon name.', 'gutenberg' ), | ||
| 'type' => 'string', | ||
| 'readonly' => true, | ||
| 'context' => array( 'view', 'edit', 'embed' ), | ||
| ), | ||
| 'content' => array( | ||
| 'description' => __( 'The icon content (SVG markup).', 'gutenberg' ), | ||
| 'type' => 'string', | ||
| 'readonly' => true, | ||
| 'context' => array( 'view', 'edit', 'embed' ), | ||
| ), | ||
| ), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is fine to start with, but let's make a mental note about properties to add soon:
- translatable label
- URL to SVG file
- internal URI to SVG file
- keywords
- (maybe) categories
What?
This PR is an attempt to implement an official API for registering and exposing SVG icons in WordPress.
Part of #16484
Blocker for #71227
Implementation details
This API basically consists of the following two classes:
WP_Icons_Registry
This class is very similar to
WP_Block_Patterns_Registry. For now, I registered all SVG icons that exist in@wordpress/iconspackage.The initial implementation will not allow external icon registration, but in the future code like this will work:
Registered icons can be rendered using code like this:
WP_REST_Icon_Controller
Expose a list of icons registered via the icon registry.
Testing Instructions
REST API
Get all icons
Search icons
Get only specific fields
Get a specific icon
Frontend Rendering
The following code is an example that filters the Paragraph block and outputs one of the registered icons.
Future plans
This is just my personal guess, any feedback is welcome.
Icons as a post type
wp_icon. The SVG data is saved in the post content.WP_Icon_Categories_Registryclass.Icons Library
Customizable icons for core blocks
For example, you can change the navigation toggle icon however you like using the Icon API.
Inline format
Format for injecting SVG elements into RichText
ToDo