-
Notifications
You must be signed in to change notification settings - Fork 163
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update imports to the variable font beta #4616
Update imports to the variable font beta #4616
Conversation
Ok it imports correctly now but I'm still not sure it uses the variable font as the |
Latest Percy build: https://percy.io/bb49709b/vanilla-framework/builds/23463708/ |
scss/_base_fontfaces.scss
Outdated
font-display: $font-display-option; | ||
font-family: 'Ubuntu'; | ||
font-style: normal; | ||
font-weight: $font-weight-regular-text; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ClementChaumel could you try remove this? Maybe this is making the font "fixed" on some font weight, instead of being flexible?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or we could specify range between 100 and 600?
https://developer.mozilla.org/en-US/docs/Web/CSS/@font-face/font-weight
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove it from fontfaces declarations, set it individually in each heading I would say
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is set on each heading, we need to let browser know which weights are supported by this particular font.
Hey, I added a little tester to display the difference in behaviour between a variable font from google font and ours One thing I noticed is that to import the font like that from GF you need to explicitly ask for a range using I don't know why the ubuntu font ignores the |
Fantastic! It finally solves the fake-weight issue when using Bold (700), and it renders crisp and clear on Ubuntu. It's just missing hinting for Windows (in the interpolated weights) and interpolation to Thin (100) in the Italic style. |
In https://vanilla-framework-4616.demos.haus/docs/base/typography#:~:text=Edit%20on%20CodePen-,Strong%20text,-Strong%20text |
@fitojb good catch, we need to point the $font-weight-bold variable to the 500 weight, it is currently set to 400. cc @ClementChaumel |
|
||
{% block content %} | ||
<style> | ||
@import url('https://fonts.googleapis.com/css2?family=Open+Sans:ital,wdth,wght@0,75..100,300..800;1,75..100,300..800&display=swap'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
drop the unnecessary import
<style> | ||
@import url('https://fonts.googleapis.com/css2?family=Open+Sans:ital,wdth,wght@0,75..100,300..800;1,75..100,300..800&display=swap'); | ||
|
||
.mono{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we need custom class names?
I guess we can simply use built-in stuff, like <em>
for italic and <code>
for mono?
<input id="weight" type="range" min="200" max="800" value="300" step="1" style="appearance: auto" oninput="doTheThing()"> | ||
<label>Width</label> | ||
<input id="width" type="range" min="75" max="100" value="100" step="1" style="appearance: auto" oninput="doTheThing()"> | ||
<label>Italic</label> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This doesn't seem to do anything, can we drop that?
TODO:
|
scss/_base_fontfaces.scss
Outdated
font-stretch: 100%; /* min and max value for the width axis, expressed as percentage */ | ||
font-style: italic; | ||
font-weight: 100 800; /* min and max value for the weight axis */ | ||
src: url('#{$assets-path}c1b12cdf-Ubuntu-Italic[wdth,wght]-latin (1).woff2') format('woff2-variations'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we avoid having unnecessary stuff in font URLs?
Having spaces and (1)
caused by downloading same name multiple times looks messy and may cause URL encoding issues.
I know that right now it means uploading it again probably, unless someone from backend guys can help us renaming those on asset server.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's important these URLs look clean and clear.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh my bad I didn't think it mattered. Do we want to agree on a format before I reupload them?
d77dbc2
to
16a41b9
Compare
Well done @ClementChaumel and @bartaz |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good to go!
Very happy with the improved weight distribution and proportions of the Mono font. Excellent overall |
@fitojb may i ask how can i get the fixed Ubuntu Mono font file, thanks |
@rink-grey Ubuntu fonts (including Mono) are available on Google fonts: https://fonts.google.com/?query=Ubuntu (although it may not be the latest variable font version yet) Latest available font files are in our repos https://github.com/canonical/Ubuntu-Sans-fonts https://github.com/canonical/Ubuntu-Sans-Mono-fonts |
@bartaz thanks very much! |
Done
Update imports to the variable font beta
The change seems not to be enough, when I look at the network tab I can see that the static font still gets downloaded instead of the variable one
Not sure where else I should make the change