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 the replace media icon with a pixel-perfect version #14807

Merged
merged 1 commit into from
Apr 5, 2019

Conversation

kjellr
Copy link
Contributor

@kjellr kjellr commented Apr 3, 2019

The "Replace" icon added in #14142 looks a little blurry on non-retina screens. Didn't catch this at first, but I'm seeing it today.

This PR replaces it with a more pixel perfect version, but I want to better understand why it's pixellated in the first place. Our icons are usually 24px square, so I originally made the icon at that size. But looking into the other toolbar icons, it appears those are 20px square, with 5px padding on the top and bottom. In this PR, I re-drew the SVG for that, and it fixed the problem. Is this the right approach?

cc @jasmussen for your icon expertise.


Before

Screen Shot 2019-04-03 at 2 48 47 PM

Zoomed in 3x:
Screen Shot 2019-04-03 at 2 50 01 PM

After

Screen Shot 2019-04-03 at 2 44 17 PM

Zoomed in 3x:
Screen Shot 2019-04-03 at 2 49 46 PM

@kjellr kjellr added [Type] Bug An existing feature does not function as intended Needs Design Feedback Needs general design feedback. labels Apr 3, 2019
@kjellr kjellr self-assigned this Apr 3, 2019
@kjellr kjellr requested review from mapk, jasmussen and melchoyce April 3, 2019 18:59
@jasmussen
Copy link
Contributor

Thank you for this. The icons must not be blurry ever, and this is a righteous fight!

So the current blur is because it's a 24px icon that's being scaled down to 20px.

We're sort of between two chairs on this one. 24x24px is the icon grid for Material icons, which are used in the block library and switcher. 20x20 is the icon grid for Dashicons, which are used elsewhere in the editor.

This split, and an effort to make it easier both for us and for third party developers to adopt and use great icons, is being discussed in a few places, including here. Which is to say — very long term I could see us moving towards a unified 24x24px grid for all icons in WordPress (optionally with 75% size 18px icons for the odds and ends).

The larger grid affords more pixels, therefore better legibility and implicitly accessibility.

So on the one hand, I would not oppose us moving this 24px icon to 20px to be sharp and crisp. On the other hand, I would love to see us move all icons to 24px, and the rules that scale them down to 20px in the long term. So if we do replace this, we'll want to replace it again if and when those efforts take hold.

What do you think?

@kjellr
Copy link
Contributor Author

kjellr commented Apr 4, 2019

So on the one hand, I would not oppose us moving this 24px icon to 20px to be sharp and crisp. On the other hand, I would love to see us move all icons to 24px, and the rules that scale them down to 20px in the long term. So if we do replace this, we'll want to replace it again if and when those efforts take hold.

What do you think?

Thank you for clarifying! Yeah, I think we should just use a 20px icon for this one for now. When we do make the jump to 24px icons everywhere, we've already got a 24px version of this all ready to go. 👍

Copy link
Contributor

@jasmussen jasmussen left a comment

Choose a reason for hiding this comment

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

Let's remember to revisit this one, among all the others, if and when we move to a larger grid.

@mapk mapk removed the Needs Design Feedback Needs general design feedback. label Apr 5, 2019
@mapk
Copy link
Contributor

mapk commented Apr 5, 2019

I love the conversation about moving to 24px icons. Until we do the solution here works.
LGTM! :shipit:

@talldan talldan merged commit 079c11b into master Apr 5, 2019
@talldan talldan deleted the update/replace-icon-for-images branch April 5, 2019 03:17
@kjellr
Copy link
Contributor Author

kjellr commented Apr 9, 2019

Thanks, all!

@youknowriad youknowriad added [Type] Enhancement A suggestion for improvement. and removed [Type] Bug An existing feature does not function as intended labels Apr 12, 2019
@youknowriad youknowriad added this to the 5.5 (Gutenberg) milestone Apr 12, 2019
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.

5 participants