-
Notifications
You must be signed in to change notification settings - Fork 23
Fix view_menu #147
Fix view_menu #147
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.
The image looks good in the sense that it's similar/equal to the existing one.
I see the issue with the SVG I proposed in eclipse-platform/eclipse.platform.ui#2995 which leads to "squarely-looking" circles due to the circles being aligned with the grid. Reason for me to do so was that it produces sharper results. See that the existing icon is more blurry than the one I proposed. On the top, you see my SVG, at the bottom, you see yours:
I agree that we should better stick to the existing look and feel.
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.
LGTM
@BeckerWdf so you agree that we should take the icon that Sebastian proposed, as it conforms to the existing one (even though it looks a bit blurry when rendered), right? |
No. I think we should go with the "sharp" one. Or does this version have other "problems"? |
There were several issues with the view_menu icon: - The checked in SVG files were not matching the PNG files which have been used so far. - The disabled PNGs were not actually disabled, but copies of the enabled ones. - A @2x.png icon was still the old chevron icon. Fix: - Use SVG file from eclipse-platform/eclipse.platform.ui#2995, clean up metadata and exactly align on pixel boundaries and (half) integer stroke width. - Delete disabled SVG files and let the renderer derive the disabled PNG files. - Re-render everything to PNG.
view_menu.svg was the wrong svg file, not matching the .pngs. Follow-up on eclipse-platform#2925. Uses the updated icons taken from eclipse-platform/eclipse.platform.images#147
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.
Great, thank you! Tested the latest proposal and found it really convincing in eclipse-platform/eclipse.platform.ui#2994 (review)
view_menu.svg was the wrong svg file, not matching the .pngs. Follow-up on #2925. Uses the updated icons taken from eclipse-platform/eclipse.platform.images#147
There were several issues with the view_menu icon:
Fix: