-
-
Notifications
You must be signed in to change notification settings - Fork 13
feat: formats #297
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
base: main
Are you sure you want to change the base?
feat: formats #297
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #297 +/- ##
==========================================
+ Coverage 98.95% 99.02% +0.07%
==========================================
Files 11 11
Lines 480 515 +35
Branches 119 132 +13
==========================================
+ Hits 475 510 +35
Misses 5 5 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| subsets: ['latin'], | ||
| weights: ['400'], | ||
| }) | ||
| expect(fonts.length).toBe(1) |
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 have an example of a provider returning several different formats we could test?
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.
We have the case with the google provider I think, see https://github.com/unjs/unifont/pull/297/files#diff-cbf7cd7589545d91f982b1cd036d597e97e37f683390ea213dc545d82081f003R270
| const styles = [ | ||
| 'oblique 0deg 15deg', | ||
| 'normal', | ||
| ] as ResolveFontOptions['styles'] |
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.
is there a reason we're not testing the different styles/weights?
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.
IIRC that's because since it's now woff2 only, there's no woff fallback with more classic values. Since this test is about testing variable axis, not the fallbakcs, I thought it'd be fine to remove. I can restore it and specify formats: ['woff2', 'woff'] if you want! No strong opinion
danielroe
left a comment
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 is phenomenal! great work.
just a few quick questions.
formats: FontFormat[]toResolveFontOptions(defaults to just woff2)cleanFontFaces()utilityformat(eg. ttf to truetype)