Admin: replace dashicons in admin bar and sidebar with @wordpress/icons - #12270

Hi there! 👋

Thank you for your contribution to WordPress! 💖

It looks like this is your first pull request to wordpress-develop. Here are a few things to be aware of that may help you out!

No one monitors this repository for new pull requests. Pull requests must be attached to a Trac ticket to be considered for inclusion in WordPress Core. To attach a pull request to a Trac ticket, please include the ticket's full URL in your pull request description.

Pull requests are never merged on GitHub. The WordPress codebase continues to be managed through the SVN repository that this GitHub repository mirrors. Please feel free to open pull requests to work on any contribution you are making.

More information about how GitHub pull requests can be used to contribute to WordPress can be found in the Core Handbook.

Please include automated tests. Including tests in your pull request is one way to help your patch be considered faster. To learn about WordPress' test suites, visit the Automated Testing page in the handbook.

If you have not had a chance, please review the Contribute with Code page in the WordPress Core Handbook.

The Developer Hub also documents the various coding standards that are followed:

• PHP Coding Standards
• CSS Coding Standards
• HTML Coding Standards
• JavaScript Coding Standards
• Accessibility Coding Standards
• Inline Documentation Standards

Thank you,
The WordPress Project

Test using WordPress Playground

The 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

• All changes will be lost when closing a tab with a Playground instance.
• All changes will be lost when refreshing the page.
• A fresh instance is created each time the link below is clicked.
• Every time this pull request is updated, a new ZIP file containing all changes is created. If changes are not reflected in the Playground instance,
  it's possible that the most recent build failed, or has not completed. Check the list of workflow runs to be sure.

For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation.

Test this pull request with WordPress Playground.

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 props-bot label.

Unlinked Accounts

The following contributors have not linked their GitHub and WordPress.org accounts: @lucasmendes-design.

Contributors, please read how to link your accounts to ensure your work is properly credited in WordPress releases.

Core Committers: Use this line as a base for the props when committing in SVN:

Props fushar, wildworks, tyxla, mcsf, mciampini, aduth, peterwilsoncc, jeffpaul, fcoveram, keoshi, joen, afercia, ugyensupport, mrwweb.

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

cc-ing (maybe) interested parties :) @t-hamano @jasmussen @tyxla @scruffian @lucasmendes-design

These all appear to be solid like-for-like replacements, though I feel least supportive of the Sites icon replacement on multisite. Perhaps a new icon could/should be generated there or review other icons for something besides a 2x2 table sort of icon? Of the existing icons, the one that feels like what I'd recommend otherwise is one that looks like a "two child node branched to the right" (sorry GitHub is giving me problems trying to upload a screenshot from my cell connection).

@jeffpaul Thanks for checking!

I feel least supportive of the Sites icon replacement on multisite.

Yep, as noted in the PR description, I'm not sure about this as well.

Of the existing icons, the one that feels like what I'd recommend otherwise is one that looks like a "two child node branched to the right"

You mean core/blockMeta? Indeed this looks more fitting. 😄

But my worry is that the icon slug is not related to "site" at all. I would expect there is something like core/sites icon. Perhaps it's better if we create a new such icon. cc: @jasmussen, @lucasmendes-design (I know you're all on vacation, feel free to respond when you're back.) @fcoveram

Another idea is to use this icon, to represent a "site switcher":

A few things from my end as I'm looking at this work:

• This is exciting and a much-awaited overhaul of icons, love it!
• It has the potential to have an impact on many plugins, plugin infrastructures and products that rely on the current icon set - something to be mindful about
• I don't see enough feedback from @WordPress/gutenberg-design on this, and I believe this is a hard requirement
• I believe it's a bit late in the 7.1 cycle for this to move forward, but it makes sense to get it to the finish line on time for landing early in the 7.2 cycle. It's paramount to give plugins and extenders enough time for testing and adopting the necessary changes.
• An angle that doesn't seem explored is trying this out as a Gutenberg experiment - not only to allow for safe iteration, but also as yet another way to get useful feedback.

@fushar yes the blockMeta was the icon I was trying to reference, though I'm not quite as concerned with the slug for it (not certain that the slug is referenced or rendered anywhere in the UI and if not then there's no end user confusion that could happen). I'm not much in favor of the chevronUpDown option for Sites, in that case would just stick with the 2x2 table-looking icon I suppose but in any case would like a better one for that icon if possible (and if as @tyxla notes this is too late for 7.1 then we've got some time to iterate on the Sites icon).

