Opened 4 months ago
Last modified 82 minutes ago
#64847 new enhancement
SVG Icon API: Iteration for WordPress 7.1
| Reported by: | wildworks | Owned by: | |
|---|---|---|---|
| Priority: | normal | Milestone: | 7.1 |
| Component: | Editor | Version: | |
| Severity: | normal | Keywords: | has-patch has-unit-tests gutenberg-merge |
| Cc: | Focuses: |
Description
Gutenberg Issue: https://github.com/WordPress/gutenberg/issues/75715
This ticket aims to ensure that iterative improvements to the Icon API are properly backported from Gutenberg to core.
Change History (50)
This ticket was mentioned in PR #11235 on WordPress/wordpress-develop by @wildworks.
4 months ago
#1
- Keywords has-patch has-unit-tests added
This ticket was mentioned in PR #11245 on WordPress/wordpress-develop by @im3dabasia1.
4 months ago
#3
@im3dabasia1 commented on PR #11245:
4 months ago
#4
Thanks for the continuous iterations on this PR. I have addressed the feedbacks in this commit https://github.com/WordPress/wordpress-develop/pull/11245/commits/bf30902e5dab0e8a32c1a4c8088c1c9bf6a27745
@wildworks commented on PR #11245:
3 months ago
#5
@im3dabasia Since https://github.com/WordPress/gutenberg/pull/76079 has now been merged, can you update this PR accordingly?
@wildworks commented on PR #11245:
3 months ago
#6
@im3dabasia Thanks for the update! As we are currently in the phase of preparing for the 7.0 release, let's commit this PR after the 7.0 release.
This ticket was mentioned in PR #11559 on WordPress/wordpress-develop by @wildworks.
3 months ago
#7
Trac ticket: https://core.trac.wordpress.org/ticket/64847
## Use of AI Tools
Claude Opus 4.6 (1M context)
@wildworks commented on PR #11559:
3 months ago
#8
I am encountering the following error.
Fatal error: Access level to Gutenberg_Icons_Registry_7_1::register() must be public (as in class WP_Icons_Registry) in /var/www/src/wp-content/plugins/gutenberg/lib/compat/wordpress-7.1/class-gutenberg-icons-registry-7-1.php on line 68
This issue will likely be resolved once https://github.com/WordPress/gutenberg/pull/77260 is merged, and the Gutenberg ref is updated in wordpress-develop.
@wildworks commented on PR #11245:
8 weeks ago
#9
Trunk is now WordPress 7.1-alpha, so we can move this PR forward.
@im3dabasia, can you address @mukeshpanchal27's review and update this PR?
This ticket was mentioned in PR #12010 on WordPress/wordpress-develop by @wildworks.
6 weeks ago
#10
The Icons API introduced in 7.0 exposes the modern WordPress icon set through WP_Icons_Registry and the REST API, but PHP rendering contexts (themes, plugins, block render callbacks) still have no first-class way to output those icons and tend to fall back to the deprecated dashicons font.
This adds wp_get_icon(), a procedural wrapper around WP_Icons_Registry that returns a registered icon as an inline SVG string, mirroring the React <Icon> component so both paths share the same SVG sources.
The function resolves the icon content via the registry and uses WP_HTML_Tag_Processor to apply a configurable size and class, plus accessibility handling: passing a label produces role="img" and aria-label, while omitting it marks the icon aria-hidden="true".
It lives in a new src/wp-includes/icons.php, following the existing registry-class-plus-procedural-API pairing already used by connectors.php and abilities.php.
Based on WordPress/gutenberg#78332.
## Use of AI Tools
AI assistance: Yes
Tool(s): Claude Code
Model(s): Claude Opus 4.8
Used for: Porting the function and PHPUnit tests from the referenced Gutenberg PR to core conventions. I reviewed and tested the result; all 9 tests pass.
This ticket was mentioned in PR #12149 on WordPress/wordpress-develop by @wildworks.
5 weeks ago
#11
Mirrors the same rename made on the Gutenberg side in https://github.com/WordPress/gutenberg/pull/79100.
Trac ticket: https://core.trac.wordpress.org/ticket/64847
## Use of AI Tools
Used for: Initial code skeleton and test suggestions; final implementation and tests were reviewed and edited by me.
This ticket was mentioned in PR #12151 on WordPress/wordpress-develop by @wildworks.
5 weeks ago
#12
- Depends on: https://github.com/WordPress/wordpress-develop/pull/12149
- Trac ticket: https://core.trac.wordpress.org/ticket/64847
- Gutenberg PR: https://github.com/WordPress/gutenberg/pull/79100
## Use of AI Tools
Used for: Initial code skeleton and test suggestions; final implementation and tests were reviewed and edited by me.
@wildworks commented on PR #11245:
5 weeks ago
#13
@im3dabasia, can you address @mukeshpanchal27's review and update this PR?
I believe we're ready to commit now, as I have addressed this feedback.
@mukesh27 commented on PR #12149:
4 weeks ago
#14
Do you have plan to work on WP_Block_Patterns_Registry as it also have filePath?
@wildworks commented on PR #12149:
4 weeks ago
#15
Do you have plan to work on WP_Block_Patterns_Registry as it also have
filePath?
No. I think it should have been file_path instead of filePath, but it's too late now because it would affect backward compatibility. The reason for only trying the change in the icon registry is that backward compatibility doesn't need to be considered. See https://github.com/WordPress/gutenberg/pull/79100#discussion_r3407921418
@wildworks commented on PR #12010:
4 weeks ago
#16
@sirreal @tyxla Thanks for the feedback! To avoid duplicate reviews, it might be better to focus reviews on the Gutenberg PR. Once the Gutenberg PR is ready for shipping, I'd like to reflect that code into this PR.
@wildworks commented on PR #11235:
4 weeks ago
#17
@mcsf, Do you think this is ready to be committed?
This ticket was mentioned in PR #12197 on WordPress/wordpress-develop by @wildworks.
4 weeks ago
#18
- Trac ticket: https://core.trac.wordpress.org/ticket/64847
- Gutenberg PR: https://github.com/WordPress/gutenberg/pull/75550
## Use of AI Tools
Used for: Initial code skeleton and test suggestions; final implementation and tests were reviewed and edited by me.
@wildworks commented on PR #11245:
4 weeks ago
#20
This change has been committed in r62515
@wildworks commented on PR #12149:
3 weeks ago
#21
https://github.com/WordPress/gutenberg/pull/79100 has been merged. I'd like to commit to this accordingly as it aligns with that.
@wildworks commented on PR #12149:
3 weeks ago
#23
This PR has been committed as r62540.
@wildworks commented on PR #12010:
3 weeks ago
#24
Update: The Gutenberg PR has been approved. This PR includes all changes for Gutenberg, but please let me know if there are any other points that need to be fixed.
@wildworks commented on PR #11235:
3 weeks ago
#26
This PR was committed by r62551.
@wildworks commented on PR #12151:
3 weeks ago
#27
I'd like to close this PR as introducing the "show_in_rest" flag is not ideal. Please refer to the comments below for details.
@wildworks commented on PR #11559:
2 weeks ago
#28
Update: The Gutenberg PR has been approved. This PR includes all changes for Gutenberg, but please let me know if there are any other points that need to be fixed.
#29
@
2 weeks ago
This currently causes an issue on wp.org as that runs a trunk checkout of WP, alongside the latest release of Gutenberg (currently 23.4.0). While this will be resolved on the next release of Gutenberg, we should probably avoid creating fatals in these types of scenarios.
#30
@
2 weeks ago
@paulkevan I apologize for the inconvenience. I was able to reproduce the issue in my environment as well. This error likely occurs when opening the icon picker in the icon block, correct?
PHP Warning: Undefined array key "file_path" in /wordpress/wp-includes/class-wp-icons-registry.php on line 265
My prediction is that committing r62540 before Gutenberg 23.5 is released caused this issue. In other words, the core commit might have been too early.
I am aware that issues arising from such discrepancies have occurred multiple times in the past, but honestly, I don't know how to detect and prevent them completely beforehand. Nevertheless, I will strive to address similar issues if they arise in the future.
#31
@
2 weeks ago
The errors I were seeing were coming from front-end request on the pattern directory site, e.g https://wordpress.org/patterns/pattern/hero-section-with-image-grid-and-labels/. It looks to come from the pattern preview.
#32
@
2 weeks ago
@paulkevan This ticket is about the Icon API and should not involve patterns. How did you determine that the issue is related to this ticket?
#33
@
2 weeks ago
How did you determine that the issue is related to this ticket?
Live stack traces on wp.org, which point directly to the change of file_path in r62540.
#34
@
2 weeks ago
If r62540 caused the problem, then the cause would be as explained here. https://core.trac.wordpress.org/ticket/64847#comment:30
#35
@
13 days ago
Update: Currently, the following three PRs are still open.
Icons: Add APIs for collection and icon registration
There are three Gutenberg PRs linked to one core PR, and I am currently working on the third Gutenberg PR.
- Core PR: https://github.com/WordPress/wordpress-develop/pull/11559
- Gutenberg PR:
Icons API: Add wp_get_icon() to render registered icons as inline SVG
The core PR is almost identical to the Gutenberg PR and is ready for review.
- Core PR: https://github.com/WordPress/wordpress-develop/pull/12010
- Gutenberg PR (Merged): https://github.com/WordPress/wordpress-develop/pull/12010
Icons Registry: improve SVG sanitizer
We need to merge the Gutenberg PR first.
- Core PR: https://github.com/WordPress/wordpress-develop/pull/12197
- Gutenberg PR (Under review): https://github.com/WordPress/wordpress-develop/pull/12197
@wildworks commented on PR #12010:
6 days ago
#36
@tyxla @mcsf, I plan to commit this by the end of this week or early next week to make it in time for the beta 1 release. Please let me know if there is anything else I should address.
@wildworks commented on PR #11559:
6 days ago
#37
@tyxla @mcsf, I plan to commit this by the end of this week or early next week to make it in time for the beta 1 release. Please let me know if there is anything else I should address.
@wildworks commented on PR #11559:
6 days ago
#38
88f1834c2f9f5d9507b312146fb74aa5f5abaa73 is the improvements made in https://github.com/WordPress/gutenberg/pull/79927/. See https://github.com/WordPress/gutenberg/pull/79927/changes#r3534498017 for more details.
@wildworks commented on PR #11559:
4 days ago
#39
Please note that the CI check will fail in this PR because a critical error occurs when the Gutenberg plugin is enabled.
PHP Fatal error: Access level to WP_Icons_Registry_Gutenberg::register() must be public (as in class WP_Icons_Registry) in /var/www/src/wp-content/plugins/gutenberg/lib/class-wp-icons-registry-gutenberg.php on line 68 PHP Fatal error: Access level to WP_Icons_Registry_Gutenberg::register() must be public (as in class WP_Icons_Registry) in /var/www/src/wp-content/plugins/gutenberg/lib/class-wp-icons-registry-gutenberg.php on line 68
This is because, in Gutenberg 23.5, WP_Icons_Registry_Gutenberg::register() is still a protected method.
This CI error should be resolved if the Gutenberg ref is updated to version 23.6.
@mcsf commented on PR #11559:
4 days ago
#40
Please note that the CI check will fail in this PR because a critical error occurs when the Gutenberg plugin is enabled.
PHP Fatal error: Access level to WP_Icons_Registry_Gutenberg::register() must be public (as in class WP_Icons_Registry) in /var/www/src/wp-content/plugins/gutenberg/lib/class-wp-icons-registry-gutenberg.php on line 68 PHP Fatal error: Access level to WP_Icons_Registry_Gutenberg::register() must be public (as in class WP_Icons_Registry) in /var/www/src/wp-content/plugins/gutenberg/lib/class-wp-icons-registry-gutenberg.php on line 68This is because, in Gutenberg 23.5,
WP_Icons_Registry_Gutenberg::register()is still a protected method.
This CI error should be resolved if the Gutenberg ref is updated to version 23.6.
What can we do to mitigate the problem? We wouldn't want to break WP trunk for Gutenberg users. It also means that users that for some reason are stuck on Gutenberg 23.5 (and several older versions) by the time that WP 7.1 comes are at risk of fatals, right?
@wildworks commented on PR #11559:
4 days ago
#41
What can we do to mitigate the problem? We wouldn't want to break WP trunk for Gutenberg users. It also means that users that for some reason are stuck on Gutenberg 23.5 (and several older versions) by the time that WP 7.1 comes are at risk of fatals, right?
Yes. I think this error occurs if Gutenberg version 23.5 or lower is enabled on the WP trunk that includes this PR.
One approach might be to ship this PR in Beta 2, not Beta 1.
- July 14: Gutenberg 23.6 RC1 release
- July 15: WordPress Beta1 release
- July 22: Gutenberg 23.6 general release
- July 22: Commit this PR
- July 22: WordPress Beta2 release
This way, Gutenberg 23.6 should not cause any errors as it includes the public WP_Icons_Registry_Gutenberg::register() method at the WP Beta 2 stage.
This approach may not be acceptable because the release of the new feature, the Icon API, would be delayed by one week.
Another approach is to release a Gutenberg 23.5 minor version immediately, making the WP_Icons_Registry_Gutenberg::register() method public.
In any case, users need to use the latest Gutenberg to prevent critical errors. Do you have any other good ideas?
@mcsf commented on PR #11559:
3 days ago
#42
I think a quick minor Gutenberg release could make sense here, but I'll defer to @tyxla and @annezazu, especially since I'll be out for a few weeks!
@tyxla commented on PR #11559:
3 days ago
#43
I would be fine with a minor release for this.
FWIW I've been considering whether to have one for https://github.com/WordPress/gutenberg/pull/79894 so let's add that to the minor, too, if possible.
@annezazu commented on PR #11559:
3 days ago
#44
Ditto—totally happy with a minor release.
This ticket was mentioned in PR #12483 on WordPress/wordpress-develop by @wildworks.
2 days ago
#45
This PR is to verify that the CI errors occurring in https://github.com/WordPress/wordpress-develop/pull/11559 are resolved after the Gutenberg 23.5.2 release.
This PR is checked out from https://github.com/WordPress/wordpress-develop/pull/11559, and only the Gutenberg SHA has been updated. The selected SHA is from https://github.com/WordPress/gutenberg/pull/80139/, and by including this PR in Gutenberg 23.5.2, the CI errors occurring in https://github.com/WordPress/wordpress-develop/pull/11559 should be resolved.
@wildworks commented on PR #11559:
15 hours ago
#46
Another approach is to release a Gutenberg 23.5 minor version immediately, making the
WP_Icons_Registry_Gutenberg::register()method public.
In any case, users need to use the latest Gutenberg to prevent critical errors. Do you have any other good ideas?
Update: We plan to release Gutenberg 23.5.2 on Monday, July 13th.
I submitted https://github.com/WordPress/gutenberg/pull/80139 to make WP_Icons_Registry_Gutenberg::register() public and fix some warnings.
I locally built the PR, enabled the plugin on this branch, and confirmed that all errors disappeared 👍
@wildworks commented on PR #11559:
7 hours ago
#47
All CI checks are green! I believe the potential issues that arise when this pull request is committed will be resolved by the Gutenberg 23.5.2 release.
https://github.com/WordPress/wordpress-develop/actions/runs/29240244378/job/86784292003?pr=11559
@wildworks commented on PR #12010:
5 hours ago
#48
Unless there are other issues to address, I plan to commit this before the 7.1 Beta1 release.
@wildworks commented on PR #11559:
5 hours ago
#49
Unless there are other issues to address, I plan to commit this before the 7.1 Beta1 release.
![(please configure the [header_logo] section in trac.ini)](/chrome/site/your_project_logo.png)
Trac ticket: https://core.trac.wordpress.org/ticket/64847
## Use of AI Tools
Cursor