Skip to content

Commit e1bf067

Browse files
matskovicb
matsko
authored and
vicb
committed
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
1 parent 884de18 commit e1bf067

File tree

12 files changed

+108
-37
lines changed

12 files changed

+108
-37
lines changed

‎packages/animations/browser/src/dsl/animation_transition_factory.ts

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -59,10 +59,13 @@ export class AnimationTransitionFactory {
5959
driver, element, this.ast.animation, enterClassName, leaveClassName, currentStateStyles,
6060
nextStateStyles, animationOptions, subInstructions, errors);
6161

62+
let totalTime = 0;
63+
timelines.forEach(tl => { totalTime = Math.max(tl.duration + tl.delay, totalTime); });
64+
6265
if (errors.length) {
6366
return createTransitionInstruction(
6467
element, this._triggerName, currentState, nextState, isRemoval, currentStateStyles,
65-
nextStateStyles, [], [], preStyleMap, postStyleMap, errors);
68+
nextStateStyles, [], [], preStyleMap, postStyleMap, totalTime, errors);
6669
}
6770

6871
timelines.forEach(tl => {
@@ -81,7 +84,7 @@ export class AnimationTransitionFactory {
8184
const queriedElementsList = iteratorToArray(queriedElements.values());
8285
return createTransitionInstruction(
8386
element, this._triggerName, currentState, nextState, isRemoval, currentStateStyles,
84-
nextStateStyles, timelines, queriedElementsList, preStyleMap, postStyleMap);
87+
nextStateStyles, timelines, queriedElementsList, preStyleMap, postStyleMap, totalTime);
8588
}
8689
}
8790

‎packages/animations/browser/src/dsl/animation_transition_instruction.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ export interface AnimationTransitionInstruction extends AnimationEngineInstructi
2121
queriedElements: any[];
2222
preStyleProps: Map<any, {[prop: string]: boolean}>;
2323
postStyleProps: Map<any, {[prop: string]: boolean}>;
24+
totalTime: number;
2425
errors?: any[];
2526
}
2627

