Support styled @font-face generation #152
Conversation
Codecov Report
@@ Coverage Diff @@
## master #152 +/- ##
==========================================
+ Coverage 47.72% 52.94% +5.21%
==========================================
Files 9 9
Lines 132 153 +21
Branches 23 32 +9
==========================================
+ Hits 63 81 +18
- Misses 69 72 +3
Continue to review full report at Codecov.
|
Codecov Report
@@ Coverage Diff @@
## master #152 +/- ##
==========================================
+ Coverage 47.72% 52.94% +5.21%
==========================================
Files 9 9
Lines 132 153 +21
Branches 23 32 +9
==========================================
+ Hits 63 81 +18
- Misses 69 72 +3
Continue to review full report at Codecov.
|
2 similar comments
Codecov Report
@@ Coverage Diff @@
## master #152 +/- ##
==========================================
+ Coverage 47.72% 52.94% +5.21%
==========================================
Files 9 9
Lines 132 153 +21
Branches 23 32 +9
==========================================
+ Hits 63 81 +18
- Misses 69 72 +3
Continue to review full report at Codecov.
|
Codecov Report
@@ Coverage Diff @@
## master #152 +/- ##
==========================================
+ Coverage 47.72% 52.94% +5.21%
==========================================
Files 9 9
Lines 132 153 +21
Branches 23 32 +9
==========================================
+ Hits 63 81 +18
- Misses 69 72 +3
Continue to review full report at Codecov.
|
| export function getFontConfig( | ||
| withStyleOverloads: boolean, | ||
| preloadOverrides?: {} | ||
| ) { |
There was a problem hiding this comment.
Is there a concrete type for this config object that folks can use? Ideally they could write their own and Flow would complain at the source if they did something like:
import type {FontConfigType} from '...';
const config: FontConfigType = {
...
};Right now, the fonts property looks like it'll be a union of AtomicFontsObjectType and StyledFontsObjectType which may be more clear for consumers if given an overall type for the entire config.
src/generate-font-faces.js
Outdated
| function asFontFaceSrc(urls) { | ||
| // `urls` is a dictionary of font types (woff, woff2 etc) to url string | ||
| return Object.keys(urls).map( | ||
| type => `url("${urls[type]}") format("${type}")\n` | ||
| // $FlowFixMe |
There was a problem hiding this comment.
Hm, curious as to why this is necessary?
There was a problem hiding this comment.
I'm not entirely sure but it relates to
type FontURLsType = {
woff?: string,
woff2: string,
};Because woff is an optional key it is somehow inferring that urls(type) might be undefined.
There was a problem hiding this comment.
Ah, that's annoying. I think this is because an object with the propery woff: undefined satisfies the type constraints and would fail the urls[type] coercion because it may be undefined. Something like this:
const urls: FontURLsType = {
woff: undefined,
woff2: "hello"
};
console.log(Object.keys(urls)); // ['woff', 'woff2'];If you want to get rid of this $FlowFixMe you could wrap the offending code in a String constructor:
type => `url("${String(urls[type])}") format("${type}")`which should resolve this.
| }; | ||
|
|
||
| export type ConfigType = { |
There was a problem hiding this comment.
Ah, nevermind. Here it is! Disregard earlier comment, assuming we export this in index.js as well.
|
Triggered Fusion.js build verification: https://buildkite.com/uberopensource/fusion-release-verification/builds/2030 |
No description provided.