Skip to content
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

Update Dashicons component. #10025

Merged
merged 1 commit into from
Sep 19, 2018
Merged

Update Dashicons component. #10025

merged 1 commit into from
Sep 19, 2018

Conversation

jasmussen
Copy link
Contributor

This updates the Dashicons component with recent additions, two which are in a PR that's yet to be merged, and one that's needed to exit fullscreen.

Some of the changes in this PR haven't yet been merged into upstream Dashicons, we are waiting for WordPress/dashicons#316 to settle. But in the mean time, this copies the new paths.

This updates the Dashicons component with recent additions, two which are in a PR that's yet to be merged, and one that's needed to exit fullscreen.

Some of the changes in this PR haven't yet been merged into upstream Dashicons, we are waiting for WordPress/dashicons#316 to settle. But in the mean time, this copies the new paths.
@jasmussen jasmussen added the [Type] Enhancement A suggestion for improvement. label Sep 19, 2018
@jasmussen jasmussen added this to the 4.0 milestone Sep 19, 2018
@jasmussen jasmussen self-assigned this Sep 19, 2018
@jasmussen jasmussen requested a review from a team September 19, 2018 08:10
Copy link
Member

@tofumatt tofumatt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand why we copy these but as it's already done this way I guess it's fine.

Why are these being added without being used? Right now it'll just bloat the package size without being used...

@jasmussen
Copy link
Contributor Author

I don't understand why we copy these but as it's already done this way I guess it's fine.

For now, because Dashicons is a separate repo, and no-one has had the bandwidth to improve the situation: https://github.com/WordPress/dashicons

Why are these being added without being used? Right now it'll just bloat the package size without being used...

"Align pull-left" and "Align pull-right" will be used when Jorge merges #9416.

I sense there's a larger question here; what is a good way to use icons?

Again we are straddling modern with old here, and there's a long term answer and a short term answer.

WordPress uses Dashicons. They have served WordPress well from a time when icon fonts were the way to go, and few big icon sets existed.

However they are starting to show their age in size — screens are larger now — this is tracked here. They are also showing their limitations, not only in how dashicons are built (you allude to this yourself here, but it used to be even worse, with every single icon being added to the icon font manually using font creation tools), but also in the amount of dashicons available.

The block library recently switched to Material icons, which is a vast, continually expanding, open source set. The argument for this was to ensure diversity in the icons, so we didn't end up in a situation where a block author creates a new "Cover Image" block and uses the "Image" icon becuase there's no appropriately diverse alternative to use instead. By leveraging the wealth of icons in Material, this is less of a problem.

So on one hand we are trying to bring Dashicons in to the future, acknowledging that for the time being this is the icons set that is used in all of WordPress and therefore ensuring some consistency in the interface. On the other hand we are looking at how to use icons in the future, not only to move beyond icon fonts and into the SVG era, but to move into a modern JavaScript era.

Ultimately I would like for all of WordPress to look like it uses only a single consistent icon set. I suspect there will always be a need for a custom set for some of the core icons that Material is likely to never have, like "Align Wide", for example. But visually, stylistically, and in dimensions, the icons should be consistent, and it would be nice if both this core set, and material icons, were as easy for developers to use as possible. Does that mean inline SVGs? Or does it mean a sprite set like we use here? A little of both? Unknown, but an answer will present itself as we continue to iterate, I'm sure.

Hope that was helpful, and thanks for the review!

@jasmussen jasmussen merged commit bc5befd into master Sep 19, 2018
@jasmussen jasmussen deleted the update/dashicons branch September 19, 2018 11:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants