primer / octicons

A scalable set of icons handcrafted with <3 by GitHub
https://primer.style/foundations/icons
MIT License
8.27k stars 827 forks source link

Icon is not Centered #915

Closed Neodevils closed 1 year ago

Neodevils commented 1 year ago

North Star icon is not centered.

image

tallys commented 1 year ago

Hi! Thanks for opening the issue - I'll forward it to the Octicons Working Group project board.

Neodevils commented 1 year ago

Hi! Thanks for opening the issue - I'll forward it to the Octicons Working Group project board.

Alright! Take your time. 👍🏻

gavinmn commented 1 year ago

Is this misalignment causing an issue? I didn't create the icon but I'm guessing it was done this way so that the vertical and horizontal lines are properly aligned to the pixel grid. Directly centering it would cause all lines to be set on half or quarter pixels.

Neodevils commented 1 year ago

Is this misalignment causing an issue? I didn't create the icon but I'm guessing it was done this way so that the vertical and horizontal lines are properly aligned to the pixel grid. Directly centering it would cause all lines to be set on half or quarter pixels.

Well, yes. Misalignment. Either the vector has to be a little bit longer or shorter then.

gavinmn commented 1 year ago

I did some more tests with this icon in the UI and feel like the half pixel alignment on the vertical and horizontal lines is probably worth the trade off. I'll see about getting this change shipped in the next update.

Neodevils commented 1 year ago

I did some more tests with this icon in the UI and feel like the half pixel alignment on the vertical and horizontal lines is probably worth the trade off. I'll see about getting this change shipped in the next update.

Thank you. I'll be watching this issue.

tallys commented 1 year ago

Hi, thanks for bringing this up and thanks for the due diligence @gavinmn. I'm chiming in with some history on the creation of this icon, there was a specific reason that we decided to make it slightly off centered, so we will not be shifting it at this time.