fix(animations): report correct totalTime value even during noOp animations (#22225)

This patch ensures that if the NoopAnimationsModule is used then it will
correctly report the associated `totalTime` property within the emitted
AnimationEvent instance when an animation event trigger is fired.

BREAKING CHANGE: When animation is trigged within a disabled zone, the
associated event (which an instance of AnimationEvent) will no longer
report the totalTime as 0 (it will emit the actual time of the
animation). To detect if an animation event is reporting a disabled
animation then the `event.disabled` property can be used instead.

PR Close #22225
This commit is contained in:
Matias Niemelä 2018-02-14 10:12:10 -08:00 committed by Victor Berchet
parent 884de18cba
commit e1bf067090
12 changed files with 108 additions and 37 deletions

View File

@ -59,10 +59,13 @@ export class AnimationTransitionFactory {
driver, element, this.ast.animation, enterClassName, leaveClassName, currentStateStyles, driver, element, this.ast.animation, enterClassName, leaveClassName, currentStateStyles,
nextStateStyles, animationOptions, subInstructions, errors); nextStateStyles, animationOptions, subInstructions, errors);
let totalTime = 0;
timelines.forEach(tl => { totalTime = Math.max(tl.duration + tl.delay, totalTime); });
if (errors.length) { if (errors.length) {
return createTransitionInstruction( return createTransitionInstruction(
element, this._triggerName, currentState, nextState, isRemoval, currentStateStyles, element, this._triggerName, currentState, nextState, isRemoval, currentStateStyles,
nextStateStyles, [], [], preStyleMap, postStyleMap, errors); nextStateStyles, [], [], preStyleMap, postStyleMap, totalTime, errors);
} }
timelines.forEach(tl => { timelines.forEach(tl => {
@ -81,7 +84,7 @@ export class AnimationTransitionFactory {
const queriedElementsList = iteratorToArray(queriedElements.values()); const queriedElementsList = iteratorToArray(queriedElements.values());
return createTransitionInstruction( return createTransitionInstruction(
element, this._triggerName, currentState, nextState, isRemoval, currentStateStyles, element, this._triggerName, currentState, nextState, isRemoval, currentStateStyles,
nextStateStyles, timelines, queriedElementsList, preStyleMap, postStyleMap); nextStateStyles, timelines, queriedElementsList, preStyleMap, postStyleMap, totalTime);
} }
} }

View File

@ -21,6 +21,7 @@ export interface AnimationTransitionInstruction extends AnimationEngineInstructi
queriedElements: any[]; queriedElements: any[];
preStyleProps: Map<any, {[prop: string]: boolean}>; preStyleProps: Map<any, {[prop: string]: boolean}>;
postStyleProps: Map<any, {[prop: string]: boolean}>; postStyleProps: Map<any, {[prop: string]: boolean}>;
totalTime: number;
errors?: any[]; errors?: any[];
} }
@ -29,7 +30,7 @@ export function createTransitionInstruction(
isRemovalTransition: boolean, fromStyles: ɵStyleData, toStyles: ɵStyleData, isRemovalTransition: boolean, fromStyles: ɵStyleData, toStyles: ɵStyleData,
timelines: AnimationTimelineInstruction[], queriedElements: any[], timelines: AnimationTimelineInstruction[], queriedElements: any[],
preStyleProps: Map<any, {[prop: string]: boolean}>, preStyleProps: Map<any, {[prop: string]: boolean}>,
postStyleProps: Map<any, {[prop: string]: boolean}>, postStyleProps: Map<any, {[prop: string]: boolean}>, totalTime: number,
errors?: any[]): AnimationTransitionInstruction { errors?: any[]): AnimationTransitionInstruction {
return { return {
type: AnimationTransitionInstructionType.TransitionAnimation, type: AnimationTransitionInstructionType.TransitionAnimation,
@ -44,6 +45,7 @@ export function createTransitionInstruction(
queriedElements, queriedElements,
preStyleProps, preStyleProps,
postStyleProps, postStyleProps,
totalTime,
errors errors
}; };
} }

View File

