diff --git a/modules/@angular/platform-browser/animations/src/render/dom_animation_engine.ts b/modules/@angular/platform-browser/animations/src/render/dom_animation_engine.ts index ced0ccc77f..9b15ab9aa7 100644 --- a/modules/@angular/platform-browser/animations/src/render/dom_animation_engine.ts +++ b/modules/@angular/platform-browser/animations/src/render/dom_animation_engine.ts @@ -69,8 +69,21 @@ export class DomAnimationEngine { } onRemove(element: any, domFn: () => any): void { - element[MARKED_FOR_REMOVAL] = true; - this._queuedRemovals.set(element, domFn); + let lookupRef = this._elementTriggerStates.get(element); + if (lookupRef) { + const possibleTriggers = Object.keys(lookupRef); + const hasRemoval = possibleTriggers.some(triggerName => { + const oldValue = lookupRef[triggerName]; + const instruction = this._triggers[triggerName].matchTransition(oldValue, 'void'); + return !!instruction; + }); + if (hasRemoval) { + element[MARKED_FOR_REMOVAL] = true; + this._queuedRemovals.set(element, domFn); + return; + } + } + domFn(); } setProperty(element: any, property: string, value: any): void { diff --git a/modules/@angular/platform-browser/animations/src/render/web_animations/web_animations_player.ts b/modules/@angular/platform-browser/animations/src/render/web_animations/web_animations_player.ts index a20febf3cc..940c329ef7 100644 --- a/modules/@angular/platform-browser/animations/src/render/web_animations/web_animations_player.ts +++ b/modules/@angular/platform-browser/animations/src/render/web_animations/web_animations_player.ts @@ -90,7 +90,8 @@ export class WebAnimationsPlayer implements AnimationPlayer { } this._player = this._triggerWebAnimation(this.element, keyframes, this.options); - this._finalKeyframe = _copyKeyframeStyles(keyframes[keyframes.length - 1]); + this._finalKeyframe = + keyframes.length ? _copyKeyframeStyles(keyframes[keyframes.length - 1]) : {}; // this is required so that the player doesn't start to animate right away this._resetDomPlayerState(); diff --git a/modules/@angular/platform-browser/animations/test/engine/animation_engine_spec.ts b/modules/@angular/platform-browser/animations/test/engine/dom_animation_engine_spec.ts similarity index 92% rename from modules/@angular/platform-browser/animations/test/engine/animation_engine_spec.ts rename to modules/@angular/platform-browser/animations/test/engine/dom_animation_engine_spec.ts index d73efceeee..4f3e012c1f 100644 --- a/modules/@angular/platform-browser/animations/test/engine/animation_engine_spec.ts +++ b/modules/@angular/platform-browser/animations/test/engine/dom_animation_engine_spec.ts @@ -626,56 +626,6 @@ export function main() { expect(container.contains(child1)).toBe(true); expect(container.contains(child2)).toBe(true); }); - - it('should queue up all `remove` DOM operations until all animations are complete', () => { - let container = el('
'); - let targetContainer = el('
'); - let otherContainer = el('
'); - let child1 = el('
'); - let child2 = el('
'); - container.appendChild(targetContainer); - container.appendChild(otherContainer); - targetContainer.appendChild(child1); - targetContainer.appendChild(child2); - - /*----------------* - container - / \ - target other - / \ - c1 c2 - *----------------*/ - - expect(container.contains(otherContainer)).toBe(true); - - const engine = makeEngine(); - engine.onRemove(child1, () => targetContainer.removeChild(child1)); - engine.onRemove(child2, () => targetContainer.removeChild(child2)); - engine.onRemove(otherContainer, () => container.removeChild(otherContainer)); - - expect(container.contains(child1)).toBe(true); - expect(container.contains(child2)).toBe(true); - expect(container.contains(otherContainer)).toBe(true); - - const instructions = - buildAnimationKeyframes([style({height: 0}), animate(1000, style({height: 100}))]); - - const player = engine.animateTimeline(targetContainer, instructions); - - expect(container.contains(child1)).toBe(true); - expect(container.contains(child2)).toBe(true); - expect(container.contains(otherContainer)).toBe(true); - - engine.flush(); - expect(container.contains(child1)).toBe(true); - expect(container.contains(child2)).toBe(true); - expect(container.contains(otherContainer)).toBe(false); - - player.finish(); - expect(container.contains(child1)).toBe(false); - expect(container.contains(child2)).toBe(false); - expect(container.contains(otherContainer)).toBe(false); - }); }); }); } diff --git a/modules/@angular/platform-browser/test/animation/animation_renderer_spec.ts b/modules/@angular/platform-browser/test/animation/animation_renderer_spec.ts index 77cfccc6ee..c5d3a1e1aa 100644 --- a/modules/@angular/platform-browser/test/animation/animation_renderer_spec.ts +++ b/modules/@angular/platform-browser/test/animation/animation_renderer_spec.ts @@ -7,7 +7,7 @@ */ import {AnimationTriggerMetadata, animate, state, style, transition, trigger} from '@angular/animations'; import {USE_VIEW_ENGINE} from '@angular/compiler/src/config'; -import {Component, Injectable, RendererFactoryV2, RendererTypeV2, ViewChild} from '@angular/core'; +import {AnimationPlayer, Component, Injectable, RendererFactoryV2, RendererTypeV2, ViewChild} from '@angular/core'; import {TestBed} from '@angular/core/testing'; import {BrowserAnimationModule, ɵAnimationEngine, ɵAnimationRendererFactory} from '@angular/platform-browser/animations'; @@ -200,6 +200,76 @@ export function main() { }); }); }); + + it('should only queue up dom removals if the element itself contains a valid leave animation', + () => { + @Component({ + selector: 'my-cmp', + template: ` +
+
+
+ `, + animations: [ + trigger('animation1', [transition('a => b', [])]), + trigger('animation2', [transition(':leave', [])]), + ] + }) + class Cmp { + exp1: any = true; + exp2: any = true; + exp3: any = true; + + @ViewChild('elm1') public elm1: any; + + @ViewChild('elm2') public elm2: any; + + @ViewChild('elm3') public elm3: any; + } + + TestBed.configureTestingModule({ + providers: [{provide: ɵAnimationEngine, useClass: InjectableAnimationEngine}], + declarations: [Cmp] + }); + + const engine = TestBed.get(ɵAnimationEngine); + const fixture = TestBed.createComponent(Cmp); + const cmp = fixture.componentInstance; + + fixture.detectChanges(); + const elm1 = cmp.elm1; + const elm2 = cmp.elm2; + const elm3 = cmp.elm3; + assertHasParent(elm1); + assertHasParent(elm2); + assertHasParent(elm3); + engine.flush(); + finishPlayers(engine.activePlayers); + + cmp.exp1 = false; + fixture.detectChanges(); + assertHasParent(elm1, false); + assertHasParent(elm2); + assertHasParent(elm3); + engine.flush(); + expect(engine.activePlayers.length).toEqual(0); + + cmp.exp2 = false; + fixture.detectChanges(); + assertHasParent(elm1, false); + assertHasParent(elm2, false); + assertHasParent(elm3); + engine.flush(); + expect(engine.activePlayers.length).toEqual(0); + + cmp.exp3 = false; + fixture.detectChanges(); + assertHasParent(elm1, false); + assertHasParent(elm2, false); + assertHasParent(elm3); + engine.flush(); + expect(engine.activePlayers.length).toEqual(1); + }); }); }); } @@ -233,3 +303,17 @@ class MockAnimationEngine extends ɵAnimationEngine { flush() {} } + + +function assertHasParent(element: any, yes: boolean = true) { + const parent = element.nativeElement.parentNode; + if (yes) { + expect(parent).toBeTruthy(); + } else { + expect(parent).toBeFalsy(); + } +} + +function finishPlayers(players: AnimationPlayer[]) { + players.forEach(player => player.finish()); +}