Conversation
| /** @const @private */ | ||
| this.css_ = new CssContextImpl(win, rootNode, baseUrl); | ||
|
|
||
| /** @const @private */ |
There was a problem hiding this comment.
Why passing the vsync instance as opposed to vsyncFor(win)?
There was a problem hiding this comment.
Easier to test. This is an internal class - the main reason is to split the code and test.
| * @param {?WebAnimationTimingDef} timing | ||
| */ | ||
| constructor(win, rootNode, baseUrl, validate) { | ||
| constructor(builder, css, path, target, vars, timing) { |
There was a problem hiding this comment.
It's not optional. Only nullable.
| return new WebAnimationRunner(this.requests_); | ||
| }); | ||
| /** @const @private {!Array<!Promise>} */ | ||
| this.deps_ = []; |
There was a problem hiding this comment.
Dependencies to fully resolve all animation requests. Some can only be done asynchronously. I added the description.
| this.resolveTargets_(spec) : | ||
| [null]; | ||
| targets.forEach(target => { | ||
| this.target_ = target || prevTarget; |
There was a problem hiding this comment.
why does this fallback to prevTarget?
There was a problem hiding this comment.
prevTarget is basically the "parent" target. Either a nested animation overrides it or the "parent" target is used. The mergeVars_ and mergeTiming_ that follow, essentially do the same thing.
| <amp-animation id="anim1" layout="nodisplay"> | ||
| <script type="application/json"> | ||
| { | ||
| "animamtion": "anim2", |
| <script type="application/json"> | ||
| { | ||
| "selector": ".target-class", | ||
| "animamtion": "anim2", |
| ``` | ||
|
|
||
|
|
||
| ### Animation composition |
There was a problem hiding this comment.
Can this clarify further on the difference between setting animation as a reference vs. animations? Some confusion can come from the fact that they can be described top-level or nested one level down.
There was a problem hiding this comment.
Added possible reasons. Similar to function call composition, two reasons: allow reuse across multiple animations (as with calling function from different places) or simply making each animation more manageable (as with splitting unwieldy function into two, even if there's no reuse).
|
@alanorozco All comments addressed. PTAL. |
A relatively small refactoring to allow animations to "include" other animations. The format extensions are described in the modified
amp-animation.md.Partial for #9129.