Replace Google Font with local variable font #1910
Conversation
✅ Deploy Preview for expressjscom-preview ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
utkarsh125
changed the title
There was a problem hiding this comment.
Please remove unused font files and folders. Also add local font names for all fonts, see suggestions below.
I tried to find the
fontsource-variablefonts foropen-sansbut I was not able to find
You can use variable fonts from this repo. This will solve the problem with woff file size. See fontsource/fontsource#773 for more info.
ShubhamOulkar
changed the title
There was a problem hiding this comment.
LGTM 👍 manually tested on Firefox and Chrome. Fonts are correctly applied and rendering as expected.
Thanks @utkarsh125, for the changes.
|
Thank you @ShubhamOulkar for pointing out mistakes and guiding me on how to fix them. It was a good learning experience for me, hope to contribute more in the future :D |
css/style.css
Outdated
| @@ -1,3 +1,5 @@ | |||
| @import url("../fonts/open-sans/fonts.css"); | |||
There was a problem hiding this comment.
This must be imported from the head
using @import is considered bad practice.
There was a problem hiding this comment.
You’re right. I was initially planning to use the {page.lang}.css file to import fonts, similar to how it's done for the Korean font. However, since the font is shared across multiple languages, it doesn’t make sense to repeat the same @font-face rule in each language-specific CSS file.
I suggest defining @font-face directly in style.css instead. WDYT?
Edit:- It is better to write @font-face rule in ko.css file.
There was a problem hiding this comment.
I’d prefer to keep the font imports separate, that file is too large. The location of the file is fine in my opinion, but it should be in the HEAD.
Edit: By the way, @ShubhamOulkar wrap font-face in backticks (` `) so we don’t ping other people unnecessarily—GitHub stuff.
There was a problem hiding this comment.
it should be in the HEAD.
Ok, then the only option is to use one more <link>.
There was a problem hiding this comment.
@ShubhamOulkar @bjohansebas I've added this in HTML file
<link
rel="preload"
href="/fonts/open-sans/OpenSans.woff2"
as="font"
type="font/woff2"
crossorigin
/>
<link
rel="preload"
href="/fonts/open-sans/OpenSans-Italic.woff2"
as="font"
type="font/woff2"
crossorigin
/>
<link
href="/fonts/open-sans/OpenSans.woff"
as="font"
type="font/woff"
crossorigin
/>
<link
href="/fonts/open-sans/woff/OpenSans-Italic.woff"
as="font"
type="font/woff"
crossorigin
/>
We can preload woff2 since the file since is smaller, and use woff as fallback. apart from that I have also removed @imports. I've kept the fonts separate from the main style.css (so only the import line is removed).
|
thanks @utkarsh125 |
|
You're welcome @bjohansebas looking forward to contributing more in future :D |
fixes: #1750
I have installed
fontsourceOpen Sans and removed the script link for Google's Open Sans as discussed in the issue.Packages installed
If any more changes are needed I would be happy to make changes