@ -34,7 +34,7 @@ export class NoopAnimationDriver implements AnimationDriver {
animate( animate(
element: any, keyframes: {[key: string]: string | number}[], duration: number, delay: number, element: any, keyframes: {[key: string]: string | number}[], duration: number, delay: number,
easing: string, previousPlayers: any[] = []): AnimationPlayer { easing: string, previousPlayers: any[] = []): AnimationPlayer {
return new NoopAnimationPlayer(); return new NoopAnimationPlayer(duration, delay);
} }
} }

View File

@ -75,23 +75,24 @@ export function listenOnPlayer(
callback: (event: any) => any) { callback: (event: any) => any) {
switch (eventName) { switch (eventName) {
case 'start': case 'start':
player.onStart(() => callback(event && copyAnimationEvent(event, 'start', player.totalTime))); player.onStart(() => callback(event && copyAnimationEvent(event, 'start', player)));
break; break;
case 'done': case 'done':
player.onDone(() => callback(event && copyAnimationEvent(event, 'done', player.totalTime))); player.onDone(() => callback(event && copyAnimationEvent(event, 'done', player)));
break; break;
case 'destroy': case 'destroy':
player.onDestroy( player.onDestroy(() => callback(event && copyAnimationEvent(event, 'destroy', player)));
() => callback(event && copyAnimationEvent(event, 'destroy', player.totalTime)));
break; break;
} }
} }
export function copyAnimationEvent( export function copyAnimationEvent(
e: AnimationEvent, phaseName?: string, totalTime?: number): AnimationEvent { e: AnimationEvent, phaseName: string, player: AnimationPlayer): AnimationEvent {
const totalTime = player.totalTime;
const disabled = (player as any).disabled ? true : false;
const event = makeAnimationEvent( const event = makeAnimationEvent(
e.element, e.triggerName, e.fromState, e.toState, phaseName || e.phaseName, e.element, e.triggerName, e.fromState, e.toState, phaseName || e.phaseName,
totalTime == undefined ? e.totalTime : totalTime); totalTime == undefined ? e.totalTime : totalTime, disabled);
const data = (e as any)['_data']; const data = (e as any)['_data'];
if (data != null) { if (data != null) {
(event as any)['_data'] = data; (event as any)['_data'] = data;
@ -101,8 +102,8 @@ export function copyAnimationEvent(
export function makeAnimationEvent( export function makeAnimationEvent(
element: any, triggerName: string, fromState: string, toState: string, phaseName: string = '', element: any, triggerName: string, fromState: string, toState: string, phaseName: string = '',
totalTime: number = 0): AnimationEvent { totalTime: number = 0, disabled?: boolean): AnimationEvent {
return {element, triggerName, fromState, toState, phaseName, totalTime}; return {element, triggerName, fromState, toState, phaseName, totalTime, disabled: !!disabled};
} }
export function getOrSetAsInMap( export function getOrSetAsInMap(

View File

@ -1081,6 +1081,8 @@ export class TransitionAnimationEngine {
if (subTimelines.has(element)) { if (subTimelines.has(element)) {
if (disabledElementsSet.has(element)) { if (disabledElementsSet.has(element)) {
player.onDestroy(() => setStyles(element, instruction.toStyles)); player.onDestroy(() => setStyles(element, instruction.toStyles));
player.disabled = true;
player.overrideTotalTime(instruction.totalTime);
skippedPlayers.push(player); skippedPlayers.push(player);
return; return;
} }
@ -1311,7 +1313,8 @@ export class TransitionAnimationEngine {
// FIXME (matsko): make sure to-be-removed animations are removed properly // FIXME (matsko): make sure to-be-removed animations are removed properly
const details = element[REMOVAL_FLAG]; const details = element[REMOVAL_FLAG];
if (details && details.removedBeforeQueried) return new NoopAnimationPlayer(); if (details && details.removedBeforeQueried)
return new NoopAnimationPlayer(timelineInstruction.duration, timelineInstruction.delay);
const isQueriedElement = element !== rootElement; const isQueriedElement = element !== rootElement;
const previousPlayers = const previousPlayers =
@ -1379,7 +1382,7 @@ export class TransitionAnimationEngine {
// special case for when an empty transition|definition is provided // special case for when an empty transition|definition is provided
// ... there is no point in rendering an empty animation // ... there is no point in rendering an empty animation
return new NoopAnimationPlayer(); return new NoopAnimationPlayer(instruction.duration, instruction.delay);
} }
} }
@ -1392,8 +1395,10 @@ export class TransitionAnimationPlayer implements AnimationPlayer {
public parentPlayer: AnimationPlayer; public parentPlayer: AnimationPlayer;
public markedForDestroy: boolean = false; public markedForDestroy: boolean = false;
public disabled = false;
readonly queued: boolean = true; readonly queued: boolean = true;
public readonly totalTime: number = 0;
constructor(public namespaceId: string, public triggerName: string, public element: any) {} constructor(public namespaceId: string, public triggerName: string, public element: any) {}
@ -1407,15 +1412,18 @@ export class TransitionAnimationPlayer implements AnimationPlayer {
}); });
this._queuedCallbacks = {}; this._queuedCallbacks = {};
this._containsRealPlayer = true; this._containsRealPlayer = true;
this.overrideTotalTime(player.totalTime);
(this as{queued: boolean}).queued = false; (this as{queued: boolean}).queued = false;
} }
getRealPlayer() { return this._player; } getRealPlayer() { return this._player; }
overrideTotalTime(totalTime: number) { (this as any).totalTime = totalTime; }
syncPlayerEvents(player: AnimationPlayer) { syncPlayerEvents(player: AnimationPlayer) {
const p = this._player as any; const p = this._player as any;
if (p.triggerCallback) { if (p.triggerCallback) {
player.onStart(() => p.triggerCallback('start')); player.onStart(() => p.triggerCallback !('start'));
} }
player.onDone(() => this.finish()); player.onDone(() => this.finish());
player.onDestroy(() => this.destroy()); player.onDestroy(() => this.destroy());
@ -1473,8 +1481,6 @@ export class TransitionAnimationPlayer implements AnimationPlayer {
getPosition(): number { return this.queued ? 0 : this._player.getPosition(); } getPosition(): number { return this.queued ? 0 : this._player.getPosition(); }
get totalTime(): number { return this._player.totalTime; }
/* @internal */ /* @internal */
triggerCallback(phaseName: string): void { triggerCallback(phaseName: string): void {
const p = this._player as any; const p = this._player as any;

View File

@ -299,7 +299,8 @@ const DEFAULT_NAMESPACE_ID = 'id';
phaseName: 'start', phaseName: 'start',
fromState: '123', fromState: '123',
toState: '456', toState: '456',
totalTime: 1234 totalTime: 1234,
disabled: false
}); });
capture = null !; capture = null !;
@ -313,7 +314,8 @@ const DEFAULT_NAMESPACE_ID = 'id';
phaseName: 'done', phaseName: 'done',
fromState: '123', fromState: '123',
toState: '456', toState: '456',
totalTime: 1234 totalTime: 1234,
disabled: false
}); });
}); });
}); });

