Add font reification #731
Add font reification #731
Conversation
| :: | ||
| :: For both specified and computed values: | ||
|
|
||
| 1. If the value is <css>none</css>, <css>weight</css>, <css>style</css> or <css>small-caps</css>, |
There was a problem hiding this comment.
The syntax is none | [ weight || style || small-caps ]. Is this correct? I wonder what will happen when we support values like weight style. Might be some ergonomic issues (sorry, I said backwards compat issue before, but realised it's backwards compatible).
There was a problem hiding this comment.
Specifically, I mean if we reify weight style as a list, then I have to write code like:
// Want to know if 'small-caps' is synthesized.
const result = styleMap.get('font-synthesis');
if (result is keyword with value 'small-caps' or result is a list containing value 'small-caps')
instead of just
if (result is a list containing value 'small-caps')
There was a problem hiding this comment.
No, "weight style" should definitely be a CSSStyleValue; it's a single value with two keywords in it, not a list-valued property.
There was a problem hiding this comment.
What about weight by itself? Basically, should weight and weight style reify to two different data structures (e.g. CSSKeywordValue and CSSPairValue), or the same data structure (e.g. CSSValueSet)? I think it might be more convenient if they reified to the same data structure (e.g. if you want to detect whether a font-synthesis contains small-caps, you don't need separate cases for single vs multiple keyword). WDYT?
There was a problem hiding this comment.
In an ideal world where we're designing all of the data structures that CSS will ever need all at once, yeah, we'd give the whole property a specialized data structure that works the same whether you specify one keyword or several.
Unfortunately, we're in the world where CSS is always evolving. Properties that once took single keywords might becomes multi-keyword in the future; we can't backwards-compatibly upgrade them to a new type of object.
That said, I wonder if we should make this sort of upgrade possible. I just raised #735 for this.
There was a problem hiding this comment.
Ah yes that makes sense. We can just rely on the upgrade mechanism here.
css-typed-om/Overview.bs
Outdated
| :: | ||
| :: For both specified and computed values, | ||
| reify as a {{CSSStyleValue}} | ||
| and return the result. | ||
|
|
||
| : <a>font-variant-alternates</a> | ||
| :: |
There was a problem hiding this comment.
Same issue as font-synthesis.
|
|
||
| : <a>font-variation-settings</a> | ||
| :: | ||
| :: For both specified and computed values: |
There was a problem hiding this comment.
The syntax is none | <something>#, so it's neither list-valued nor single-valued. I think reification-wise it's fine, but the wording for styleMap.append etc. might be hard (can append <something> if it's not none, can never append none). Alternatively, we can always represent the list values as a separate type (like transform which suffers the same problem), but it means we can never append to it directly.
There was a problem hiding this comment.
Ah, I figured we probably had an example of this somewhere; the 'animation' handling of "none" seemed sufficiently weird that I was pretty sure somewhere wouldn't have gone for it.
There was a problem hiding this comment.
(It's "normal", not "none".)
Let's leave this off for the moment - we can just reify to a CSSStyleValue until we figure it out. I suspect this'll be something we want to run by the group and get opinions on.
|
Hi @tabatkins , can you PTAL? |
|
Other than font-variation-settings, looks good. |
|
Fixed |
Hi @tabatkins , PTAL. I got some questions too.