-
Notifications
You must be signed in to change notification settings - Fork 29
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
Use browserslist to configure which font formats to supply subsets and fallbacks in, and whether to add the JS-based preload polyfill #120
Conversation
Pull Request Test Coverage Report for Build 535
💛 - Coveralls |
ed45a7d
to
0e1e6fb
Compare
0e1e6fb
to
2bf7cf4
Compare
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.
I thought the implementation would require much more complexity :)
I only have reservations about the CLI interface
lib/parseCommandLineOptions.js
Outdated
type: 'array', | ||
choices: ['woff2', 'woff', 'truetype'], | ||
}) | ||
.options('js-preload', { |
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.
I'm not sure we should expose this option. I'd rather promote usage of browserslist project configuration to have the tool just do the right thing
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.
Hmm, yeah, I agree. I've taken it out in dccddcf
formats = ['woff2']; | ||
if ( | ||
_.intersection( | ||
browsersList('supports woff, not supports woff2'), |
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.
I haven't seen this interface before. That's pretty awesome!
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.
Yeah, it's pretty nifty. A shame that we have to use lodash to combine it with the user's query though.
Co-authored-by: Peter Müller <munter@fumle.dk>
@Munter, good to go in? |
Yeah. Let's merge |
Fixes #119
The current set of default browsers happens to result in
woff
+woff2
and the JS preload polyfill being selected atm., which is luckily the same as our current--formats
default 🎉TODO: