diff --git a/packages/animations/browser/src/render/dom_animation_engine.ts b/packages/animations/browser/src/render/dom_animation_engine.ts index 7b9372e69f..c9c752a86b 100644 --- a/packages/animations/browser/src/render/dom_animation_engine.ts +++ b/packages/animations/browser/src/render/dom_animation_engine.ts @@ -43,6 +43,8 @@ export class DomAnimationEngine { private _triggers: {[triggerName: string]: AnimationTrigger} = Object.create(null); private _triggerListeners = new Map(); + private _pendingListenerRemovals = new Map(); + constructor(private _driver: AnimationDriver, private _normalizer: AnimationStyleNormalizer) {} get queuedPlayers(): AnimationPlayer[] { @@ -83,6 +85,13 @@ export class DomAnimationEngine { return; } } + + // this means that there are no animations to take on this + // leave operation therefore we should fire the done|start callbacks + if (this._triggerListeners.has(element)) { + element[MARKED_FOR_REMOVAL] = true; + this._queuedRemovals.set(element, () => {}); + } domFn(); } @@ -128,13 +137,27 @@ export class DomAnimationEngine { const tuple = {triggerName: eventName, phase: eventPhase, callback}; elementListeners.push(tuple); return () => { - const index = elementListeners.indexOf(tuple); - if (index >= 0) { - elementListeners.splice(index, 1); - } + // this is queued up in the event that a removal animation is set + // to fire on the element (the listeners need to be set during flush) + getOrSetAsInMap(this._pendingListenerRemovals, element, []).push(tuple); }; } + private _clearPendingListenerRemovals() { + this._pendingListenerRemovals.forEach((tuples: TriggerListenerTuple[], element: any) => { + const elementListeners = this._triggerListeners.get(element); + if (elementListeners) { + tuples.forEach(tuple => { + const index = elementListeners.indexOf(tuple); + if (index >= 0) { + elementListeners.splice(index, 1); + } + }); + } + }); + this._pendingListenerRemovals.clear(); + } + private _onRemovalTransition(element: any): AnimationPlayer[] { // when a parent animation is set to trigger a removal we want to // find all of the children that are currently animating and clear @@ -293,13 +316,6 @@ export class DomAnimationEngine { if (parent[MARKED_FOR_REMOVAL]) continue parentLoop; } - // if a removal exists for the given element then we need cancel - // all the queued players so that a proper removal animation can go - if (this._queuedRemovals.has(element)) { - player.destroy(); - continue; - } - const listeners = this._triggerListeners.get(element); if (listeners) { listeners.forEach(tuple => { @@ -309,6 +325,13 @@ export class DomAnimationEngine { }); } + // if a removal exists for the given element then we need cancel + // all the queued players so that a proper removal animation can go + if (this._queuedRemovals.has(element)) { + player.destroy(); + continue; + } + this._markPlayerAsActive(element, player); // in the event that an animation throws an error then we do @@ -321,6 +344,18 @@ export class DomAnimationEngine { } flush() { + const leaveListeners = new Map(); + this._queuedRemovals.forEach((callback, element) => { + const tuple = this._pendingListenerRemovals.get(element); + if (tuple) { + leaveListeners.set(element, tuple); + this._pendingListenerRemovals.delete(element); + } + }); + + this._clearPendingListenerRemovals(); + this._pendingListenerRemovals = leaveListeners; + this._flushQueuedAnimations(); let flushAgain = false; @@ -355,11 +390,15 @@ export class DomAnimationEngine { const stateDetails = this._elementTriggerStates.get(element); if (stateDetails) { Object.keys(stateDetails).forEach(triggerName => { + flushAgain = true; const oldValue = stateDetails[triggerName]; const instruction = this._triggers[triggerName].matchTransition(oldValue, 'void'); if (instruction) { players.push(this.animateTransition(element, instruction)); - flushAgain = true; + } else { + const event = makeAnimationEvent(element, triggerName, oldValue, 'void', '', 0); + const player = new NoopAnimationPlayer(); + this._queuePlayer(element, triggerName, player, event); } }); } @@ -378,6 +417,7 @@ export class DomAnimationEngine { // this means that one or more leave animations were detected if (flushAgain) { this._flushQueuedAnimations(); + this._clearPendingListenerRemovals(); } } } diff --git a/packages/animations/src/players/animation_player.ts b/packages/animations/src/players/animation_player.ts index 077b389795..e67384229f 100644 --- a/packages/animations/src/players/animation_player.ts +++ b/packages/animations/src/players/animation_player.ts @@ -39,7 +39,7 @@ export class NoopAnimationPlayer implements AnimationPlayer { private _destroyed = false; private _finished = false; public parentPlayer: AnimationPlayer = null; - constructor() { scheduleMicroTask(() => this._onFinish()); } + constructor() {} private _onFinish() { if (!this._finished) { this._finished = true; @@ -54,17 +54,24 @@ export class NoopAnimationPlayer implements AnimationPlayer { init(): void {} play(): void { if (!this.hasStarted()) { - this._onStartFns.forEach(fn => fn()); - this._onStartFns = []; + scheduleMicroTask(() => this._onFinish()); + this._onStart(); } this._started = true; } + private _onStart() { + this._onStartFns.forEach(fn => fn()); + this._onStartFns = []; + } pause(): void {} restart(): void {} finish(): void { this._onFinish(); } destroy(): void { if (!this._destroyed) { this._destroyed = true; + if (!this.hasStarted()) { + this._onStart(); + } this.finish(); this._onDestroyFns.forEach(fn => fn()); this._onDestroyFns = []; diff --git a/packages/animations/test/animation_player_spec.ts b/packages/animations/test/animation_player_spec.ts new file mode 100644 index 0000000000..bf86c8f8aa --- /dev/null +++ b/packages/animations/test/animation_player_spec.ts @@ -0,0 +1,44 @@ +/** + * @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 {fakeAsync} from '@angular/core/testing'; + +import {flushMicrotasks} from '../../core/testing/src/fake_async'; +import {NoopAnimationPlayer} from '../src/players/animation_player'; + +export function main() { + describe('NoopAnimationPlayer', function() { + it('should finish after the next microtask once started', fakeAsync(() => { + const log: string[] = []; + + const player = new NoopAnimationPlayer(); + player.onStart(() => log.push('started')); + player.onDone(() => log.push('done')); + flushMicrotasks(); + + expect(log).toEqual([]); + player.play(); + expect(log).toEqual(['started']); + + flushMicrotasks(); + expect(log).toEqual(['started', 'done']); + })); + + it('should fire all callbacks when destroyed', () => { + const log: string[] = []; + + const player = new NoopAnimationPlayer(); + player.onStart(() => log.push('started')); + player.onDone(() => log.push('done')); + player.onDestroy(() => log.push('destroy')); + expect(log).toEqual([]); + + player.destroy(); + expect(log).toEqual(['started', 'done', 'destroy']); + }); + }); +} diff --git a/packages/core/test/animation/animation_integration_spec.ts b/packages/core/test/animation/animation_integration_spec.ts index 00c5af35d8..992385c70b 100644 --- a/packages/core/test/animation/animation_integration_spec.ts +++ b/packages/core/test/animation/animation_integration_spec.ts @@ -6,14 +6,14 @@ * found in the LICENSE file at https://angular.io/license */ import {AUTO_STYLE, AnimationEvent, animate, keyframes, state, style, transition, trigger} from '@angular/animations'; -import {AnimationDriver, ɵAnimationEngine} from '@angular/animations/browser'; +import {AnimationDriver, ɵAnimationEngine, ɵNoopAnimationDriver} from '@angular/animations/browser'; import {MockAnimationDriver, MockAnimationPlayer} from '@angular/animations/browser/testing'; import {Component, HostBinding, HostListener, RendererFactory2, ViewChild} from '@angular/core'; import {ɵDomRendererFactory2} from '@angular/platform-browser'; import {BrowserAnimationsModule} from '@angular/platform-browser/animations'; import {getDOM} from '@angular/platform-browser/src/dom/dom_adapter'; -import {TestBed} from '../../testing'; +import {TestBed, fakeAsync, flushMicrotasks} from '../../testing'; export function main() { // these tests are only mean't to be run within the DOM (for now) @@ -551,6 +551,80 @@ export function main() { expect(cmp.event.fromState).toEqual('void'); expect(cmp.event.toState).toEqual('TRUE'); }); + + it('should always fire callbacks even when a transition is not detected', fakeAsync(() => { + @Component({ + selector: 'my-cmp', + template: ` +
+ `, + animations: [trigger('myAnimation', [])] + }) + class Cmp { + exp: string; + log: any[] = []; + callback = (event: any) => { this.log.push(`${event.phaseName} => ${event.toState}`); } + } + + TestBed.configureTestingModule({ + providers: [{provide: AnimationDriver, useClass: ɵNoopAnimationDriver}], + declarations: [Cmp] + }); + + const fixture = TestBed.createComponent(Cmp); + const cmp = fixture.componentInstance; + + cmp.exp = 'a'; + fixture.detectChanges(); + flushMicrotasks(); + expect(cmp.log).toEqual(['start => a', 'done => a']); + + cmp.log = []; + cmp.exp = 'b'; + fixture.detectChanges(); + flushMicrotasks(); + + expect(cmp.log).toEqual(['start => b', 'done => b']); + })); + + it('should fire callback events for leave animations', fakeAsync(() => { + @Component({ + selector: 'my-cmp', + template: ` +
+ `, + animations: [trigger('myAnimation', [])] + }) + class Cmp { + exp: boolean = false; + log: any[] = []; + callback = (event: any) => { + const state = event.toState || '_default_'; + this.log.push(`${event.phaseName} => ${state}`); + } + } + + TestBed.configureTestingModule({ + providers: [{provide: AnimationDriver, useClass: ɵNoopAnimationDriver}], + declarations: [Cmp] + }); + + const fixture = TestBed.createComponent(Cmp); + const cmp = fixture.componentInstance; + + cmp.exp = true; + fixture.detectChanges(); + flushMicrotasks(); + expect(cmp.log).toEqual(['start => _default_', 'done => _default_']); + + cmp.log = []; + + cmp.exp = false; + fixture.detectChanges(); + flushMicrotasks(); + + expect(cmp.log).toEqual(['start => void', 'done => void']); + })); }); describe('errors for not using the animation module', () => {