simple-icons / simple-icons-font

SVG icon font for popular brands
https://simpleicons.org
Creative Commons Zero v1.0 Universal
82 stars 12 forks source link

Suggestion: Further Default Normalization Styles #161

Closed bhadaway closed 2 years ago

bhadaway commented 2 years ago
.si {
    display: inline-block;
    font-family: 'Simple Icons', sans-serif;
    font-style: normal;
    line-height: normal;
    vertical-align: middle;
}

PS: Unless I'm just being a complete dumb-dumb, the actual /font/ folder appears to be missing from the GitHub repo? I had to go to unpkg.com to find the font files and download them.

mondeja commented 2 years ago

Nope, the folder is there. Are you really sure that you're not getting it? This is the output in my console:

$ npm i --no-save simple-icons-font
$ tree node_modules/simple-icons-font
node_modules/simple-icons-font
├── DISCLAIMER.md
├── font
│   ├── simple-icons.css
│   ├── SimpleIcons.eot
│   ├── simple-icons.min.css
│   ├── SimpleIcons.otf
│   ├── SimpleIcons.ttf
│   ├── SimpleIcons.woff
│   └── SimpleIcons.woff2
├── LICENSE.md
├── package.json
└── README.md

1 directory, 11 files
bhadaway commented 2 years ago

Oh, I see. The fonts are generated dynamically through installation, for obvious practical reasons? They're not just sitting in a folder plainly, within the repo. I'm setting them up manually and just needed to straight download them, so that's why I was a bit confused.

mondeja commented 2 years ago

Could you explain why we must add the following CSS properties?

        font-style: normal;
    line-height: normal;
    vertical-align: middle;
bhadaway commented 2 years ago

I threw together a quick demonstration:

https://bryanhadaway.com/testing/simple-icons.html

mondeja commented 2 years ago

Thanks, but are the font-style: normal and line-height: normal properties needed? I can see the icon correctly positionated as well if I remove them:

demo-vertical-align

bhadaway commented 2 years ago

Yes, they're needed. This is just a vanilla test, so doesn't take into account some common variables that might take place out in the wild.

font-style: normal protects the icons from warping if they're added between <em></em> tags. Note: this is already in the existing style reset.

line-height: normal protects the icons from overflow problems that can occur (especially on mobile) if the user has set the icons to a big enough size.

I even ran into an issue where I needed to use text-shadow: none, but that probably really is too aggressive in terms of finding the right balance for most users.


For what it's worth, I actually came up with my own hybrid solution where I combined the font method with actual svgs so that I could benefit from a lighter front-end load (the font method adds over 1mb to page load, and my method brings that down to under 100kb) and the responsiveness of svgs. Anyway, that's a discussion for another day, if anyone's interested.

mondeja commented 2 years ago

Thank you for the report, but I think that:

For the problem of high load added by fonts, that's true. It would be great to expose the build process to create a customized build, filtering by icons slugs. I've opened #163 to track it.

So I'm fine with adding vertical-align: middle but I want to study before what other icon fonts like FontAwesome do.

bhadaway commented 2 years ago

Just a quick reminder that I'm not suggesting the addition of font-style: normal; it's already in the official code:

https://github.com/simple-icons/simple-icons-font/blob/develop/scripts/templates/base.css

The logic (I would assume) would be that while it's technically a font, that that's only a formality as a trick/method for delivering icons that shouldn't otherwise be treated like text. The same logic applies to line-height: normal and maybe there should even be font-weight: normal too. The icons should only appear exactly as intended concerning the shape/design of each respective icon, without warping/stylization of any kind beyond size and color.

Anyway, that's all I have to offer on the subject; I'll let you take it from here.

Thanks