From 3e91a65fb67c27734b124d5c5a3d5989aad7955a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matias=20Niemel=C3=A4?= Date: Fri, 4 Nov 2016 13:23:12 -0700 Subject: [PATCH] fix(animations): allow animations to be destroyed manually --- .../src/animation/animation_compiler.ts | 23 ++++--- .../src/animation/animation_group_player.ts | 3 - .../animation/animation_sequence_player.ts | 3 - .../animation/animation_group_player_spec.ts | 61 +++++++------------ .../animation_sequence_player_spec.ts | 59 ++++++------------ .../core/testing/mock_animation_player.ts | 4 -- .../src/dom/web_animations_player.ts | 4 -- .../test/dom/web_animations_player_spec.ts | 4 +- 8 files changed, 57 insertions(+), 104 deletions(-) diff --git a/modules/@angular/compiler/src/animation/animation_compiler.ts b/modules/@angular/compiler/src/animation/animation_compiler.ts index 0ad4501dd1dc37..633aa47a1e213e 100644 --- a/modules/@angular/compiler/src/animation/animation_compiler.ts +++ b/modules/@angular/compiler/src/animation/animation_compiler.ts @@ -251,17 +251,22 @@ class _AnimationBuilder implements AnimationAstVisitor { _ANIMATION_PLAYER_VAR .callMethod( 'onDone', - [o.fn( - [], - [RENDER_STYLES_FN - .callFn([ - _ANIMATION_FACTORY_ELEMENT_VAR, _ANIMATION_FACTORY_RENDERER_VAR, - o.importExpr(resolveIdentifier(Identifiers.prepareFinalAnimationStyles)) + [o + .fn([], + [ + _ANIMATION_PLAYER_VAR.callMethod('destroy', []).toStmt(), + RENDER_STYLES_FN .callFn([ - _ANIMATION_START_STATE_STYLES_VAR, _ANIMATION_END_STATE_STYLES_VAR + _ANIMATION_FACTORY_ELEMENT_VAR, _ANIMATION_FACTORY_RENDERER_VAR, + o.importExpr( + resolveIdentifier(Identifiers.prepareFinalAnimationStyles)) + .callFn([ + _ANIMATION_START_STATE_STYLES_VAR, + _ANIMATION_END_STATE_STYLES_VAR + ]) ]) - ]) - .toStmt()])]) + .toStmt() + ])]) .toStmt()); statements.push(_ANIMATION_FACTORY_VIEW_CONTEXT diff --git a/modules/@angular/core/src/animation/animation_group_player.ts b/modules/@angular/core/src/animation/animation_group_player.ts index 9145d4abed247e..daf0d0b8a1fd71 100644 --- a/modules/@angular/core/src/animation/animation_group_player.ts +++ b/modules/@angular/core/src/animation/animation_group_player.ts @@ -38,9 +38,6 @@ export class AnimationGroupPlayer implements AnimationPlayer { private _onFinish() { if (!this._finished) { this._finished = true; - if (!isPresent(this.parentPlayer)) { - this.destroy(); - } this._onDoneFns.forEach(fn => fn()); this._onDoneFns = []; } diff --git a/modules/@angular/core/src/animation/animation_sequence_player.ts b/modules/@angular/core/src/animation/animation_sequence_player.ts index 28becdf9bb9aba..20545bacb205f9 100644 --- a/modules/@angular/core/src/animation/animation_sequence_player.ts +++ b/modules/@angular/core/src/animation/animation_sequence_player.ts @@ -48,9 +48,6 @@ export class AnimationSequencePlayer implements AnimationPlayer { private _onFinish() { if (!this._finished) { this._finished = true; - if (!isPresent(this.parentPlayer)) { - this.destroy(); - } this._onDoneFns.forEach(fn => fn()); this._onDoneFns = []; } diff --git a/modules/@angular/core/test/animation/animation_group_player_spec.ts b/modules/@angular/core/test/animation/animation_group_player_spec.ts index bb259cea599305..9958e3a4fe25d9 100644 --- a/modules/@angular/core/test/animation/animation_group_player_spec.ts +++ b/modules/@angular/core/test/animation/animation_group_player_spec.ts @@ -95,7 +95,7 @@ export function main() { assertLastStatus(players[2], 'restart', true); }); - it('should finish all the players', () => { + it('should not destroy the inner the players when finished', () => { var group = new AnimationGroupPlayer(players); var completed = false; @@ -113,55 +113,36 @@ export function main() { group.finish(); - assertLastStatus(players[0], 'finish', true, -1); - assertLastStatus(players[1], 'finish', true, -1); - assertLastStatus(players[2], 'finish', true, -1); - - assertLastStatus(players[0], 'destroy', true); - assertLastStatus(players[1], 'destroy', true); - assertLastStatus(players[2], 'destroy', true); + assertLastStatus(players[0], 'finish', true); + assertLastStatus(players[1], 'finish', true); + assertLastStatus(players[2], 'finish', true); expect(completed).toBeTruthy(); }); - it('should call destroy automatically when finished if no parent player is present', () => { - var group = new AnimationGroupPlayer(players); - - group.play(); - - assertLastStatus(players[0], 'destroy', false); - assertLastStatus(players[1], 'destroy', false); - assertLastStatus(players[2], 'destroy', false); - - group.finish(); - - assertLastStatus(players[0], 'destroy', true); - assertLastStatus(players[1], 'destroy', true); - assertLastStatus(players[2], 'destroy', true); - }); - - it('should not call destroy automatically when finished if a parent player is present', () => { - var group = new AnimationGroupPlayer(players); - var parent = new AnimationGroupPlayer([group, new MockAnimationPlayer()]); + it('should not call destroy automatically when finished even if a parent player finishes', + () => { + var group = new AnimationGroupPlayer(players); + var parent = new AnimationGroupPlayer([group, new MockAnimationPlayer()]); - group.play(); + group.play(); - assertLastStatus(players[0], 'destroy', false); - assertLastStatus(players[1], 'destroy', false); - assertLastStatus(players[2], 'destroy', false); + assertLastStatus(players[0], 'destroy', false); + assertLastStatus(players[1], 'destroy', false); + assertLastStatus(players[2], 'destroy', false); - group.finish(); + group.finish(); - assertLastStatus(players[0], 'destroy', false); - assertLastStatus(players[1], 'destroy', false); - assertLastStatus(players[2], 'destroy', false); + assertLastStatus(players[0], 'destroy', false); + assertLastStatus(players[1], 'destroy', false); + assertLastStatus(players[2], 'destroy', false); - parent.finish(); + parent.finish(); - assertLastStatus(players[0], 'destroy', true); - assertLastStatus(players[1], 'destroy', true); - assertLastStatus(players[2], 'destroy', true); - }); + assertLastStatus(players[0], 'destroy', false); + assertLastStatus(players[1], 'destroy', false); + assertLastStatus(players[2], 'destroy', false); + }); it('should function without any players', () => { var group = new AnimationGroupPlayer([]); diff --git a/modules/@angular/core/test/animation/animation_sequence_player_spec.ts b/modules/@angular/core/test/animation/animation_sequence_player_spec.ts index 5a90f991ed4ed1..6c59eadf421382 100644 --- a/modules/@angular/core/test/animation/animation_sequence_player_spec.ts +++ b/modules/@angular/core/test/animation/animation_sequence_player_spec.ts @@ -135,55 +135,36 @@ export function main() { sequence.finish(); - assertLastStatus(players[0], 'finish', true, -1); - assertLastStatus(players[1], 'finish', true, -1); - assertLastStatus(players[2], 'finish', true, -1); - - assertLastStatus(players[0], 'destroy', true); - assertLastStatus(players[1], 'destroy', true); - assertLastStatus(players[2], 'destroy', true); + assertLastStatus(players[0], 'finish', true); + assertLastStatus(players[1], 'finish', true); + assertLastStatus(players[2], 'finish', true); expect(completed).toBeTruthy(); }); - it('should call destroy automatically when finished if no parent player is present', () => { - var sequence = new AnimationSequencePlayer(players); - - sequence.play(); - - assertLastStatus(players[0], 'destroy', false); - assertLastStatus(players[1], 'destroy', false); - assertLastStatus(players[2], 'destroy', false); - - sequence.finish(); - - assertLastStatus(players[0], 'destroy', true); - assertLastStatus(players[1], 'destroy', true); - assertLastStatus(players[2], 'destroy', true); - }); - - it('should not call destroy automatically when finished if a parent player is present', () => { - var sequence = new AnimationSequencePlayer(players); - var parent = new AnimationSequencePlayer([sequence, new MockAnimationPlayer()]); + it('should not call destroy automatically when finished even if a parent player is present', + () => { + var sequence = new AnimationSequencePlayer(players); + var parent = new AnimationSequencePlayer([sequence, new MockAnimationPlayer()]); - sequence.play(); + sequence.play(); - assertLastStatus(players[0], 'destroy', false); - assertLastStatus(players[1], 'destroy', false); - assertLastStatus(players[2], 'destroy', false); + assertLastStatus(players[0], 'destroy', false); + assertLastStatus(players[1], 'destroy', false); + assertLastStatus(players[2], 'destroy', false); - sequence.finish(); + sequence.finish(); - assertLastStatus(players[0], 'destroy', false); - assertLastStatus(players[1], 'destroy', false); - assertLastStatus(players[2], 'destroy', false); + assertLastStatus(players[0], 'destroy', false); + assertLastStatus(players[1], 'destroy', false); + assertLastStatus(players[2], 'destroy', false); - parent.finish(); + parent.finish(); - assertLastStatus(players[0], 'destroy', true); - assertLastStatus(players[1], 'destroy', true); - assertLastStatus(players[2], 'destroy', true); - }); + assertLastStatus(players[0], 'destroy', false); + assertLastStatus(players[1], 'destroy', false); + assertLastStatus(players[2], 'destroy', false); + }); it('should function without any players', () => { var sequence = new AnimationSequencePlayer([]); diff --git a/modules/@angular/core/testing/mock_animation_player.ts b/modules/@angular/core/testing/mock_animation_player.ts index 65b1bff4ce5952..c887fcd19f23cd 100644 --- a/modules/@angular/core/testing/mock_animation_player.ts +++ b/modules/@angular/core/testing/mock_animation_player.ts @@ -7,7 +7,6 @@ */ import {AnimationPlayer} from '@angular/core'; -import {isPresent} from './facade/lang'; export class MockAnimationPlayer implements AnimationPlayer { private _onDoneFns: Function[] = []; @@ -27,9 +26,6 @@ export class MockAnimationPlayer implements AnimationPlayer { this._onDoneFns.forEach(fn => fn()); this._onDoneFns = []; - if (!isPresent(this.parentPlayer)) { - this.destroy(); - } } } diff --git a/modules/@angular/platform-browser/src/dom/web_animations_player.ts b/modules/@angular/platform-browser/src/dom/web_animations_player.ts index db1eb4495a13a7..8c39e85c46c19a 100644 --- a/modules/@angular/platform-browser/src/dom/web_animations_player.ts +++ b/modules/@angular/platform-browser/src/dom/web_animations_player.ts @@ -7,7 +7,6 @@ */ import {AUTO_STYLE} from '@angular/core'; -import {isPresent} from '../facade/lang'; import {AnimationPlayer} from '../private_import_core'; import {getDOM} from './dom_adapter'; @@ -33,9 +32,6 @@ export class WebAnimationsPlayer implements AnimationPlayer { private _onFinish() { if (!this._finished) { this._finished = true; - if (!isPresent(this.parentPlayer)) { - this.destroy(); - } this._onDoneFns.forEach(fn => fn()); this._onDoneFns = []; } diff --git a/modules/@angular/platform-browser/test/dom/web_animations_player_spec.ts b/modules/@angular/platform-browser/test/dom/web_animations_player_spec.ts index c89c1622cb25bd..b290772ed7e9f6 100644 --- a/modules/@angular/platform-browser/test/dom/web_animations_player_spec.ts +++ b/modules/@angular/platform-browser/test/dom/web_animations_player_spec.ts @@ -110,12 +110,12 @@ export function main() { expect(count).toEqual(2); }); - it('should destroy itself automatically if a parent player is not present', () => { + it('should not destroy itself automatically if a parent player is not present', () => { captures['cancel'] = []; player.finish(); expect(captures['finish'].length).toEqual(1); - expect(captures['cancel'].length).toEqual(1); + expect(captures['cancel'].length).toEqual(0); var next = makePlayer(); var player2 = next['player'];