@@ -29,7 +30,7 @@ export function createTransitionInstruction(
2930
isRemovalTransition: boolean, fromStyles: ɵStyleData, toStyles: ɵStyleData,
3031
timelines: AnimationTimelineInstruction[], queriedElements: any[],
3132
preStyleProps: Map<any, {[prop: string]: boolean}>,
32-
postStyleProps: Map<any, {[prop: string]: boolean}>,
33+
postStyleProps: Map<any, {[prop: string]: boolean}>, totalTime: number,
3334
errors?: any[]): AnimationTransitionInstruction {
3435
return {
3536
type: AnimationTransitionInstructionType.TransitionAnimation,
@@ -44,6 +45,7 @@ export function createTransitionInstruction(
4445
queriedElements,
4546
preStyleProps,
4647
postStyleProps,
48+
totalTime,
4749
errors
4850
};
4951
}

‎packages/animations/browser/src/render/animation_driver.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ export class NoopAnimationDriver implements AnimationDriver {
3434
animate(
3535
element: any, keyframes: {[key: string]: string | number}[], duration: number, delay: number,
3636
easing: string, previousPlayers: any[] = []): AnimationPlayer {
37-
return new NoopAnimationPlayer();
37+
return new NoopAnimationPlayer(duration, delay);
3838
}
3939
}
4040

‎packages/animations/browser/src/render/shared.ts

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -75,23 +75,24 @@ export function listenOnPlayer(
7575
callback: (event: any) => any) {
7676
switch (eventName) {
7777
case 'start':
78-
player.onStart(() => callback(event && copyAnimationEvent(event, 'start', player.totalTime)));
78+
player.onStart(() => callback(event && copyAnimationEvent(event, 'start', player)));
7979
break;
8080
case 'done':
81-
player.onDone(() => callback(event && copyAnimationEvent(event, 'done', player.totalTime)));
81+
player.onDone(() => callback(event && copyAnimationEvent(event, 'done', player)));
8282
break;
8383
case 'destroy':
84-
player.onDestroy(
85-
() => callback(event && copyAnimationEvent(event, 'destroy', player.totalTime)));
84+
player.onDestroy(() => callback(event && copyAnimationEvent(event, 'destroy', player)));
8685
break;
8786
}
8887
}
8988

9089
export function copyAnimationEvent(
91-
e: AnimationEvent, phaseName?: string, totalTime?: number): AnimationEvent {
90+
e: AnimationEvent, phaseName: string, player: AnimationPlayer): AnimationEvent {
91+
const totalTime = player.totalTime;
92+
const disabled = (player as any).disabled ? true : false;
9293
const event = makeAnimationEvent(
9394
e.element, e.triggerName, e.fromState, e.toState, phaseName || e.phaseName,
94-
totalTime == undefined ? e.totalTime : totalTime);
95+
totalTime == undefined ? e.totalTime : totalTime, disabled);
9596
const data = (e as any)['_data'];
9697
if (data != null) {
9798
(event as any)['_data'] = data;
@@ -101,8 +102,8 @@ export function copyAnimationEvent(
101102

102103
export function makeAnimationEvent(
103104
element: any, triggerName: string, fromState: string, toState: string, phaseName: string = '',
104-
totalTime: number = 0): AnimationEvent {
105-
return {element, triggerName, fromState, toState, phaseName, totalTime};
105+
totalTime: number = 0, disabled?: boolean): AnimationEvent {
106+
return {element, triggerName, fromState, toState, phaseName, totalTime, disabled: !!disabled};
106107
}
107108

108109
export function getOrSetAsInMap(

‎packages/animations/browser/src/render/transition_animation_engine.ts

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1081,6 +1081,8 @@ export class TransitionAnimationEngine {
10811081
if (subTimelines.has(element)) {
10821082
if (disabledElementsSet.has(element)) {
10831083
player.onDestroy(() => setStyles(element, instruction.toStyles));
1084+
player.disabled = true;
1085+
player.overrideTotalTime(instruction.totalTime);
10841086
skippedPlayers.push(player);
10851087
return;
10861088
}
@@ -1311,7 +1313,8 @@ export class TransitionAnimationEngine {
13111313

13121314
// FIXME (matsko): make sure to-be-removed animations are removed properly
13131315
const details = element[REMOVAL_FLAG];
1314-
if (details && details.removedBeforeQueried) return new NoopAnimationPlayer();
1316+
if (details && details.removedBeforeQueried)
1317+
return new NoopAnimationPlayer(timelineInstruction.duration, timelineInstruction.delay);
13151318

13161319
const isQueriedElement = element !== rootElement;
13171320
const previousPlayers =
@@ -1379,7 +1382,7 @@ export class TransitionAnimationEngine {
13791382

13801383
// special case for when an empty transition|definition is provided
13811384
// ... there is no point in rendering an empty animation
1382-
return new NoopAnimationPlayer();
1385+
return new NoopAnimationPlayer(instruction.duration, instruction.delay);
13831386
}
13841387
}
13851388

@@ -1392,8 +1395,10 @@ export class TransitionAnimationPlayer implements AnimationPlayer {
13921395
public parentPlayer: AnimationPlayer;
13931396

13941397
public markedForDestroy: boolean = false;
1398+
public disabled = false;
13951399

13961400
readonly queued: boolean = true;
1401+
public readonly totalTime: number = 0;
13971402

13981403
constructor(public namespaceId: string, public triggerName: string, public element: any) {}
13991404

@@ -1407,15 +1412,18 @@ export class TransitionAnimationPlayer implements AnimationPlayer {
14071412
});
14081413
this._queuedCallbacks = {};
14091414
this._containsRealPlayer = true;
1415+
this.overrideTotalTime(player.totalTime);
14101416
(this as{queued: boolean}).queued = false;
14111417
}
14121418

14131419
getRealPlayer() { return this._player; }
14141420

1421+
overrideTotalTime(totalTime: number) { (this as any).totalTime = totalTime; }
1422+
14151423
syncPlayerEvents(player: AnimationPlayer) {
14161424
const p = this._player as any;
14171425
if (p.triggerCallback) {
1418-
player.onStart(() => p.triggerCallback('start'));
1426+
player.onStart(() => p.triggerCallback !('start'));
14191427
}
14201428
player.onDone(() => this.finish());
14211429
player.onDestroy(() => this.destroy());
@@ -1473,8 +1481,6 @@ export class TransitionAnimationPlayer implements AnimationPlayer {
14731481

14741482
getPosition(): number { return this.queued ? 0 : this._player.getPosition(); }
14751483

1476-
get totalTime(): number { return this._player.totalTime; }
1477-
14781484
/* @internal */
14791485
triggerCallback(phaseName: string): void {
14801486
const p = this._player as any;

‎packages/animations/browser/test/render/transition_animation_engine_spec.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -299,7 +299,8 @@ const DEFAULT_NAMESPACE_ID = 'id';
299299
phaseName: 'start',
300300
fromState: '123',
301301
toState: '456',
302-
totalTime: 1234
302+
totalTime: 1234,
303+
disabled: false
303304
});
304305

305306
capture = null !;
@@ -313,7 +314,8 @@ const DEFAULT_NAMESPACE_ID = 'id';
313314
phaseName: 'done',
314315
fromState: '123',
315316
toState: '456',
316-
totalTime: 1234
317+
totalTime: 1234,
318+
disabled: false
317319
});
318320
});
319321
});

