From e6031fbef0698dac0a346cd6202c47f2abf54f95 Mon Sep 17 00:00:00 2001 From: Liam DeBeasi Date: Mon, 9 Oct 2023 11:16:39 -0400 Subject: [PATCH] fix(animation): play method resolves when animation is stopped (#28264) Issue number: N/A --------- ## What is the current behavior? When trying to fix https://github.com/ionic-team/ionic-framework/issues/20092, I discovered that https://github.com/ionic-team/ionic-framework/blob/ac2c8e6c22da4d0d8224def24ddef56ee9d26246/core/src/components/menu/menu.tsx#L483 was never resolving when the animation was aborted in https://github.com/ionic-team/ionic-framework/blob/ac2c8e6c22da4d0d8224def24ddef56ee9d26246/core/src/components/menu/menu.tsx#L699. This can happen if `menu.disabled` is set to `true` mid-animation. In order to fix the menu bug, I need this promise to resolve when the animation is stopped. ## What is the new behavior? - The `play` method now correctly resolves when the animation is cancelled. ## Does this introduce a breaking change? - [ ] Yes - [x] No ## Other information The `play` method resolves when a particular run of the animation is finished. The `stop` method ensures that this run never finishes which is why I've chosen to have `play` resolve. Note that `onFinish` callbacks should not be fired because the animation run did not complete. --- core/src/utils/animation/animation.ts | 72 ++++++++++++++++++- .../utils/animation/test/animation.spec.ts | 18 +++++ 2 files changed, 89 insertions(+), 1 deletion(-) diff --git a/core/src/utils/animation/animation.ts b/core/src/utils/animation/animation.ts index 8d7b358bed..c547bafd3c 100644 --- a/core/src/utils/animation/animation.ts +++ b/core/src/utils/animation/animation.ts @@ -30,6 +30,8 @@ interface AnimationOnFinishCallback { o?: AnimationCallbackOptions; } +type AnimationOnStopCallback = AnimationOnFinishCallback; + export const createAnimation = (animationId?: string): Animation => { let _delay: number | undefined; let _duration: number | undefined; @@ -63,6 +65,7 @@ export const createAnimation = (animationId?: string): Animation => { const id: string | undefined = animationId; const onFinishCallbacks: AnimationOnFinishCallback[] = []; const onFinishOneTimeCallbacks: AnimationOnFinishCallback[] = []; + const onStopOneTimeCallbacks: AnimationOnStopCallback[] = []; const elements: HTMLElement[] = []; const childAnimations: Animation[] = []; const stylesheets: HTMLElement[] = []; @@ -134,6 +137,35 @@ export const createAnimation = (animationId?: string): Animation => { return numAnimationsRunning !== 0 && !paused; }; + /** + * @internal + * Remove a callback from a chosen callback array + * @param callbackToRemove: A reference to the callback that should be removed + * @param callbackObjects: An array of callbacks that callbackToRemove should be removed from. + */ + const clearCallback = ( + callbackToRemove: AnimationLifecycle, + callbackObjects: AnimationOnFinishCallback[] | AnimationOnStopCallback[] + ) => { + const index = callbackObjects.findIndex((callbackObject) => callbackObject.c === callbackToRemove); + + if (index > -1) { + callbackObjects.splice(index, 1); + } + }; + + /** + * @internal + * Add a callback to be fired when an animation is stopped/cancelled. + * @param callback: A reference to the callback that should be fired + * @param opts: Any options associated with this particular callback + */ + const onStop = (callback: AnimationLifecycle, opts?: AnimationCallbackOptions) => { + onStopOneTimeCallbacks.push({ c: callback, o: opts }); + + return ani; + }; + const onFinish = (callback: AnimationLifecycle, opts?: AnimationCallbackOptions) => { const callbacks = opts?.oneTimeCallback ? onFinishOneTimeCallbacks : onFinishCallbacks; callbacks.push({ c: callback, o: opts }); @@ -953,7 +985,34 @@ export const createAnimation = (animationId?: string): Animation => { shouldCalculateNumAnimations = false; } - onFinish(() => resolve(), { oneTimeCallback: true }); + /** + * When one of these callbacks fires we + * need to clear the other's callback otherwise + * you can potentially get these callbacks + * firing multiple times if the play method + * is subsequently called. + * Example: + * animation.play() (onStop and onFinish callbacks are registered) + * animation.stop() (onStop callback is fired, onFinish is not) + * animation.play() (onStop and onFinish callbacks are registered) + * Total onStop callbacks: 1 + * Total onFinish callbacks: 2 + */ + const onStopCallback = () => { + clearCallback(onFinishCallback, onFinishOneTimeCallbacks); + resolve(); + }; + const onFinishCallback = () => { + clearCallback(onStopCallback, onStopOneTimeCallbacks); + resolve(); + }; + + /** + * The play method resolves when an animation + * run either finishes or is cancelled. + */ + onFinish(onFinishCallback, { oneTimeCallback: true }); + onStop(onStopCallback, { oneTimeCallback: true }); childAnimations.forEach((animation) => { animation.play(); @@ -969,6 +1028,14 @@ export const createAnimation = (animationId?: string): Animation => { }); }; + /** + * Stops an animation and resets it state to the + * beginning. This does not fire any onFinish + * callbacks because the animation did not finish. + * However, since the animation was not destroyed + * (i.e. the animation could run again) we do not + * clear the onFinish callbacks. + */ const stop = () => { childAnimations.forEach((animation) => { animation.stop(); @@ -980,6 +1047,9 @@ export const createAnimation = (animationId?: string): Animation => { } resetFlags(); + + onStopOneTimeCallbacks.forEach((onStopCallback) => onStopCallback.c(0, ani)); + onStopOneTimeCallbacks.length = 0; }; const from = (property: string, value: any) => { diff --git a/core/src/utils/animation/test/animation.spec.ts b/core/src/utils/animation/test/animation.spec.ts index 3f267b6038..b9cfb18c3d 100644 --- a/core/src/utils/animation/test/animation.spec.ts +++ b/core/src/utils/animation/test/animation.spec.ts @@ -4,6 +4,24 @@ import { processKeyframes } from '../animation-utils'; import { getTimeGivenProgression } from '../cubic-bezier'; describe('Animation Class', () => { + describe('play()', () => { + it('should resolve when the animation is cancelled', async () => { + // Tell Jest to expect 1 assertion for async code + expect.assertions(1); + const el = document.createElement('div'); + const animation = createAnimation() + .addElement(el) + .fromTo('transform', 'translateX(0px)', 'translateX(100px)') + .duration(100000); + + const animationPromise = animation.play(); + + animation.stop(); + + // Expect that the promise resolves and returns undefined + expect(animationPromise).resolves.toEqual(undefined); + }); + }); describe('isRunning()', () => { let animation: Animation; beforeEach(() => {