-
Notifications
You must be signed in to change notification settings - Fork 40
Description
This is a follow-up for #1841. @quicksketch left this comment in the PR that was merged:
Seems like part of the problem here is that the icons are different sizes, which is odd considering they're all named --64.png, which I would have thought meant 64x64 pixels. I think a better solution to using a background-image directly on the links would be to use a pseudo-element that is placed with :after, then it can be made a consistent size and not have to worry about percentage differences.
But in any case, I think that can happen later. For now I went ahead and went with the 90% across the board as suggested by opi, which looks better on mobile where they're all lined up vertically and doesn't make a discernible difference on desktop (to my eye). Let's go with this and refine as needed.
As mentioned, these icons were added in issue #1841, but our core committer recommended that we look at a better solution in the future. He suggested to - use a pseudo-element that is placed with :after, then it can be made a consistent size and not have to worry about percentage differences.
If you get in and look the current css for these icons, then this suggestion might make more sense. Please, post any questions you have here in the issue queue.
(edited by: @stpaultim)