Change Icon Library asset location#11313
Conversation
This updates the build script to: - copy the icon library SVG files to `wp-includes/images/icon-library` instead of `wp-includes/icons` - Moves the `manifest.php` file to the `wp-includes/assets/` directory. - Gives the `manifest.php` file a more specific name: `icon-library-manifest.php` - Remove `gutenberg-` from the corresponding `grunt copy` tasks.
|
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 Core Committers: Use this line as a base for the props when committing in SVN: To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
Test using WordPress PlaygroundThe changes in this pull request can previewed and tested using a WordPress Playground instance. WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser. Some things to be aware of
For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation. |
desrosj
requested review from
aaronjorbin,
adamsilverstein,
johnbillion,
sirreal and
youknowriad
There was a problem hiding this comment.
Pull request overview
Updates WordPress core’s Icon Library asset layout so SVGs live under wp-includes/images/ (as images) and the generated manifest lives under wp-includes/assets/, aligning runtime lookups with the new build output locations.
Changes:
- Move Icon Library SVG output to
wp-includes/images/icon-libraryand rename/relocate the generated manifest towp-includes/assets/icon-library-manifest.php. - Update
WP_Icons_Registryto load the manifest and SVGs from their new locations (and remove the redundanttrailingslashit()call). - Adjust ignore/build-clean patterns to account for the new output directory and temporary cleanup of the old one.
Reviewed changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
src/wp-includes/class-wp-icons-registry.php |
Updates filesystem paths used by the icons registry to match the new manifest and SVG locations. |
Gruntfile.js |
Splits/renames copy tasks, changes destinations for SVGs and manifest, and adds a manifest transform to match the new SVG directory structure. |
.gitignore |
Updates ignored generated output path for the icon library SVG directory. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
I accidentally included "Fixed" instead of "See" in r62073. |
| 'copy:icon-library-images', | ||
| 'copy:icon-library-manifest', |
There was a problem hiding this comment.
Why drop the "gutenberg-" prefix, since it's kept for the other tasks?
There was a problem hiding this comment.
I'd like to remove gutenberg- entirely from these steps over time. While they come from the Gutenberg repository, Gutenberg = Core. In my opinion, if we are unable to reference each part of the code base by a descriptive name without a gutenberg- prefix, then we've done something wrong.
|
IIRC, I originally set svn:ignore to reflect the .gitignore settings for icons. Something for you to keep in mind when you commit. :) |
|
I'm going to commit this so that it's included in the silent beta 6 later today. |
|
Merged in r62077. |
|
Sorry for the late reply! All changes look good to me👍 |
The Icon Library image files are images, so they should live in the
wp-includes/images/directory.This updates the build script to:
wp-includes/images/icon-libraryinstead ofwp-includes/iconsmanifest.phpfile to thewp-includes/assets/directory.manifest.phpfile a more specific name:icon-library-manifest.phpgutenberg-from the correspondinggrunt copytasks.trailingslashit()call in the icon registry class.Trac ticket: Core-64393
Use of AI Tools
AI assistance: Yes
Tools: GitHub Copilot
Used for code review on the PR.
This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.