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

Add WooCommerce icon #63

Merged
merged 5 commits into from Jul 26, 2018
Merged

Add WooCommerce icon #63

merged 5 commits into from Jul 26, 2018

Conversation

jameskoster
Copy link
Contributor

@jameskoster jameskoster commented Jul 3, 2018

icon

Based on the woocommerce.com Favicon, using the bubble from the WC logo.

@davewhitley
Copy link
Contributor

LGTM

@jameskoster
Copy link
Contributor Author

Should have noted - the corner radii are from the original logo - scaled down. They're pretty close to lining up perfectly with our grid. Curious if there's any benefit to making this small tweak?

@folletto
Copy link
Contributor

Curious if there's any benefit to making this small tweak?

I'd check the pixel preview. If it makes it sharper, I'd tweak. :)

@jameskoster
Copy link
Contributor Author

jameskoster commented Jul 20, 2018

That's pixel preview, one with aligned radii, one with scaled-down radii from the original logo.

Not really any difference to my eye.

Edit: Actually there is a stray pixel in the icon on the right. Could just be an Illustrator thing, but I'll use the icon on the left to remove all doubt.

Radii now matches the soclal-logos grid.
@folletto
Copy link
Contributor

Is that at the default 24px? Can we maybe tweak it slightly so the edges are all sharp at that size? :)

@davewhitley
Copy link
Contributor

The edges on the left icon are much fuzzier.

@jameskoster
Copy link
Contributor Author

jameskoster commented Jul 25, 2018

Strange, looks like that might be an up-scaling issue with macOS and screenshots. Here's the exported version;

The edges are all from a rectangle that was drawn with snap-to-pixel on, so I'm not sure why it's looking fuzzy in the pixel preview...

@folletto
Copy link
Contributor

That looks good.
There seem to be a slight blur still on the bottom edge, do you think it's worth fixing for sharpness?

@jameskoster
Copy link
Contributor Author

jameskoster commented Jul 25, 2018

That's fixable but means breaking the grid;

Edit: That's pretty funny. I moved the bottom-most points of the bubble a little to fix that preview blurring issue. I just pasted the updated version into the template to see how far "off-grid" that change made things...

I'm beginning to question the reliability of the pixel preview :p

@folletto
Copy link
Contributor

Hah! Looking good :)

@jameskoster
Copy link
Contributor Author

Cheers. If you wouldn't mind approving @folletto I'll go ahead and merge this in.

Copy link
Contributor

@folletto folletto left a comment

Choose a reason for hiding this comment

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

Looks good :D

@jameskoster jameskoster merged commit f38f9f9 into master Jul 26, 2018
@jameskoster jameskoster deleted the add/woocommerce branch July 26, 2018 11:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants