From a6fb78ee3c2bcb301bad3cf631f44e187b27ce05 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matias=20Niemel=C3=A4?= Date: Fri, 17 Mar 2017 12:31:51 -0700 Subject: [PATCH] fix(animations): make sure non-transitioned leave operations cancel existing animations (#15254) Closes #15213 PR Close #15254 --- .../src/render/dom_animation_engine.ts | 10 +++-- .../animation/animation_integration_spec.ts | 45 +++++++++++++++++++ 2 files changed, 51 insertions(+), 4 deletions(-) diff --git a/packages/animations/browser/src/render/dom_animation_engine.ts b/packages/animations/browser/src/render/dom_animation_engine.ts index 9f7e9dfe90..c92deae667 100644 --- a/packages/animations/browser/src/render/dom_animation_engine.ts +++ b/packages/animations/browser/src/render/dom_animation_engine.ts @@ -28,7 +28,8 @@ export interface TriggerListenerTuple { callback: (event: any) => any; } -const MARKED_FOR_ANIMATION = 'ng-animate'; +const MARKED_FOR_ANIMATION_CLASSNAME = 'ng-animating'; +const MARKED_FOR_ANIMATION_SELECTOR = '.ng-animating'; const MARKED_FOR_REMOVAL = '$$ngRemove'; export class DomAnimationEngine { @@ -92,6 +93,7 @@ export class DomAnimationEngine { element[MARKED_FOR_REMOVAL] = true; this._queuedRemovals.set(element, () => {}); } + this._onRemovalTransition(element).forEach(player => player.destroy()); domFn(); } @@ -162,7 +164,7 @@ export class DomAnimationEngine { // when a parent animation is set to trigger a removal we want to // find all of the children that are currently animating and clear // them out by destroying each of them. - const elms = element.querySelectorAll(MARKED_FOR_ANIMATION); + const elms = element.querySelectorAll(MARKED_FOR_ANIMATION_SELECTOR); for (let i = 0; i < elms.length; i++) { const elm = elms[i]; const activePlayers = this._activeElementAnimations.get(elm); @@ -300,8 +302,8 @@ export class DomAnimationEngine { this._queuedTransitionAnimations.push(tuple); player.init(); - element.classList.add(MARKED_FOR_ANIMATION); - player.onDone(() => { element.classList.remove(MARKED_FOR_ANIMATION); }); + element.classList.add(MARKED_FOR_ANIMATION_CLASSNAME); + player.onDone(() => { element.classList.remove(MARKED_FOR_ANIMATION_CLASSNAME); }); } private _flushQueuedAnimations() { diff --git a/packages/core/test/animation/animation_integration_spec.ts b/packages/core/test/animation/animation_integration_spec.ts index 15c6559b29..b1c320b9df 100644 --- a/packages/core/test/animation/animation_integration_spec.ts +++ b/packages/core/test/animation/animation_integration_spec.ts @@ -366,6 +366,51 @@ export function main() { assertHasParent(element, false); }); + it('should properly cancel all existing animations when a removal occurs', () => { + @Component({ + selector: 'ani-cmp', + template: ` +
+ `, + animations: [ + trigger( + 'myAnimation', + [ + transition( + '* => go', [style({width: '0px'}), animate(1000, style({width: '100px'}))]), + ]), + ] + }) + class Cmp { + public exp: string; + } + + TestBed.configureTestingModule({declarations: [Cmp]}); + + const engine = TestBed.get(ɵAnimationEngine); + const fixture = TestBed.createComponent(Cmp); + const cmp = fixture.componentInstance; + cmp.exp = 'go'; + fixture.detectChanges(); + engine.flush(); + expect(getLog().length).toEqual(1); + const [player1] = getLog(); + resetLog(); + + let finished = false; + player1.onDone(() => finished = true); + + let destroyed = false; + player1.onDestroy(() => destroyed = true); + + cmp.exp = null; + fixture.detectChanges(); + engine.flush(); + + expect(finished).toBeTruthy(); + expect(destroyed).toBeTruthy(); + }); + it('should not run inner child animations when a parent is set to be removed', () => { @Component({ selector: 'ani-cmp',