-
-
Notifications
You must be signed in to change notification settings - Fork 709
Update icons for HIG #3824
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 icons for HIG #3824
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- It looks like
contrast-example2.pngis the wrong size of image. The goal of this section is to show the stroke style and it looks like we overlay a grid. The replacement image here has strokes that are half as wide as the original image. I'm guessing that the 128px image was exported at 4x here in the replacement, but it's probably 64px @ 8x in the original - in
example-icon1.png, the text here is about showing the concept of the "mean line" across different icons with different x heights. The arrow in the replacement icon no longer aligns on this mean line, so it makes the accompanying text not make sense. We probably should just get rid of this "mean line" section since I don't think we really adhere to that anymore, but either way this replacement image doesn't really make sense here - We're missing the replacements for
shadow-example1.pngetc that go with the updated Music icon. I'm okay with not updating this if you intend to remove that section in anticipation of elementary/icons#1353
|
@danirabbit Yes, I wasn't intending to fully remove sections yet, just leave them as vestigial here and come back to them later, but perhaps it's better to change the coolant while we're changing the oil, so to speak. I can make a proposal to remove/deprecate relevant sections from the HIG. |
danirabbit
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm happy to merge this now, and then yeah I think we should probably remove the section about the "mean line" as a follow up
|
Thanks for updating these! |
Related: elementary/hig#54
Changes Summary
This pull request is ready for review.
Linked with elementary/hig#62