Skip to content

Commit fe35bc3

Browse files
matskovikerman
matsko
authored and
vikerman
committed
fix(animations): allow animations to be destroyed manually (angular#12719)
Closes angular#12456 Closes angular#12719
1 parent ad3bf6c commit fe35bc3

8 files changed

Lines changed: 175 additions & 121 deletions

File tree

‎modules/@angular/compiler/src/animation/animation_compiler.ts‎

Lines changed: 14 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -251,17 +251,22 @@ class _AnimationBuilder implements AnimationAstVisitor {
251251
_ANIMATION_PLAYER_VAR
252252
.callMethod(
253253
'onDone',
254-
[o.fn(
255-
[],
256-
[RENDER_STYLES_FN
257-
.callFn([
258-
_ANIMATION_FACTORY_ELEMENT_VAR, _ANIMATION_FACTORY_RENDERER_VAR,
259-
o.importExpr(resolveIdentifier(Identifiers.prepareFinalAnimationStyles))
254+
[o
255+
.fn([],
256+
[
257+
_ANIMATION_PLAYER_VAR.callMethod('destroy', []).toStmt(),
258+
RENDER_STYLES_FN
260259
.callFn([
261-
_ANIMATION_START_STATE_STYLES_VAR, _ANIMATION_END_STATE_STYLES_VAR
260+
_ANIMATION_FACTORY_ELEMENT_VAR, _ANIMATION_FACTORY_RENDERER_VAR,
261+
o.importExpr(
262+
resolveIdentifier(Identifiers.prepareFinalAnimationStyles))
263+
.callFn([
264+
_ANIMATION_START_STATE_STYLES_VAR,
265+
_ANIMATION_END_STATE_STYLES_VAR
266+
])
262267
])
263-
])
264-
.toStmt()])])
268+
.toStmt()
269+
])])
265270
.toStmt());
266271