‎packages/animations/browser/testing/src/mock_animation_driver.ts

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ export class MockAnimationPlayer extends NoopAnimationPlayer {
5858
public element: any, public keyframes: {[key: string]: string | number}[],
5959
public duration: number, public delay: number, public easing: string,
6060
public previousPlayers: any[]) {
61-
super();
61+
super(duration, delay);
6262

6363
if (allowPreviousPlayerStylesMerge(duration, delay)) {
6464
previousPlayers.forEach(player => {
@@ -68,8 +68,6 @@ export class MockAnimationPlayer extends NoopAnimationPlayer {
6868
}
6969
});
7070
}
71-
72-
this.totalTime = delay + duration;
7371
}
7472

7573
/* @internal */

‎packages/animations/src/animation_event.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,4 +44,5 @@ export interface AnimationEvent {
4444
phaseName: string;
4545
element: any;
4646
triggerName: string;
47+
disabled: boolean;
4748
}

‎packages/animations/src/animation_metadata.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -352,6 +352,14 @@ export interface AnimationStaggerMetadata extends AnimationMetadata {
352352
elements located in disabled areas of the template and still animate them as it sees fit. This is
353353
also the case for when a sub animation is queried by a parent and then later animated using {@link
354354
animateChild animateChild}.
355+
356+
* ### Detecting when an animation is disabled
357+
* If a region of the DOM (or the entire application) has its animations disabled, then animation
358+
* trigger callbacks will still fire just as normal (only for zero seconds).
359+
*
360+
* When a trigger callback fires it will provide an instance of an {@link AnimationEvent}. If
361+
animations
362+
* are disabled then the `.disabled` flag on the event will be true.
355363
*
356364
* @experimental Animation support is experimental.
357365
*/

‎packages/animations/src/players/animation_player.ts

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,8 @@ export interface AnimationPlayer {
3333
beforeDestroy?: () => any;
3434
/* @internal */
3535
triggerCallback?: (phaseName: string) => void;
36+
/* @internal */
37+
disabled?: boolean;
3638
}
3739

3840
/**
@@ -46,8 +48,8 @@ export class NoopAnimationPlayer implements AnimationPlayer {
4648
private _destroyed = false;
4749
private _finished = false;
4850
public parentPlayer: AnimationPlayer|null = null;
49-
public totalTime = 0;
50-
constructor() {}
51+
public readonly totalTime: number;
52+
constructor(duration: number = 0, delay: number = 0) { this.totalTime = duration + delay; }
5153
private _onFinish() {
5254
if (!this._finished) {
5355
this._finished = true;
@@ -100,4 +102,4 @@ export class NoopAnimationPlayer implements AnimationPlayer {
100102
methods.forEach(fn => fn());
101103
methods.length = 0;
102104
}
103-
}
105+
}

‎packages/core/test/animation/animation_integration_spec.ts

Lines changed: 57 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,8 @@
55
* Use of this source code is governed by an MIT-style license that can be
66
* found in the LICENSE file at https://angular.io/license
77
*/
8-
import {AUTO_STYLE, AnimationEvent, AnimationOptions, animate, animateChild, group, keyframes, query, state, style, transition, trigger, ɵPRE_STYLE as PRE_STYLE} from '@angular/animations';
9-
import {AnimationDriver, ɵAnimationEngine, ɵNoopAnimationDriver} from '@angular/animations/browser';
8+
import {AUTO_STYLE, AnimationEvent, AnimationOptions, AnimationPlayer, NoopAnimationPlayer, animate, animateChild, group, keyframes, query, state, style, transition, trigger, ɵPRE_STYLE as PRE_STYLE} from '@angular/animations';
9+
import {AnimationDriver, ɵAnimationEngine, ɵNoopAnimationDriver as NoopAnimationDriver} from '@angular/animations/browser';
1010
import {MockAnimationDriver, MockAnimationPlayer} from '@angular/animations/browser/testing';
1111
import {ChangeDetectorRef, Component, HostBinding, HostListener, RendererFactory2, ViewChild} from '@angular/core';
1212
import {ɵDomRendererFactory2} from '@angular/platform-browser';
@@ -112,6 +112,50 @@ const DEFAULT_COMPONENT_ID = '1';
112112
flushMicrotasks();
113113
expect(cmp.log).toEqual(['start', 'done']);
114114
}));
115+
116+
it('should emit the correct totalTime value for a noop-animation', fakeAsync(() => {
117+
@Component({
118+
selector: 'cmp',
119+
template: `
120+
<div [@myAnimation]="exp" (@myAnimation.start)="cb($event)" (@myAnimation.done)="cb($event)"></div>
121+
`,
122+
animations: [
123+
trigger(
124+
'myAnimation',
125+
[
126+
transition(
127+
'* => go',
128+
[
129+
animate('1s', style({opacity: 0})),
130+
]),
131+
]),
132+
]
133+
})
134+
class Cmp {
135+
exp: any = false;
136+
log: AnimationEvent[] = [];
137+
cb(event: AnimationEvent) { this.log.push(event); }
138+
}
139+
140+
TestBed.configureTestingModule({
141+
declarations: [Cmp],
142+
providers: [
143+
{provide: AnimationDriver, useClass: NoopAnimationDriver},
144+
],
145+
});
146+
147+
const fixture = TestBed.createComponent(Cmp);
148+
const cmp = fixture.componentInstance;
149+
cmp.exp = 'go';
150+
fixture.detectChanges();
151+
expect(cmp.log).toEqual([]);
152+
153+
flushMicrotasks();
154+
expect(cmp.log.length).toEqual(2);
155+
const [start, end] = cmp.log;
156+
expect(start.totalTime).toEqual(1000);
157+
expect(end.totalTime).toEqual(1000);
158+
}));
115159
});
116160

