primer / octicons

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

[Bug] `no-entry` icon circle is inconsistently sized at 16px #641

Closed laserlemon closed 3 years ago

laserlemon commented 3 years ago

As far as I can tell, of all the circular icons, the no-entry icon is the only one that's sized slightly differently; it's just a bit smaller than the others. You can see a comparison below taken from primer.style between circle and no-entry. The comparison still holds if you swap out circle for issue-opened, clock, or any other circular icon.

16px

circle no-entry
Screen Shot 2021-07-02 at 10 43 21 AM Screen Shot 2021-07-02 at 10 43 40 AM

24px

At first, I thought the size difference might be intentional, but I also noticed that the sizes are identical at 24px. See below:

circle no-entry
Screen Shot 2021-07-02 at 10 44 09 AM Screen Shot 2021-07-02 at 10 44 28 AM

Thank you for your ongoing work on this amazing set of icons! 💛

ashygee commented 3 years ago

Thanks for calling this out @laserlemon and apologies for the long delay. This icon was intentionally made smaller based on the usage within code diffs and should be smaller than the other circle-shaped icons. The incorrect one would be the 24px icon which @Juliusschaeper will correct. In addition, this icon also needs to be added to our library in Figma.

laserlemon commented 3 years ago

Interesting, thanks for the background! For our use case, we have a mechanism for adding/overriding icons with our own custom icons whenever needed, so we just bumped up the circle size here to match our other related cases (seen below):

Screen Shot 2021-09-21 at 3 04 38 PM

Feel free to use the issue to track the incorrectly sized 24px version. Otherwise, also feel free to close it. Thank you!

ashygee commented 3 years ago

Notes from working session:

We met with @laserlemon to discuss the inconsistency in our circle icons. Because the no-entry icon’s original purpose was to be used at the end of code lines in the diff view, this had been intentionally created smaller. Through further discussion, @laserlemon brought up a great point that we are not the only folks using Octicons and, although we prioritize usage for dotcom, we may want to consider how others could be using similarly shaped icons. As a team, we always want to be sure that our open source community also has a great experience with octicons but as we’ve prioritized our own needs first, some items, such as this inconsistency issue, have fallen through the cracks. @laserlemon’s comment got us thinking and we have come up with a solution we will work to implement.

Next steps

ashygee commented 3 years ago

The first phase to have the circle icons with parity in the outer shape dimensions has been completed and will be merged in the next release of octicons. The small icons are still in progress and are being tracked in https://github.com/github/primer/issues/332