267272
statements.push(_ANIMATION_FACTORY_VIEW_CONTEXT

‎modules/@angular/core/src/animation/animation_group_player.ts‎

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ export class AnimationGroupPlayer implements AnimationPlayer {
1515
private _onStartFns: Function[] = [];
1616
private _finished = false;
1717
private _started = false;
18+
private _destroyed = false;
1819

1920
public parentPlayer: AnimationPlayer = null;
2021

@@ -38,9 +39,6 @@ export class AnimationGroupPlayer implements AnimationPlayer {
3839
private _onFinish() {
3940
if (!this._finished) {
4041
this._finished = true;
41-
if (!isPresent(this.parentPlayer)) {
42-
this.destroy();
43-
}
4442
this._onDoneFns.forEach(fn => fn());
4543
this._onDoneFns = [];
4644
}
@@ -76,11 +74,19 @@ export class AnimationGroupPlayer implements AnimationPlayer {
7674
}
7775

7876
destroy(): void {
79-
this._onFinish();
80-
this._players.forEach(player => player.destroy());
77+
if (!this._destroyed) {
78+
this._onFinish();
79+
this._players.forEach(player => player.destroy());
80+
this._destroyed = true;
81+
}
8182
}
8283

83-
reset(): void { this._players.forEach(player => player.reset()); }
84+
reset(): void {
85+
this._players.forEach(player => player.reset());
86+
this._destroyed = false;
87+
this._finished = false;
88+
this._started = false;
89+
}
8490

8591
setPosition(p: any /** TODO #9100 */): void {
8692
this._players.forEach(player => { player.setPosition(p); });

‎modules/@angular/core/src/animation/animation_sequence_player.ts‎

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,8 @@ export class AnimationSequencePlayer implements AnimationPlayer {
1616
private _onDoneFns: Function[] = [];
1717
private _onStartFns: Function[] = [];
1818
private _finished = false;
19-
private _started: boolean = false;
19+
private _started = false;
20+
private _destroyed = false;
2021

2122
public parentPlayer: AnimationPlayer = null;
2223

@@ -48,9 +49,6 @@ export class AnimationSequencePlayer implements AnimationPlayer {
4849
private _onFinish() {
4950
if (!this._finished) {
5051
this._finished = true;
51-
if (!isPresent(this.parentPlayer)) {
52-
this.destroy();
53-
}
5452
this._onDoneFns.forEach(fn => fn());
5553
this._onDoneFns = [];
5654
}
@@ -79,22 +77,30 @@ export class AnimationSequencePlayer implements AnimationPlayer {
7977
pause(): void { this._activePlayer.pause(); }
8078

8179
restart(): void {
80+
this.reset();
8281
if (this._players.length > 0) {
83-
this.reset();
8482
this._players[0].restart();
8583
}
8684
}
8785

88-
reset(): void { this._players.forEach(player => player.reset()); }
86+
reset(): void {
87+
this._players.forEach(player => player.reset());
88+
this._destroyed = false;
89+
this._finished = false;
90+
this._started = false;
91+
}
8992

9093
finish(): void {
9194
this._onFinish();
9295
this._players.forEach(player => player.finish());
9396
}
9497

9598
destroy(): void {
96-
this._onFinish();
97-
this._players.forEach(player => player.destroy());
99+
if (!this._destroyed) {
100+
this._onFinish();
101+
this._players.forEach(player => player.destroy());
102+
this._destroyed = true;
103+
}
98104
}
99105

100106
setPosition(p: any /** TODO #9100 */): void { this._players[0].setPosition(p); }

‎modules/@angular/core/test/animation/animation_group_player_spec.ts‎

Lines changed: 47 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@ export function main() {
9595
assertLastStatus(players[2], 'restart', true);
9696
});
9797

98-
it('should finish all the players', () => {
98+
it('should not destroy the inner the players when finished', () => {
9999
var group = new AnimationGroupPlayer(players);
100100

101101
var completed = false;
@@ -113,55 +113,36 @@ export function main() {
113113

114114
group.finish();
115115

116-
assertLastStatus(players[0], 'finish', true, -1);
117-
assertLastStatus(players[1], 'finish', true, -1);
118-
assertLastStatus(players[2], 'finish', true, -1);
119-
120-
assertLastStatus(players[0], 'destroy', true);
121-
assertLastStatus(players[1], 'destroy', true);
122-
assertLastStatus(players[2], 'destroy', true);
116+
assertLastStatus(players[0], 'finish', true);
117+
assertLastStatus(players[1], 'finish', true);
118+
assertLastStatus(players[2], 'finish', true);
123119

124120
expect(completed).toBeTruthy();
125121
});
126122

127-
it('should call destroy automatically when finished if no parent player is present', () => {
128-
var group = new AnimationGroupPlayer(players);
123+
it('should not call destroy automatically when finished even if a parent player finishes',
124+
() => {
125+
var group = new AnimationGroupPlayer(players);
126+
var parent = new AnimationGroupPlayer([group, new MockAnimationPlayer()]);
129127

130-
group.play();
128+
group.play();
131129

132-
assertLastStatus(players[0], 'destroy', false);
133-
assertLastStatus(players[1], 'destroy', false);
134-
assertLastStatus(players[2], 'destroy', false);
130+
assertLastStatus(players[0], 'destroy', false);
131+
assertLastStatus(players[1], 'destroy', false);
132+
assertLastStatus(players[2], 'destroy', false);
135133

136-
group.finish();
134+
group.finish();
137135

138-
assertLastStatus(players[0], 'destroy', true);
139-
assertLastStatus(players[1], 'destroy', true);
140-
assertLastStatus(players[2], 'destroy', true);
141-
});
136+
assertLastStatus(players[0], 'destroy', false);
137+
assertLastStatus(players[1], 'destroy', false);
138+
assertLastStatus(players[2], 'destroy', false);
142139

143-
it('should not call destroy automatically when finished if a parent player is present', () => {
144-
var group = new AnimationGroupPlayer(players);
145-
var parent = new AnimationGroupPlayer([group, new MockAnimationPlayer()]);
146-
147-
group.play();
148-
149-
assertLastStatus(players[0], 'destroy', false);
150-
assertLastStatus(players[1], 'destroy', false);
151-
assertLastStatus(players[2], 'destroy', false);
152-
153-
group.finish();
140+
parent.finish();
154141

155-
assertLastStatus(players[0], 'destroy', false);
156-
assertLastStatus(players[1], 'destroy', false);
157-
assertLastStatus(players[2], 'destroy', false);
158-
159-
parent.finish();
160-
161-
assertLastStatus(players[0], 'destroy', true);
162-
assertLastStatus(players[1], 'destroy', true);
163-
assertLastStatus(players[2], 'destroy', true);
164-
});
142+
assertLastStatus(players[0], 'destroy', false);
143+
assertLastStatus(players[1], 'destroy', false);
144+
assertLastStatus(players[2], 'destroy', false);
145+
});
165146

166147
it('should function without any players', () => {
167148
var group = new AnimationGroupPlayer([]);
@@ -193,5 +174,31 @@ export function main() {
193174
flushMicrotasks();
194175
expect(completed).toEqual(true);
195176
}));
177+
178+
it('should not allow the player to be destroyed if it already has been destroyed unless reset',
179+
fakeAsync(() => {
180+
var p1 = new MockAnimationPlayer();
181+
var p2 = new MockAnimationPlayer();
182+
var innerPlayers = [p1, p2];
183+
184+
var groupPlayer = new AnimationGroupPlayer(innerPlayers);
185+
expect(p1.log[p1.log.length - 1]).not.toContain('destroy');
186+
expect(p2.log[p2.log.length - 1]).not.toContain('destroy');
187+
188+
groupPlayer.destroy();
189+
expect(p1.log[p1.log.length - 1]).toContain('destroy');
190+
expect(p2.log[p2.log.length - 1]).toContain('destroy');
191+
192+
p1.log = p2.log = [];
193+
194+
groupPlayer.destroy();
195+
expect(p1.log[p1.log.length - 1]).not.toContain('destroy');
196+
expect(p2.log[p2.log.length - 1]).not.toContain('destroy');
197+
198+
groupPlayer.reset();
199+
groupPlayer.destroy();
200+
expect(p1.log[p1.log.length - 1]).toContain('destroy');
201+
expect(p2.log[p2.log.length - 1]).toContain('destroy');
202+
}));
196203
});
197204
}

‎modules/@angular/core/test/animation/animation_sequence_player_spec.ts‎

Lines changed: 46 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -135,55 +135,36 @@ export function main() {
135135

136136
sequence.finish();
137137

138-
assertLastStatus(players[0], 'finish', true, -1);
139-
assertLastStatus(players[1], 'finish', true, -1);
140-
assertLastStatus(players[2], 'finish', true, -1);
141-
142-
assertLastStatus(players[0], 'destroy', true);
143-
assertLastStatus(players[1], 'destroy', true);
144-
assertLastStatus(players[2], 'destroy', true);
138+
assertLastStatus(players[0], 'finish', true);
139+
assertLastStatus(players[1], 'finish', true);
140+
assertLastStatus(players[2], 'finish', true);
145141

146142
expect(completed).toBeTruthy();
147143
});
148144

149-
it('should call destroy automatically when finished if no parent player is present', () => {
150-
var sequence = new AnimationSequencePlayer(players);
145+
it('should not call destroy automatically when finished even if a parent player is present',
146+
() => {
147+
var sequence = new AnimationSequencePlayer(players);
148+
var parent = new AnimationSequencePlayer([sequence, new MockAnimationPlayer()]);
151149

152-
sequence.play();
150+
sequence.play();
153151

154-
assertLastStatus(players[0], 'destroy', false);
155-
assertLastStatus(players[1], 'destroy', false);
156-
assertLastStatus(players[2], 'destroy', false);
152+
assertLastStatus(players[0], 'destroy', false);
153+
assertLastStatus(players[1], 'destroy', false);
154+
assertLastStatus(players[2], 'destroy', false);
157155

158-
sequence.finish();
156+
sequence.finish();
159157

160-
assertLastStatus(players[0], 'destroy', true);
161-
assertLastStatus(players[1], 'destroy', true);
162-
assertLastStatus(players[2], 'destroy', true);
163-
});
158+
assertLastStatus(players[0], 'destroy', false);
159+
assertLastStatus(players[1], 'destroy', false);
160+
assertLastStatus(players[2], 'destroy', false);
164161

165-
it('should not call destroy automatically when finished if a parent player is present', () => {
166-
var sequence = new AnimationSequencePlayer(players);
167-
var parent = new AnimationSequencePlayer([sequence, new MockAnimationPlayer()]);
168-
169-
sequence.play();
170-
171-
assertLastStatus(players[0], 'destroy', false);
172-
assertLastStatus(players[1], 'destroy', false);
173-
assertLastStatus(players[2], 'destroy', false);
174-
175-
sequence.finish();
162+
parent.finish();
176163

177-
assertLastStatus(players[0], 'destroy', false);
178-
assertLastStatus(players[1], 'destroy', false);
179-
assertLastStatus(players[2], 'destroy', false);
180-
181-
parent.finish();
182-
183-
assertLastStatus(players[0], 'destroy', true);
184-
assertLastStatus(players[1], 'destroy', true);
185-
assertLastStatus(players[2], 'destroy', true);
186-
});
164+
assertLastStatus(players[0], 'destroy', false);
165+
assertLastStatus(players[1], 'destroy', false);
166+
assertLastStatus(players[2], 'destroy', false);
167+
});
187168

188169
it('should function without any players', () => {
189170
var sequence = new AnimationSequencePlayer([]);
@@ -215,5 +196,31 @@ export function main() {
215196
flushMicrotasks();
216197
expect(completed).toEqual(true);
217198
}));
199+
200+
it('should not allow the player to be destroyed if it already has been destroyed unless reset',
201+
fakeAsync(() => {
202+
var p1 = new MockAnimationPlayer();
203+
var p2 = new MockAnimationPlayer();
204+
var innerPlayers = [p1, p2];
205+
206+
var sequencePlayer = new AnimationSequencePlayer(innerPlayers);
207+
expect(p1.log[p1.log.length - 1]).not.toContain('destroy');
208+
expect(p2.log[p2.log.length - 1]).not.toContain('destroy');
209+
210+
sequencePlayer.destroy();
211+
expect(p1.log[p1.log.length - 1]).toContain('destroy');
212+
expect(p2.log[p2.log.length - 1]).toContain('destroy');
213+
214+
p1.log = p2.log = [];
215+
216+
sequencePlayer.destroy();
217+
expect(p1.log[p1.log.length - 1]).not.toContain('destroy');
218+
expect(p2.log[p2.log.length - 1]).not.toContain('destroy');
219+
220+
sequencePlayer.reset();
221+
sequencePlayer.destroy();
222+
expect(p1.log[p1.log.length - 1]).toContain('destroy');
223+
expect(p2.log[p2.log.length - 1]).toContain('destroy');
224+
}));
218225
});
219226
}

‎modules/@angular/core/testing/mock_animation_player.ts‎

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -7,14 +7,13 @@
77
*/
88

99
import {AnimationPlayer} from '@angular/core';
10-
import {isPresent} from './facade/lang';
1110

1211
export class MockAnimationPlayer implements AnimationPlayer {
1312
private _onDoneFns: Function[] = [];
1413
private _onStartFns: Function[] = [];
1514
private _finished = false;
1615
private _destroyed = false;
17-
private _started: boolean = false;
16+
private _started = false;
1817

1918
public parentPlayer: AnimationPlayer = null;
2019

@@ -27,9 +26,6 @@ export class MockAnimationPlayer implements AnimationPlayer {
2726

2827
this._onDoneFns.forEach(fn => fn());
2928
this._onDoneFns = [];
30-
if (!isPresent(this.parentPlayer)) {
31-
this.destroy();
32-
}
3329
}
3430
}
3531

@@ -56,7 +52,12 @@ export class MockAnimationPlayer implements AnimationPlayer {
5652

5753
finish(): void { this._onFinish(); }
5854

59-
reset(): void { this.log.push('reset'); }
55+
reset(): void {
56+
this.log.push('reset');
57+
this._destroyed = false;
58+
this._finished = false;
59+
this._started = false;
60+
}
6061

6162
destroy(): void {
6263
if (!this._destroyed) {

0 commit comments

Comments
 (0)