refactor: Replace icon font with SVG icons #790
Conversation
✅ Deploy Preview for fipguide ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
Hi there! 👋 Thank you for your contribution to the FIP Guide! 🚀 Checklist before merging:
|
MoritzWeber0
added
the
technical
therobrob
left a comment
therobrob
left a comment
There was a problem hiding this comment.
thank you Moritz :) the icons have an other size than before, is this intended? In addition to the comments below, I noticed that the icons of the highlights are much smaller (14x14px), also of the buttons (15x15px) and dialog-close (15x15px). The line thickness of all icons is also different. The icon of the copy-toast is a little too high up.
| } | ||
|
|
||
| .a-icon { | ||
| font-size: 1.2em; |
There was a problem hiding this comment.
the icon is smaller than before, it should be 20x20px
There was a problem hiding this comment.
With the general size increase, it's now 17x17 vs 18x18 on my side. I do think it looks slightly better with the alignment:
Before:
After:
If you like the previous icon more, I can increase the font-size to 1.25em, then it's ~18px on my side again.
assets/sass/fonts.scss
Outdated
| width: 1em; | ||
| height: 1em; |
There was a problem hiding this comment.
the icons are smaller than before, it should be 20x20px
There was a problem hiding this comment.
For me the icons (size of the ::before element) is 18x18, so it probably depends on the device settings for scaling. However, I increased the default width / height to 1.125rem to better match the previous state. Still not 100% the same, but I don't recognize a visual change.
| } | ||
|
|
||
| > .a-icon { | ||
| font-size: 1.15em; |
There was a problem hiding this comment.
the icon is larger than before, it should be 20x20px
| display: block; | ||
| position: absolute; | ||
| top: 23px; | ||
| top: 22px; |
There was a problem hiding this comment.
| top: 22px; | |
| top: 20px; |
| content: ""; | ||
| display: block; | ||
| width: 1.8rem; | ||
| height: 1.8rem; | ||
| background-color: currentColor; | ||
| mask: url("/icons/material-symbols-rounded/subdirectory_arrow_right.svg") | ||
| no-repeat center / contain; | ||
| position: absolute; | ||
| left: -2.4rem; |
There was a problem hiding this comment.
The icons are a little too high up. We should add:
top: 0;
To some extend the size of the icons will vary as it's not straightforward to represent the exact size of the font. To be able to control the size of the icons via the font-size attribute as it was done before (and also makes sense to match the current text size for in-line icons), I use the I'll check the mentioned issues and will fix them as best as possible, thanks for checking 👍
I couldn't find a way to derive the icon weight (different file) via the CSS weight / bold property. So for simplicity reasons, I removed the icons with weight 400 for now and just kept using the icons with weight 700; in comparison I liked them more. |
2 participants
Only load used icons via SVGs instead of loading all icons via the font (this is more efficient as GitHub pages has no proper caching)
Avoid rendering issues when using slow internet connections:
See also this discussion: #419 (comment)