fix(animations): always fire callbacks even for noop animations (#15170)

Closes #15170
This commit is contained in:
Matias Niemelä 2017-03-15 13:41:00 -07:00 committed by Chuck Jazdzewski
parent 80112a9ea1
commit 3f38c6fdcd
4 changed files with 182 additions and 17 deletions

View File

@ -43,6 +43,8 @@ export class DomAnimationEngine {
private _triggers: {[triggerName: string]: AnimationTrigger} = Object.create(null);
private _triggerListeners = new Map<any, TriggerListenerTuple[]>();
private _pendingListenerRemovals = new Map<any, TriggerListenerTuple[]>();
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 = <TriggerListenerTuple>{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<any, TriggerListenerTuple[]>();
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();
}
}
}

View File

@ -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 = [];

View File

@ -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']);
});
});
}

View File

@ -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: `
<div [@myAnimation]="exp" (@myAnimation.start)="callback($event)" (@myAnimation.done)="callback($event)"></div>
`,
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: `
<div *ngIf="exp" @myAnimation (@myAnimation.start)="callback($event)" (@myAnimation.done)="callback($event)"></div>
`,
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', () => {