117161
describe('component fixture integration', () => {
@@ -166,7 +210,7 @@ const DEFAULT_COMPONENT_ID = '1';
166210
}
167211

168212
TestBed.configureTestingModule({
169-
providers: [{provide: AnimationDriver, useClass: ɵNoopAnimationDriver}],
213+
providers: [{provide: AnimationDriver, useClass: NoopAnimationDriver}],
170214
declarations: [Cmp]
171215
});
172216

@@ -2461,7 +2505,7 @@ const DEFAULT_COMPONENT_ID = '1';
24612505
}
24622506

24632507
TestBed.configureTestingModule({
2464-
providers: [{provide: AnimationDriver, useClass: ɵNoopAnimationDriver}],
2508+
providers: [{provide: AnimationDriver, useClass: NoopAnimationDriver}],
24652509
declarations: [Cmp]
24662510
});
24672511

@@ -2500,7 +2544,7 @@ const DEFAULT_COMPONENT_ID = '1';
25002544
}
25012545

25022546
TestBed.configureTestingModule({
2503-
providers: [{provide: AnimationDriver, useClass: ɵNoopAnimationDriver}],
2547+
providers: [{provide: AnimationDriver, useClass: NoopAnimationDriver}],
25042548
declarations: [Cmp]
25052549
});
25062550

@@ -2971,8 +3015,8 @@ const DEFAULT_COMPONENT_ID = '1';
29713015
class Cmp {
29723016
disableExp = false;
29733017
exp = '';
2974-
startEvent: any;
2975-
doneEvent: any;
3018+
startEvent: AnimationEvent;
3019+
doneEvent: AnimationEvent;
29763020
}
29773021

29783022
TestBed.configureTestingModule({declarations: [Cmp]});
@@ -2988,14 +3032,17 @@ const DEFAULT_COMPONENT_ID = '1';
29883032
cmp.exp = '1';
29893033
fixture.detectChanges();
29903034
flushMicrotasks();
2991-
expect(cmp.startEvent.totalTime).toEqual(0);
2992-
expect(cmp.doneEvent.totalTime).toEqual(0);
3035+
expect(cmp.startEvent.totalTime).toEqual(9876);
3036+
expect(cmp.startEvent.disabled).toBeTruthy();
3037+
expect(cmp.doneEvent.totalTime).toEqual(9876);
3038+
expect(cmp.doneEvent.disabled).toBeTruthy();
29933039

29943040
cmp.exp = '2';
29953041
cmp.disableExp = false;
29963042
fixture.detectChanges();
29973043
flushMicrotasks();
29983044
expect(cmp.startEvent.totalTime).toEqual(9876);
3045+
expect(cmp.startEvent.disabled).toBeFalsy();
29993046
// the done event isn't fired because it's an actual animation
30003047
}));
30013048

@@ -3428,7 +3475,7 @@ const DEFAULT_COMPONENT_ID = '1';
34283475
});
34293476
});
34303477
});
3431-
});
3478+
})();
34323479

34333480
function assertHasParent(element: any, yes: boolean) {
34343481
const parent = getDOM().parentElement(element);

0 commit comments

Comments
 (0)