An angle that doesn't seem explored is trying this out as a Gutenberg experiment

Thanks for the suggestion. I spent a good chunk of my day today trying to make this as a Gutenberg experiment.

With the help of Claude, I (we) came up with this PR: WordPress/gutenberg#79588. Now, I understand that the changes are complicated, but I can't simplify it any further. Reasons:

• I need to render the icons that are not yet public. wp_get_icon() won't allow me to render such icons, so I need to replicate the implementation for now.
• There's no way to hook into admin menu icon. Basically I really need this new logic in Core, which can't be extended via hooks. Hence the experiment needs to hijack the output of the admin menu HTML and replace dashicons with SVG before it gets rendered.
• The CSS needs to duplicate the admin color scheme to achieve hover effect in the admin bar. In this Core change, I can just update the _admin.scss directly, which can't be replicated in experiments.

Am I approaching this correctly? Or do you have better idea for a cleaner approach? Thanks!

cc: @mcsf

It looks great ✨ I noticed a few things that I'm not fully sure belong to the PR scope.

Icon size

In most cases, the icon size is 22px, but the W symbol is 24px.

22px	24px

Not sure if this is intentional or not.

Color on the frontend

On the frontend, the color at 0.6 looks slightly lighter. I know it is consistent, but at 0.85 looks visually more similar to the current. This is not a blocker but sharing it for thoughts.

"Sites" icon

Regarding the icon for Sites, I agree that blockMeta works well until having a dedicated icon.

I've been testing it on Arc, Chrome, and Safari and on multiple viewports. And they all look good ⭐

Am I approaching this correctly? Or do you have better idea for a cleaner approach?

Hmm, this seems correct to me. There are side ways for achieving some of the things you mentioned, but not for others, unless we want to be super hacky. A bummer that this part of the menu is not as extensible as usual. Thank you for trying, though.

Another way to get more attention on this would be to post a call for testing on the https://make.wordpress.org/core blog.

Am I approaching this correctly? Or do you have better idea for a cleaner approach? Thanks!

I'd say so, at least for the purposes of a Gutenberg experiment. I left a comment or two in that PR.

yes the blockMeta was the icon I was trying to reference

@jeffpaul: I agree that it looks better than a 2x2 grid. For the purpose of demonstration, I updated this PR to use that for the time being:

@fcoveram: Thanks for the thorough testing!

In most cases, the icon size is 22px, but the W symbol is 24px.
Not sure if this is intentional or not.

Yeah, it is intentional. I'm not sure if it's the best way though. It's to achieve the same look and feel with the existing W dashicon. See the above image comparison, pasted here again:

Right now, if we use 22px, then the W logo is too small, because there's outer gap in the SVG. Maybe we need to fill the entire SVG box without any outer gap.

On the frontend, the color at 0.6 looks slightly lighter. I know it is consistent, but at 0.85 looks visually more similar to the current.

Yeah agree that 0.85 looks better. But I'm not sure if we can just update it only for the SVG icon. It's part of the Fresh color scheme; maybe we should do a separate PR to update the scheme 😄

All right everyone, thanks for taking a look. I want to summarize where we're at with this PR:

• To use currently non-public icons, we need the public: false flag as discussed in Icons: ship required icons for admin bar and menu with public: false gutenberg#79451 (comment).
  • @t-hamano, is this something that you're currently pursuing? How confident are we with the solution?
• This PR adds support to use registered icons in left sidebar menu items. See this diff. To move forward, I need a review on this as it's modifying a public hook (admin_menu).
  
  • Also, maybe we should at least ship this part first in 7.1 so that plugins can start using it?
  • If we do that, it will be beneficial to ship the CSS changes for rendering SVG icons in admin bar / menu.
• We need to iterate on My Sites icon. I can ask for help for this. @jasmussen / @lucasmendes-design should be interested.
• As suggested, we should publish a public Make post as a heads-up to the community and plugin developers. In the post, we should suggest the default (core) icon set for the admin, and how plugins can use SVG icons in the admin menu (the public API change I mentioned above).

It feels like there's interest to push at least some parts of it in 7.1, and we're not that sure if we can achieve everything by 7.1. @t-hamano, what's your opinion on this? It seems you have the most context for all this icon API discussion 😄

cc: @tyxla, @mcsf, @scruffian.

It feels like there's interest to push at least some parts of it in 7.1, and we're not that sure if we can achieve everything by 7.1. @t-hamano, what's your opinion on this? It seems you have the most context for all this icon API discussion 😄

