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

Fix Tools icon being 24x24 instead of 20x20. #18829

Merged
merged 1 commit into from Nov 30, 2019
Merged

Fix Tools icon being 24x24 instead of 20x20. #18829

merged 1 commit into from Nov 30, 2019

Conversation

@ZebulanStanphill
Copy link
Contributor

ZebulanStanphill commented Nov 29, 2019

Fixes #18826.

Before:
image

After:
image

const editIcon = <SVG xmlns="http://www.w3.org/2000/svg" width="24" height="24" viewBox="0 0 24 24"><Path fill="none" d="M0 0h24v24H0V0z" /><Path d="M14.06 9.02l.92.92L5.92 19H5v-.92l9.06-9.06M17.66 3c-.25 0-.51.1-.7.29l-1.83 1.83 3.75 3.75 1.83-1.83c.39-.39.39-1.02 0-1.41l-2.34-2.34c-.2-.2-.45-.29-.71-.29zm-3.6 3.19L3 17.25V21h3.75L17.81 9.94l-3.75-3.75z" /></SVG>;
const selectIcon = <SVG xmlns="http://www.w3.org/2000/svg" width="24" height="24" viewBox="0 0 24 24"><Path d="M6.5 1v21.5l6-6.5H21L6.5 1zm5.1 13l-3.1 3.4V5.9l7.8 8.1h-4.7z" /></SVG>;
const editIcon = <SVG xmlns="http://www.w3.org/2000/svg" width="20" height="20" viewBox="0 0 24 24"><Path fill="none" d="M0 0h24v24H0V0z" /><Path d="M14.06 9.02l.92.92L5.92 19H5v-.92l9.06-9.06M17.66 3c-.25 0-.51.1-.7.29l-1.83 1.83 3.75 3.75 1.83-1.83c.39-.39.39-1.02 0-1.41l-2.34-2.34c-.2-.2-.45-.29-.71-.29zm-3.6 3.19L3 17.25V21h3.75L17.81 9.94l-3.75-3.75z" /></SVG>;
const selectIcon = <SVG xmlns="http://www.w3.org/2000/svg" width="20" height="20" viewBox="0 0 24 24"><Path d="M6.5 1v21.5l6-6.5H21L6.5 1zm5.1 13l-3.1 3.4V5.9l7.8 8.1h-4.7z" /></SVG>;

This comment has been minimized.

Copy link
@ellatrix

ellatrix Nov 29, 2019

Member

Should viewBox also be updated?

This comment has been minimized.

Copy link
@ellatrix

ellatrix Nov 29, 2019

Member

I mean, Ideally the SVG needs to be adjusted for this size, unless all icons use a 24px grid.

This comment has been minimized.

Copy link
@ZebulanStanphill

ZebulanStanphill Nov 29, 2019

Author Contributor

The SVGs used in the other buttons all have the same viewBox value as this one, so I didn't change it.

This comment has been minimized.

Copy link
@ellatrix

ellatrix Nov 29, 2019

Member

Oh, ok, that's interesting.

This comment has been minimized.

Copy link
@youknowriad

youknowriad Dec 2, 2019

Contributor

I guess we could also remove these attributes and use CSS

This comment has been minimized.

Copy link
@jasmussen

jasmussen Dec 2, 2019

Contributor

I guess we could also remove these attributes and use CSS

This could have adverse effects, because when CSS isn't applied (for whatever reason), the SVG will size to fit the availabl space which usually means it grows giant.

I think this is another use case for introducing an <Icon /> component so we can abstract away much of the complexity.

@ellatrix ellatrix requested a review from jasmussen Nov 29, 2019
@ellatrix ellatrix merged commit 991f90d into master Nov 30, 2019
2 checks passed
2 checks passed
pull-request-automation
Details
Travis CI - Pull Request Build Passed
Details
@ellatrix ellatrix deleted the fix/18826 branch Nov 30, 2019
@youknowriad youknowriad added this to the Gutenberg 7.1 milestone Dec 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.