From e25f05ae7c9e8012ccfbd37d6ea49e126735b8a2 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Matias=20Niemel=C3=A4?= <matias@yearofmoo.com>
Date: Fri, 4 Aug 2017 15:10:47 -0700
Subject: [PATCH] fix(animations): make sure @.disabled respects disabled
 parent/sub animation sequences (#18715)

Prior to this fix if @parent and @child animations ran at the same
time within a disabled region then there was a chance that a @child
sub animation would never complete. This would cause *directives to
never close a removal when a @child trigger was placed on them. This
patch fixes this issue.

PR Close #18715
---
 .../src/render/transition_animation_engine.ts | 17 ++++-
 .../animation/animation_integration_spec.ts   | 70 +++++++++++++++++++
 2 files changed, 84 insertions(+), 3 deletions(-)

diff --git a/packages/animations/browser/src/render/transition_animation_engine.ts b/packages/animations/browser/src/render/transition_animation_engine.ts
index a72eba42a3..2b7a97e4f8 100644
--- a/packages/animations/browser/src/render/transition_animation_engine.ts
+++ b/packages/animations/browser/src/render/transition_animation_engine.ts
@@ -1016,11 +1016,20 @@ export class TransitionAnimationEngine {
       } else {
         eraseStyles(element, instruction.fromStyles);
         player.onDestroy(() => setStyles(element, instruction.toStyles));
+        // there still might be a ancestor player animating this
+        // element therefore we will still add it as a sub player
+        // even if its animation may be disabled
         subPlayers.push(player);
+        if (disabledElementsSet.has(element)) {
+          skippedPlayers.push(player);
+        }
       }
     });
 
+    // find all of the sub players' corresponding inner animation player
     subPlayers.forEach(player => {
+      // even if any players are not found for a sub animation then it
+      // will still complete itself after the next tick since it's Noop
       const playersForElement = skippedPlayersMap.get(player.element);
       if (playersForElement && playersForElement.length) {
         const innerPlayer = optimizeGroupPlayer(playersForElement);
@@ -1052,7 +1061,7 @@ export class TransitionAnimationEngine {
       // until that animation is over (or the parent queried animation)
       if (details && details.hasAnimation) continue;
 
-      let players: AnimationPlayer[] = [];
+      let players: TransitionAnimationPlayer[] = [];
 
       // if this element is queried or if it contains queried children
       // then we want for the element not to be removed from the page
@@ -1071,8 +1080,10 @@ export class TransitionAnimationEngine {
           }
         }
       }
-      if (players.length) {
-        removeNodesAfterAnimationDone(this, element, players);
+
+      const activePlayers = players.filter(p => !p.destroyed);
+      if (activePlayers.length) {
+        removeNodesAfterAnimationDone(this, element, activePlayers);
       } else {
         this.processLeaveNode(element);
       }
diff --git a/packages/core/test/animation/animation_integration_spec.ts b/packages/core/test/animation/animation_integration_spec.ts
index 6f94ad690e..9694c7fc59 100644
--- a/packages/core/test/animation/animation_integration_spec.ts
+++ b/packages/core/test/animation/animation_integration_spec.ts
@@ -2727,6 +2727,76 @@ export function main() {
           fixture.detectChanges();
           expect(getLog().length).toEqual(0);
         });
+        
+        it('should respect parent/sub animations when the respective area in the DOM is disabled',
+           fakeAsync(() => {
+             @Component({
+               selector: 'parent-cmp',
+               animations: [
+                 trigger(
+                     'parent',
+                     [
+                       transition(
+                           '* => empty',
+                           [
+                             style({opacity: 0}),
+                             query(
+                                 '@child',
+                                 [
+                                   animateChild(),
+                                 ]),
+                             animate('1s', style({opacity: 1})),
+                           ]),
+                     ]),
+                 trigger(
+                     'child',
+                     [
+                       transition(
+                           ':leave',
+                           [
+                             animate('1s', style({opacity: 0})),
+                           ]),
+                     ]),
+               ],
+               template: `
+              <div [@.disabled]="disableExp" #container>
+                <div [@parent]="exp" (@parent.done)="onDone($event)">
+                  <div class="item" *ngFor="let item of items" @child (@child.done)="onDone($event)"></div>
+                </div>
+              </div>
+                `
+             })
+             class Cmp {
+               @ViewChild('container') public container: any;
+
+               disableExp = false;
+               exp = '';
+               items: any[] = [];
+               doneLog: any[] = [];
+
+               onDone(event: any) { this.doneLog.push(event); }
+             }
+
+             TestBed.configureTestingModule({declarations: [Cmp]});
+             const engine = TestBed.get(ɵAnimationEngine);
+             const fixture = TestBed.createComponent(Cmp);
+             const cmp = fixture.componentInstance;
+             cmp.disableExp = true;
+             cmp.items = [0, 1, 2, 3, 4];
+             fixture.detectChanges();
+             flushMicrotasks();
+
+             cmp.exp = 'empty';
+             cmp.items = [];
+             cmp.doneLog = [];
+             fixture.detectChanges();
+             flushMicrotasks();
+
+             const elms = cmp.container.nativeElement.querySelectorAll('.item');
+             expect(elms.length).toEqual(0);
+
+             expect(cmp.doneLog.length).toEqual(6);
+           }));
       });
     });