From a2a9c14d0c2b2c42fa5a3f3843a697755ed0342b Mon Sep 17 00:00:00 2001 From: iRealNirmal Date: Wed, 14 Apr 2021 09:37:11 +0530 Subject: [PATCH] fix(platform-browser): update started state on reset (#41608) This commit fixes the state of variable _started on call of reset method. Earlier behaviour was on call of `reset()` method we are not setting back `_started` flag's value to false and it created various issue for end user which were expecting this behaviour as per name of method. We provided end user `reset()` method, but it was empty method and on call of it wasn't doing anything. As end user/developer were not able to reuse animation not animation after call of reset method. In this PR on call of `reset()` method we are setting flag `_started` value to false which can be accessed by `hasStarted()`. Resolves #18140 PR Close #41608 --- .../animations/browser/testing/testing.d.ts | 1 + .../css_keyframes/css_keyframes_player.ts | 3 +- .../testing/src/mock_animation_driver.ts | 5 +++ .../src/players/animation_player.ts | 4 +- .../animations/src/animation_builder.ts | 1 + .../test/browser_animation_builder_spec.ts | 38 +++++++++++++++++++ 6 files changed, 50 insertions(+), 2 deletions(-) diff --git a/goldens/public-api/animations/browser/testing/testing.d.ts b/goldens/public-api/animations/browser/testing/testing.d.ts index b127115a5bfbf..b3cc0242e8061 100644 --- a/goldens/public-api/animations/browser/testing/testing.d.ts +++ b/goldens/public-api/animations/browser/testing/testing.d.ts @@ -31,4 +31,5 @@ export declare class MockAnimationPlayer extends NoopAnimationPlayer { finish(): void; hasStarted(): boolean; play(): void; + reset(): void; } diff --git a/packages/animations/browser/src/render/css_keyframes/css_keyframes_player.ts b/packages/animations/browser/src/render/css_keyframes/css_keyframes_player.ts index c047e0511f866..4facf82ff5aac 100644 --- a/packages/animations/browser/src/render/css_keyframes/css_keyframes_player.ts +++ b/packages/animations/browser/src/render/css_keyframes/css_keyframes_player.ts @@ -15,6 +15,7 @@ const DEFAULT_FILL_MODE = 'forwards'; const DEFAULT_EASING = 'linear'; export const enum AnimatorControlState { + RESET = 0, INITIALIZED = 1, STARTED = 2, FINISHED = 3, @@ -26,7 +27,6 @@ export class CssKeyframesPlayer implements AnimationPlayer { private _onStartFns: Function[] = []; private _onDestroyFns: Function[] = []; - private _started = false; // TODO(issue/24571): remove '!'. private _styler!: ElementAnimationStyleHandler; @@ -139,6 +139,7 @@ export class CssKeyframesPlayer implements AnimationPlayer { this.play(); } reset(): void { + this._state = AnimatorControlState.RESET; this._styler.destroy(); this._buildStyler(); this._styler.apply(); diff --git a/packages/animations/browser/testing/src/mock_animation_driver.ts b/packages/animations/browser/testing/src/mock_animation_driver.ts index 93f962d3963b5..b4d3dd46587d1 100644 --- a/packages/animations/browser/testing/src/mock_animation_driver.ts +++ b/packages/animations/browser/testing/src/mock_animation_driver.ts @@ -83,6 +83,11 @@ export class MockAnimationPlayer extends NoopAnimationPlayer { this._onInitFns = []; } + reset() { + super.reset(); + this.__started = false; + } + finish(): void { super.finish(); this.__finished = true; diff --git a/packages/animations/src/players/animation_player.ts b/packages/animations/src/players/animation_player.ts index 607a996b6a8a6..06775a0c254c1 100644 --- a/packages/animations/src/players/animation_player.ts +++ b/packages/animations/src/players/animation_player.ts @@ -184,7 +184,9 @@ export class NoopAnimationPlayer implements AnimationPlayer { this._onDestroyFns = []; } } - reset(): void {} + reset(): void { + this._started = false; + } setPosition(position: number): void { this._position = this.totalTime ? position * this.totalTime : 1; } diff --git a/packages/platform-browser/animations/src/animation_builder.ts b/packages/platform-browser/animations/src/animation_builder.ts index 057b1b5d55685..74959020dee27 100644 --- a/packages/platform-browser/animations/src/animation_builder.ts +++ b/packages/platform-browser/animations/src/animation_builder.ts @@ -104,6 +104,7 @@ export class RendererAnimationPlayer implements AnimationPlayer { reset(): void { this._command('reset'); + this._started = false; } setPosition(p: number): void { diff --git a/packages/platform-browser/animations/test/browser_animation_builder_spec.ts b/packages/platform-browser/animations/test/browser_animation_builder_spec.ts index 7a45c7d2e9a57..04ec02c68ae9d 100644 --- a/packages/platform-browser/animations/test/browser_animation_builder_spec.ts +++ b/packages/platform-browser/animations/test/browser_animation_builder_spec.ts @@ -104,5 +104,43 @@ import {el} from '../../testing/src/browser_util'; expect(finished).toBeTruthy(); expect(destroyed).toBeTruthy(); })); + + it('should update `hasStarted()` on `play()` and `reset()`', fakeAsync(() => { + @Component({ + selector: 'ani-another-cmp', + template: '...', + }) + class CmpAnother { + @ViewChild('target') public target: any; + + constructor(public builder: AnimationBuilder) {} + + build() { + const definition = + this.builder.build([style({opacity: 0}), animate(1000, style({opacity: 1}))]); + + return definition.create(this.target); + } + } + + TestBed.configureTestingModule({declarations: [CmpAnother]}); + + const fixture = TestBed.createComponent(CmpAnother); + const cmp = fixture.componentInstance; + fixture.detectChanges(); + + const player = cmp.build(); + + expect(player.hasStarted()).toBeFalsy(); + flushMicrotasks(); + + player.play(); + flushMicrotasks(); + expect(player.hasStarted()).toBeTruthy(); + + player.reset(); + flushMicrotasks(); + expect(player.hasStarted()).toBeFalsy(); + })); }); }