Animations: CSS calc/var polyfill parser #9287
Conversation
| set = nodeOrSet; | ||
| } else { | ||
| set = new CssConcatNode(); | ||
| set.array_.push(nodeOrSet); |
There was a problem hiding this comment.
Nit: pass this as a constructor argument.
| set.array_.push(nodeOrSet); | ||
| } | ||
| if (otherNodeOrSet instanceof CssConcatNode) { | ||
| set.array_.concat(otherNodeOrSet.array_); |
There was a problem hiding this comment.
Nit: implement this as a instance method.
There was a problem hiding this comment.
I'd like to avoid construction and state leaking into public methods of AST nodes. I'd rather it all be fully compressed to this module only and preserve full obfuscation of these details on compiler level.
| super(); | ||
| /** @const @private {string} */ | ||
| this.name_ = endsWith(name, '(') ? | ||
| name.substring(0, name.length - 1) : name; |
| * A CSS `calc()` expression: `calc(100px)`, `calc(80vw - 30em)`, etc. | ||
| * See https://drafts.csswg.org/css-values-3/#calc-notation. | ||
| */ | ||
| export class CssCalcNode extends CssNode { |
There was a problem hiding this comment.
Isn't this just a CssFuncNode?
There was a problem hiding this comment.
Technically no. calc() and var() are expressions - they resolve in actual values. While translate(), rotate() and such are actual CSS functions. The formats within are very different, e.g. + and - are only possible in calc() and no , args. Implementation-wise they will also differ a lot with calc() and var() actually resolving to final values, e.g. 100px.
| /** | ||
| * A CSS `calc()` product expression: `100px * 2`, `80vw / 2`, etc. | ||
| */ | ||
| export class CssCalcProductNode extends CssNode { |
There was a problem hiding this comment.
Nit: can we join this with the above and call them CssCalcBinaryNode?
There was a problem hiding this comment.
I could and may still do so later. But implementation-wise per spec they are super different. So not sure if I'll win anything if I do at this time.
| Y [Yy] | ||
| Z [Zz] | ||
|
|
||
| num [+-]?[0-9]+(\.[0-9]+)?([eE][+\-]?[0-9]+)?|[+-]?"."[0-9]+([eE][+\-]?[0-9]+)? |
There was a problem hiding this comment.
Fixed. These and all others actually come directly from CSS spec (https://www.w3.org/TR/CSS22/grammar.html) and such.
| {num}{M}{S} return 'TIME_MS' | ||
| {num}{S} return 'TIME_S' | ||
|
|
||
| {num}"%" return 'PERCENTAGE' |
| Z [Zz] | ||
|
|
||
| num [+-]?[0-9]+(\.[0-9]+)?([eE][+\-]?[0-9]+)?|[+-]?"."[0-9]+([eE][+\-]?[0-9]+)? | ||
| hex [a-fA-F0-9]+ |
There was a problem hiding this comment.
Should we make this stricter? [a-fA-F0-9]{3}|[a-fA-F0-9]{6}?
There was a problem hiding this comment.
CSS spec is super relaxed about colors for some reason. So, i'd rather keep.
| constructor(name, args) { | ||
| super(); | ||
| /** @const @private {string} */ | ||
| this.name_ = endsWith(name, '(') ? |
There was a problem hiding this comment.
Why can't we parse out the (?
|
PTAL |
| set = new CssConcatNode([nodeOrSet]); | ||
| } | ||
| if (otherNodeOrSet instanceof CssConcatNode) { | ||
| set.array_.concat(otherNodeOrSet.array_); |
There was a problem hiding this comment.
this needs to be reassigned to set.array_
also concat works with non-array single values, so two cases could be merged.
There was a problem hiding this comment.
Great catch! Fixed and added tests for this as well.
| expect(parsePseudo('10s')).to.equal('TME<10 S>'); | ||
| }); | ||
|
|
||
| it('should parse url', () => { |
There was a problem hiding this comment.
I believe validator has special rules for valid values of url, need to apply the same rules, either at parse or eval time (being being passed through)
There was a problem hiding this comment.
Fixed. We simply need to control that this is not a http URL.
|
@aghassemi thanks! ptal. |
Partial for #9129.
This PR is only concerned with grammar and parser. Evaluations will be added in the follow-up PRs.