-
-
Notifications
You must be signed in to change notification settings - Fork 25
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
Support font-weight, font-style and font-stretch @font-face properties #264
base: main
Are you sure you want to change the base?
Conversation
…s as they can impact the font metrics
# Conflicts: # playground/index.html # src/transform.ts # test/e2e.spec.ts
I see there's no enough test coverage for the new code I suppose but I am not entirely sure what tests should I implement. Can you point me, please? |
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 looks really good. But it doesn't seem to be working in the playground (as in, actually reducing layout shift). Am I just missing something?
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #264 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 2 2
Lines 197 230 +33
Branches 31 37 +6
=========================================
+ Hits 197 230 +33
☔ View full report in Codecov by Sentry. |
@danielroe looking into this now, thanks for the tweaks! |
I have missed the Roboto and Inter Does this mean that this PR is useless and the way it worked previously is enough? @font-face {
font-family: "Inter fallback";
src: local("Arial");
size-adjust: 107.4014%;
ascent-override: 90.199%;
descent-override: 22.4836%;
line-gap-override: 0%;
}
@font-face {
font-family: 'Inter';
font-display: swap;
src: url('https://fonts.gstatic.com/s/inter/v12/UcC73FwrK3iLTeHuS_fvQtMwCp50KnMa1ZL7.woff2')
format('woff2');
}
@font-face {
font-family: "Inter fallback";
src: local("Arial");
size-adjust: 107.4014%;
ascent-override: 90.199%;
descent-override: 22.4836%;
line-gap-override: 0%;
font-weight: 700;
}
@font-face {
font-family: 'Inter';
font-weight: 700;
font-display: swap;
src: url('https://fonts.gstatic.com/s/inter/v12/UcCO3FwrK3iLTeHuS_fvQtMwCp50KnMw2boKoduKmMEVuFuYAZ9hiJ-Ek-_EeA.woff2')
format('woff2');
} |
This may be an upstream issue to raise in capsize in that case. We can pend this PR until there are different metrics available for different weights. |
In theory, there is a way to obtain different metrics for different font weights e.g. through dropping each font file here and collecting relevant metrics from the 'Data' tab. However, as Daniel mentioned, it would be much nicer if capsize provided these alongside the already existing metrics for the normal font weight. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #264 +/- ##
==========================================
+ Coverage 50.21% 54.08% +3.87%
==========================================
Files 4 4
Lines 235 257 +22
Branches 33 40 +7
==========================================
+ Hits 118 139 +21
- Misses 117 118 +1 ☔ View full report in Codecov by Sentry. |
Support font-weight, font-style, and font-stretch @font-face properties as discussed in #53