Leveraging the icon API for the dashboard icons is a great idea, and introducing a public:false flag shouldn't be difficult. However, I noticed that core icons might be unregistered by consumers.

add_action( 'init', function () {
    wp_unregister_icon_collection( 'core' );
}, 20 );

add_action( 'init', function () {
    wp_unregister_icon( 'core/info' );
}, 20 );

In this case, what happens to the core menu icons registered by the icon API? This is my biggest concern. Here are the ideas I can come up with at the moment:

• Allow file paths for menu_icon. This ensures that icons are rendered even if they are unregistered from the icon registry, as the file itself will still exist within the core.
• Allow SVG strings for menu_icon. This might be a larger undertaking, requiring a proper SVG processing pipeline.
• Introduce a "built-in" collection to house icons used in dashboards and protect the unregisteredion of this collection or its icons would enhance robustness.

Do you have any other ideas?

Thanks for taking a look.

I noticed that core icons might be unregistered by consumers.

Hmm interesting question. I would say, the last option (consider core/* as built-in icon set which can't be unregistered) looks the most sensible to me. It's like having a fixed set of dashicons-* slugs that consumers can always use currently.

Otherwise, we will also need another way to get the SVG string for the admin bar icons as well, because in this PR I also use wp_get_icon() to prepend the icon to the title 🤔

Thanks for the additional context 😄 the search icon direction is now fixed in the latest commit 6c3fc8b.

The icon direction looks correct now ✨

@peterwilsoncc Let's discuss your comment here instead of on the diff. GitHub UI is bad for PRs with many comments; it's hard to follow chronological discussion 😅

Reading the comment, I think there's a bit a more fundamental issue here. If plugins want to "migrate" their dashicons to SVG icons, should we tell them to use core-admin/ or core/ collection? Both have issues:

• If they're supposed to use core-admin/ only, the collection currently only has few icons, because the set was meant to contain only Core's built-in icons.
• If they may use core/, then they won't survive when the core/ collection is unregistered. See the discussion in the original PR, the main idea being that we don't want the icons in the admin screens to be unregisterable at all.

I'm thinking a few approaches here:

1. Expand core-admin/ collection to have a few more "popular" icons from core/.
2. Or just simply make core-admin/ a proper superset of core/.

• And we might optionally ban core/ (or any public collection actually) from being used when constructing the sidebar menu and admin bar nodes. (Not sure)

Sorry, I just realized about this now. I might be overthinking, but the potential confusion is real here.

Thoughts @t-hamano, @tyxla, @mcsf ?

Reading the comment, I think there's a bit a more fundamental issue here. If plugins want to "migrate" their dashicons to SVG icons, should we tell them to use core-admin/ or core/ collection? Both have issues:

• If they're supposed to use core-admin/ only, the collection currently only has few icons, because the set was meant to contain only Core's built-in icons.
• If they may use core/, then they won't survive when the core/ collection is unregistered. See the discussion in the original PR, the main idea being that we don't want the icons in the admin screens to be unregisterable at all.

@fushar Thanks for raising this. I agree that your concern is valid.

Here are my thoughts and suggestions:

• core-admin is still the collection for wp-admin, while core is the collection for the front end, content, and the Icon block.
• Don't automatically migrate Dashicons to SVG icons at all. It's not possible to map every Dashicon one-to-one to an SVG icon in the first place. The only exception is the core menu items, which are migrated to SVG icons (third-party items are not).
• Let consumers know the following through the dev note:
  • Encourage them to use SVG icons instead of Dashicons where possible when registering menus and post types.
  • Publish the list of icons currently available in the core-admin collection.
• Expand the core-admin icons as much as possible, based on requests from developers or as a follow-up.

I tried installing a lot of plugins, and personally, I don't find the mix of SVG icons and Dashicons that jarring. What do you think?

Thanks @t-hamano I agree with all your suggestions. They all make sense to me. Particularly:

I tried installing a lot of plugins, and personally, I don't find the mix of SVG icons and Dashicons that jarring.

Yep, I agree. I know that @jasmussen et al have designed the icons in such a way that they don't look too out of place compared to the original Dashicons 🙂

Let consumers know the following through the dev note

I'll add that to the dev note.

Expand the core-admin icons as much as possible

Yeah, agree. I think we should add more generic icons, like envelope, cart, etc. That would need to happen on Gutenberg first (I can try to raise the PR). WDYT @jasmussen?

I agree with @t-hamano's plan, including not auto-mapping third-party dashicons, with a few small notes:

• If the dev note tells plugins to use core-admin/*, those icons become public API, even with public => false. We'll likely want to say this clearly in the dev note and the wp_register_icon() docs. We'll also have to agree that we never rename or remove an icon once it's added.
• Worth mentioning in the dev note that plugins can register their own collection (like my-plugin/cart). It already works in menus and the admin bar.
• I'd prefer not banning public collections. New collections are public by default, so a ban would also block plugin icons and adding unnecessarily limits.

Stroke width on mobile
With the stroke scaling work (WordPress/gutenberg#84081), the 32px size becomes bolder. I see that non-scaling-stroke is still there, so a follow-up would be to find a coherent way to use icons that keeps the stroke width the same in both the admin bar and the sidebar.

WordPress icon scale
The WordPress icon looks slightly bigger because it uses 20 of the 24 units in its container. Until the icon becomes stroke-based, perhaps we can scale it to 18px inside the same 24px container. This also relates to WordPress/gutenberg#84105.

I wonder if we can approach it differently and apply an ad hoc property that scales it to 18px inside the same 24px container.

Honestly, I don't fully understand this and this is a bit beyond my expertise 😄 I'd like to defer the implementation details to the icon designers @jasmussen / @lucasmendes-design. We should be able to follow up as separate PR during the Beta if needed.

Folks, thanks for all the help and feedback for this PR so far. Really appreciate it. As Beta 1 is around the corner, I think we should wrap up this PR soon if we want to include it in 7.2. Based on what I read so far, there's no major blocker. What's left is to have an agreement on the proposed back-compat strategy in the dev note, and test its claims. Personally, I've already tested this PR so many times with various plugins / color schemes 🙂

Hey folks, sorry I'd missed the resolutions of the previous comments -- as @fushar said, the GH UI can sometimes be confusing.

Folks, thanks for all the help and feedback for this PR so far. Really appreciate it. As Beta 1 is around the corner, I think we should wrap up this PR soon if we want to include it in 7.2. Based on what I read so far, there's no major blocker. What's left is to have an agreement on the proposed back-compat strategy in the dev note, and test its claims. Personally, I've already tested this PR so many times with various plugins / color schemes 🙂

Agreed: this can go in beta 1 and further substitutions discussed and an approach figured out during the beta period.

I do like the idea of including the converted icons in the core group within the core-admin group. The font's name dashicons does imply they are available in the dashboard so it's not an unreasonable assumption that the converted icons will be available in the dash.

It would be good to get the dev note out very shortly after beta 1 so devs in plugin land can provide feedback asap.

There are two things I'd like to note, both of which can probably be explored in follow-ups:

Yep, I think they can be followed up later and don't need to block this PR 😄

Thanks for all your reviews!

Catching up, please let me know if I missed anything, it was Danish potato vacation last week.

Yeah, agree. I think we should add more generic icons, like envelope, cart, etc. That would need to happen on Gutenberg first (I can try to raise the PR).

I agree, and yes please open issues, happy to draw. To note, though, envelope and cart both exist, same names.

Honestly, I don't fully understand this and this is a bit beyond my expertise 😄 I'd like to defer the implementation details to the icon designers @jasmussen / @lucasmendes-design. We should be able to follow up as separate PR during the Beta if needed.

That's from this comment, and I can unpack it: the new icons have flexibility the old ones didn't: we can tweak the stroke-width. There's an ongoing question around whether we should, and even how we tweak it if we do. All of that, honestly I'm increasingly leaning towards the instincts of Fran, Aki, @jameskoster and @ciampo on the precise details.

What we do, if anything, in this iteration, I think should not overall affect what we do with this branch: one of the benefits of unifying on WordPress icons (which is the overwhelming focus of this branch), is to unify the codebase too, so whatever improvements we make to the core set, including any changes to stroke-scaling behavior for better legibility, will benefit the admin in future iterations too. So I don't know that we should do any local changes here in this branch or not, I lean to whatever is the simplest to let us focus on the shipping. Is that helpful?

Catching up as well. I agree with @jasmussen. No icon-specific changes in this branch. Both open design points are library questions, and Core gets the answer when it bumps @wordpress/icons.

A commit was made that fixes the Trac ticket referenced in the description of this pull request.

SVN changeset: 64583
GitHub commit: 5832f82

This PR will be closed, but please confirm the accuracy of this and reopen if there is more work to be done.