View File

@ -58,7 +58,7 @@ export class MockAnimationPlayer extends NoopAnimationPlayer {
public element: any, public keyframes: {[key: string]: string | number}[], public element: any, public keyframes: {[key: string]: string | number}[],
public duration: number, public delay: number, public easing: string, public duration: number, public delay: number, public easing: string,
public previousPlayers: any[]) { public previousPlayers: any[]) {
super(); super(duration, delay);
if (allowPreviousPlayerStylesMerge(duration, delay)) { if (allowPreviousPlayerStylesMerge(duration, delay)) {
previousPlayers.forEach(player => { previousPlayers.forEach(player => {
@ -68,8 +68,6 @@ export class MockAnimationPlayer extends NoopAnimationPlayer {
} }
}); });
} }
this.totalTime = delay + duration;
} }
/* @internal */ /* @internal */

View File

@ -44,4 +44,5 @@ export interface AnimationEvent {
phaseName: string; phaseName: string;
element: any; element: any;
triggerName: string; triggerName: string;
disabled: boolean;
} }

View File

@ -352,6 +352,14 @@ export interface AnimationStaggerMetadata extends AnimationMetadata {
elements located in disabled areas of the template and still animate them as it sees fit. This is elements located in disabled areas of the template and still animate them as it sees fit. This is
also the case for when a sub animation is queried by a parent and then later animated using {@link also the case for when a sub animation is queried by a parent and then later animated using {@link
animateChild animateChild}. animateChild animateChild}.
* ### Detecting when an animation is disabled
* If a region of the DOM (or the entire application) has its animations disabled, then animation
* trigger callbacks will still fire just as normal (only for zero seconds).
*
* When a trigger callback fires it will provide an instance of an {@link AnimationEvent}. If
animations
* are disabled then the `.disabled` flag on the event will be true.
* *
* @experimental Animation support is experimental. * @experimental Animation support is experimental.
*/ */

View File

@ -33,6 +33,8 @@ export interface AnimationPlayer {
beforeDestroy?: () => any; beforeDestroy?: () => any;
/* @internal */ /* @internal */
triggerCallback?: (phaseName: string) => void; triggerCallback?: (phaseName: string) => void;
/* @internal */
disabled?: boolean;
} }
/** /**
@ -46,8 +48,8 @@ export class NoopAnimationPlayer implements AnimationPlayer {
private _destroyed = false; private _destroyed = false;
private _finished = false; private _finished = false;
public parentPlayer: AnimationPlayer|null = null; public parentPlayer: AnimationPlayer|null = null;
public totalTime = 0; public readonly totalTime: number;
constructor() {} constructor(duration: number = 0, delay: number = 0) { this.totalTime = duration + delay; }
private _onFinish() { private _onFinish() {
if (!this._finished) { if (!this._finished) {
this._finished = true; this._finished = true;

View File

@ -5,8 +5,8 @@
* Use of this source code is governed by an MIT-style license that can be * 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 * found in the LICENSE file at https://angular.io/license
*/ */
import {AUTO_STYLE, AnimationEvent, AnimationOptions, animate, animateChild, group, keyframes, query, state, style, transition, trigger, ɵPRE_STYLE as PRE_STYLE} from '@angular/animations'; import {AUTO_STYLE, AnimationEvent, AnimationOptions, AnimationPlayer, NoopAnimationPlayer, animate, animateChild, group, keyframes, query, state, style, transition, trigger, ɵPRE_STYLE as PRE_STYLE} from '@angular/animations';
import {AnimationDriver, ɵAnimationEngine, ɵNoopAnimationDriver} from '@angular/animations/browser'; import {AnimationDriver, ɵAnimationEngine, ɵNoopAnimationDriver as NoopAnimationDriver} from '@angular/animations/browser';
import {MockAnimationDriver, MockAnimationPlayer} from '@angular/animations/browser/testing'; import {MockAnimationDriver, MockAnimationPlayer} from '@angular/animations/browser/testing';
import {ChangeDetectorRef, Component, HostBinding, HostListener, RendererFactory2, ViewChild} from '@angular/core'; import {ChangeDetectorRef, Component, HostBinding, HostListener, RendererFactory2, ViewChild} from '@angular/core';
import {ɵDomRendererFactory2} from '@angular/platform-browser'; import {ɵDomRendererFactory2} from '@angular/platform-browser';
@ -112,6 +112,50 @@ const DEFAULT_COMPONENT_ID = '1';
flushMicrotasks(); flushMicrotasks();
expect(cmp.log).toEqual(['start', 'done']); expect(cmp.log).toEqual(['start', 'done']);
})); }));
it('should emit the correct totalTime value for a noop-animation', fakeAsync(() => {
@Component({
selector: 'cmp',
template: `
<div [@myAnimation]="exp" (@myAnimation.start)="cb($event)" (@myAnimation.done)="cb($event)"></div>
`,
animations: [
trigger(
'myAnimation',
[
transition(
'* => go',
[
animate('1s', style({opacity: 0})),
]),
]),
]
})
class Cmp {
exp: any = false;
log: AnimationEvent[] = [];
cb(event: AnimationEvent) { this.log.push(event); }
}
TestBed.configureTestingModule({
declarations: [Cmp],
providers: [
{provide: AnimationDriver, useClass: NoopAnimationDriver},
],
});
const fixture = TestBed.createComponent(Cmp);
const cmp = fixture.componentInstance;
cmp.exp = 'go';
fixture.detectChanges();
expect(cmp.log).toEqual([]);
flushMicrotasks();
expect(cmp.log.length).toEqual(2);
const [start, end] = cmp.log;
expect(start.totalTime).toEqual(1000);
expect(end.totalTime).toEqual(1000);
}));
}); });
describe('component fixture integration', () => { describe('component fixture integration', () => {
@ -166,7 +210,7 @@ const DEFAULT_COMPONENT_ID = '1';
} }
TestBed.configureTestingModule({ TestBed.configureTestingModule({
providers: [{provide: AnimationDriver, useClass: ɵNoopAnimationDriver}], providers: [{provide: AnimationDriver, useClass: NoopAnimationDriver}],
declarations: [Cmp] declarations: [Cmp]
}); });
@ -2461,7 +2505,7 @@ const DEFAULT_COMPONENT_ID = '1';
} }
TestBed.configureTestingModule({ TestBed.configureTestingModule({
providers: [{provide: AnimationDriver, useClass: ɵNoopAnimationDriver}], providers: [{provide: AnimationDriver, useClass: NoopAnimationDriver}],
declarations: [Cmp] declarations: [Cmp]
}); });
@ -2500,7 +2544,7 @@ const DEFAULT_COMPONENT_ID = '1';
} }
TestBed.configureTestingModule({ TestBed.configureTestingModule({
providers: [{provide: AnimationDriver, useClass: ɵNoopAnimationDriver}], providers: [{provide: AnimationDriver, useClass: NoopAnimationDriver}],
declarations: [Cmp] declarations: [Cmp]
}); });
@ -2971,8 +3015,8 @@ const DEFAULT_COMPONENT_ID = '1';
class Cmp { class Cmp {
disableExp = false; disableExp = false;
exp = ''; exp = '';
startEvent: any; startEvent: AnimationEvent;
doneEvent: any; doneEvent: AnimationEvent;
} }
TestBed.configureTestingModule({declarations: [Cmp]}); TestBed.configureTestingModule({declarations: [Cmp]});
@ -2988,14 +3032,17 @@ const DEFAULT_COMPONENT_ID = '1';
cmp.exp = '1'; cmp.exp = '1';
fixture.detectChanges(); fixture.detectChanges();
flushMicrotasks(); flushMicrotasks();
expect(cmp.startEvent.totalTime).toEqual(0); expect(cmp.startEvent.totalTime).toEqual(9876);
expect(cmp.doneEvent.totalTime).toEqual(0); expect(cmp.startEvent.disabled).toBeTruthy();
expect(cmp.doneEvent.totalTime).toEqual(9876);
expect(cmp.doneEvent.disabled).toBeTruthy();
cmp.exp = '2'; cmp.exp = '2';
cmp.disableExp = false; cmp.disableExp = false;
fixture.detectChanges(); fixture.detectChanges();
flushMicrotasks(); flushMicrotasks();
expect(cmp.startEvent.totalTime).toEqual(9876); expect(cmp.startEvent.totalTime).toEqual(9876);
expect(cmp.startEvent.disabled).toBeFalsy();
// the done event isn't fired because it's an actual animation // the done event isn't fired because it's an actual animation
})); }));
@ -3428,7 +3475,7 @@ const DEFAULT_COMPONENT_ID = '1';
}); });
}); });
}); });
}); })();
function assertHasParent(element: any, yes: boolean) { function assertHasParent(element: any, yes: boolean) {
const parent = getDOM().parentElement(element); const parent = getDOM().parentElement(element);

View File

@ -43,6 +43,7 @@ export declare abstract class AnimationBuilder {
/** @experimental */ /** @experimental */
export interface AnimationEvent { export interface AnimationEvent {
disabled: boolean;
element: any; element: any;
fromState: string; fromState: string;
phaseName: string; phaseName: string;
@ -199,8 +200,8 @@ export declare function keyframes(steps: AnimationStyleMetadata[]): AnimationKey
/** @experimental */ /** @experimental */
export declare class NoopAnimationPlayer implements AnimationPlayer { export declare class NoopAnimationPlayer implements AnimationPlayer {
parentPlayer: AnimationPlayer | null; parentPlayer: AnimationPlayer | null;
totalTime: number; readonly totalTime: number;
constructor(); constructor(duration?: number, delay?: number);
destroy(): void; destroy(): void;
finish(): void; finish(): void;
getPosition(): number; getPosition(): number;