fix(animations): ensure parent animations are triggered before children (angular#11201)

Author: matsko

modules/@angular/compiler/src/view_compiler/compile_view.ts

@@ -37,6 +37,7 @@ export class CompileView implements NameResolver {
 
   public classStatements: o.Statement[] = [];
   public createMethod: CompileMethod;
+  public animationBindingsMethod: CompileMethod;
   public injectorGetMethod: CompileMethod;
   public updateContentQueriesMethod: CompileMethod;
   public dirtyParentQueriesMethod: CompileMethod;
@@ -74,6 +75,7 @@ export class CompileView implements NameResolver {
       public animations: CompiledAnimationTriggerResult[], public viewIndex: number,
       public declarationElement: CompileElement, public templateVariableBindings: string[][]) {
     this.createMethod = new CompileMethod(this);
+    this.animationBindingsMethod = new CompileMethod(this);
     this.injectorGetMethod = new CompileMethod(this);
     this.updateContentQueriesMethod = new CompileMethod(this);
     this.dirtyParentQueriesMethod = new CompileMethod(this);

modules/@angular/compiler/src/view_compiler/property_binder.ts

@@ -105,6 +105,7 @@ function bindAndWriteToRenderer(
     var oldRenderValue: o.Expression = sanitizedValue(boundProp, fieldExpr);
     var renderValue: o.Expression = sanitizedValue(boundProp, currValExpr);
     var updateStmts: any[] /** TODO #9100 */ = [];
+    var compileMethod = view.detectChangesRenderPropertiesMethod;
     switch (boundProp.type) {
       case PropertyBindingType.Property:
         if (view.genConfig.logBindingUpdate) {
@@ -150,6 +151,8 @@ function bindAndWriteToRenderer(
           targetViewExpr = compileElement.appElement.prop('componentView');
         }
 
+        compileMethod = view.animationBindingsMethod;
+
         var animationFnExpr =
             targetViewExpr.prop('componentType').prop('animations').key(o.literal(animationName));
 
@@ -178,19 +181,12 @@ function bindAndWriteToRenderer(
             animationFnExpr.callFn([o.THIS_EXPR, renderNode, oldRenderValue, emptyStateValue])
                 .toStmt());
 
-        if (!_animationViewCheckedFlagMap.get(view)) {
-          _animationViewCheckedFlagMap.set(view, true);
-          var triggerStmt = o.THIS_EXPR.callMethod('triggerQueuedAnimations', []).toStmt();
-          view.afterViewLifecycleCallbacksMethod.addStmt(triggerStmt);
-          view.detachMethod.addStmt(triggerStmt);
-        }
-
         break;
     }
 
     bind(
-        view, currValExpr, fieldExpr, boundProp.value, context, updateStmts,
-        view.detectChangesRenderPropertiesMethod, view.bindings.length);
+        view, currValExpr, fieldExpr, boundProp.value, context, updateStmts, compileMethod,
+        view.bindings.length);
   });
 }
 

modules/@angular/compiler/src/view_compiler/view_builder.ts

@@ -574,12 +574,14 @@ function generateCreateMethod(view: CompileView): o.Statement[] {
 
 function generateDetectChangesMethod(view: CompileView): o.Statement[] {
   var stmts: any[] = [];
-  if (view.detectChangesInInputsMethod.isEmpty() && view.updateContentQueriesMethod.isEmpty() &&
+  if (view.animationBindingsMethod.isEmpty() && view.detectChangesInInputsMethod.isEmpty() &&
+      view.updateContentQueriesMethod.isEmpty() &&
       view.afterContentLifecycleCallbacksMethod.isEmpty() &&
       view.detectChangesRenderPropertiesMethod.isEmpty() &&
       view.updateViewQueriesMethod.isEmpty() && view.afterViewLifecycleCallbacksMethod.isEmpty()) {
     return stmts;
   }
+  ListWrapper.addAll(stmts, view.animationBindingsMethod.finish());
   ListWrapper.addAll(stmts, view.detectChangesInInputsMethod.finish());
   stmts.push(
       o.THIS_EXPR.callMethod('detectContentChildrenChanges', [DetectChangesVars.throwOnChange])

modules/@angular/core/src/animation/animation_queue.ts

@@ -0,0 +1,25 @@
+/**
+ * @license
+ * Copyright Google Inc. All Rights Reserved.
+ *
+ * Use of this source code is governed by an MIT-style license that can be
+ * found in the LICENSE file at https://angular.io/license
+ */
+
+import {AnimationPlayer} from './animation_player';
+
+var _queuedAnimations: AnimationPlayer[] = [];
+
+/** @internal */
+export function queueAnimation(player: AnimationPlayer) {
+  _queuedAnimations.push(player);
+}
+
+/** @internal */
+export function triggerQueuedAnimations() {
+  for (var i = 0; i < _queuedAnimations.length; i++) {
+    var player = _queuedAnimations[i];
+    player.play();
+  }
+  _queuedAnimations = [];
+}

modules/@angular/core/src/linker/view.ts

@@ -9,6 +9,7 @@
 import {AnimationGroupPlayer} from '../animation/animation_group_player';
 import {AnimationOutput} from '../animation/animation_output';
 import {AnimationPlayer, NoOpAnimationPlayer} from '../animation/animation_player';
+import {queueAnimation} from '../animation/animation_queue';
 import {AnimationTransitionEvent} from '../animation/animation_transition_event';
 import {ViewAnimationMap} from '../animation/view_animation_map';
 import {ChangeDetectorRef, ChangeDetectorStatus} from '../change_detection/change_detection';
@@ -84,23 +85,18 @@ export abstract class AppView<T> {
   queueAnimation(
       element: any, animationName: string, player: AnimationPlayer, totalTime: number,
       fromState: string, toState: string): void {
+    queueAnimation(player);
     var event = new AnimationTransitionEvent(
         {'fromState': fromState, 'toState': toState, 'totalTime': totalTime});
     this.animationPlayers.set(element, animationName, player);
+
     player.onDone(() => {
       // TODO: make this into a datastructure for done|start
       this.triggerAnimationOutput(element, animationName, 'done', event);
       this.animationPlayers.remove(element, animationName);
     });
-    player.onStart(() => { this.triggerAnimationOutput(element, animationName, 'start', event); });
-  }
 
-  triggerQueuedAnimations() {
-    this.animationPlayers.getAllPlayers().forEach(player => {
-      if (!player.hasStarted()) {
-        player.play();
-      }
-    });
+    player.onStart(() => { this.triggerAnimationOutput(element, animationName, 'start', event); });
   }
 
   triggerAnimationOutput(

modules/@angular/core/src/linker/view_ref.ts

@@ -6,11 +6,14 @@
  * found in the LICENSE file at https://angular.io/license
  */
 
+import {triggerQueuedAnimations} from '../animation/animation_queue';
 import {ChangeDetectorRef} from '../change_detection/change_detector_ref';
 import {ChangeDetectorStatus} from '../change_detection/constants';
 import {unimplemented} from '../facade/errors';
+
 import {AppView} from './view';
 
+
 /**
  * @stable
  */
@@ -104,7 +107,10 @@ export class ViewRef_<C> implements EmbeddedViewRef<C>, ChangeDetectorRef {
 
   markForCheck(): void { this._view.markPathToRootAsCheckOnce(); }
   detach(): void { this._view.cdMode = ChangeDetectorStatus.Detached; }
-  detectChanges(): void { this._view.detectChanges(false); }
+  detectChanges(): void {
+    this._view.detectChanges(false);
+    triggerQueuedAnimations();
+  }
   checkNoChanges(): void { this._view.detectChanges(true); }
   reattach(): void {
     this._view.cdMode = this._originalMode;

modules/@angular/core/test/animation/animation_integration_spec.ts

@@ -30,6 +30,8 @@ export function main() {
 function declareTests({useJit}: {useJit: boolean}) {
   describe('animation tests', function() {
     beforeEach(() => {
+      InnerContentTrackingAnimationPlayer.initLog = [];
+
       TestBed.configureCompiler({useJit: useJit});
       TestBed.configureTestingModule({
         declarations: [DummyLoadingCmp, DummyIfCmp],
@@ -961,6 +963,85 @@ function declareTests({useJit}: {useJit: boolean}) {
            var player = <InnerContentTrackingAnimationPlayer>animation['player'];
            expect(player.playAttempts).toEqual(1);
          }));
+
+      it('should always trigger animations on the parent first before starting the child',
+         fakeAsync(() => {
+           TestBed.overrideComponent(DummyIfCmp, {
+             set: {
+               template: `
+              <div *ngIf="exp" [@outer]="exp">
+                outer
+                <div *ngIf="exp2" [@inner]="exp">
+                  inner 
+<               </div>
+<             </div>
+            `,
+               animations: [
+                 trigger('outer', [transition('* => *', [animate(1000)])]),
+                 trigger('inner', [transition('* => *', [animate(1000)])]),
+               ]
+             }
+           });
+
+           const driver = TestBed.get(AnimationDriver) as InnerContentTrackingAnimationDriver;
+           let fixture = TestBed.createComponent(DummyIfCmp);
+           var cmp = fixture.debugElement.componentInstance;
+           cmp.exp = true;
+           cmp.exp2 = true;
+           fixture.detectChanges();
+           flushMicrotasks();
+
+           expect(driver.log.length).toEqual(2);
+           var inner: any = driver.log.pop();
+           var innerPlayer: any = <InnerContentTrackingAnimationPlayer>inner['player'];
+           var outer: any = driver.log.pop();
+           var outerPlayer: any = <InnerContentTrackingAnimationPlayer>outer['player'];
+
+           expect(InnerContentTrackingAnimationPlayer.initLog).toEqual([
+             outerPlayer.element, innerPlayer.element
+           ]);
+         }));
+
+      it('should trigger animations that exist in nested views even if a parent embedded view does not contain an animation',
+         fakeAsync(() => {
+           TestBed.overrideComponent(DummyIfCmp, {
+             set: {
+               template: `
+              <div *ngIf="exp" [@outer]="exp">
+                outer
+                <div *ngIf="exp">
+                  middle
+                  <div *ngIf="exp2" [@inner]="exp">
+                    inner
+                  </div>
+<               </div>
+<             </div>
+            `,
+               animations: [
+                 trigger('outer', [transition('* => *', [animate(1000)])]),
+                 trigger('inner', [transition('* => *', [animate(1000)])]),
+               ]
+             }
+           });
+
+           const driver = TestBed.get(AnimationDriver) as InnerContentTrackingAnimationDriver;
+           let fixture = TestBed.createComponent(DummyIfCmp);
+           var cmp = fixture.debugElement.componentInstance;
+           cmp.exp = true;
+           cmp.exp2 = true;
+           fixture.detectChanges();
+           flushMicrotasks();
+
+           expect(driver.log.length).toEqual(2);
+           var inner: any = driver.log.pop();
+           var innerPlayer: any = <InnerContentTrackingAnimationPlayer>inner['player'];
+           var outer: any = driver.log.pop();
+           var outerPlayer: any = <InnerContentTrackingAnimationPlayer>outer['player'];
+
+           expect(InnerContentTrackingAnimationPlayer.initLog).toEqual([
+             outerPlayer.element, innerPlayer.element
+           ]);
+         }));
     });
 
     describe('animation output events', () => {
@@ -1714,17 +1795,23 @@ class InnerContentTrackingAnimationDriver extends MockAnimationDriver {
 }
 
 class InnerContentTrackingAnimationPlayer extends MockAnimationPlayer {
+  static initLog: any[] = [];
+
   constructor(public element: any) { super(); }
 
   public computedHeight: number;
   public capturedInnerText: string;
   public playAttempts = 0;
 
-  init() { this.computedHeight = getDOM().getComputedStyle(this.element)['height']; }
+  init() {
+    InnerContentTrackingAnimationPlayer.initLog.push(this.element);
+    this.computedHeight = getDOM().getComputedStyle(this.element)['height'];
+  }
 
   play() {
     this.playAttempts++;
-    this.capturedInnerText = this.element.querySelector('.inner').innerText;
+    var innerElm = this.element.querySelector('.inner');
+    this.capturedInnerText = innerElm ? innerElm.innerText : '';
   }
 }