Skip to content

Commit e6031fb

Browse files
authored
fix(animation): play method resolves when animation is stopped (#28264)
Issue number: N/A --------- <!-- Please do not submit updates to dependencies unless it fixes an issue. --> <!-- Please try to limit your pull request to one type (bugfix, feature, etc). Submit multiple pull requests if needed. --> ## What is the current behavior? <!-- Please describe the current behavior that you are modifying. --> When trying to fix #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? <!-- Please describe the behavior or changes that are being added by this PR. --> - The `play` method now correctly resolves when the animation is cancelled. ## Does this introduce a breaking change? - [ ] Yes - [x] No <!-- If this introduces a breaking change, please describe the impact and migration path for existing applications below. --> ## Other information <!-- Any other information that is important to this PR such as screenshots of how the component looks before and after the change. --> 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.
1 parent d5f0c77 commit e6031fb

File tree

2 files changed

+89
-1
lines changed

2 files changed

+89
-1
lines changed

core/src/utils/animation/animation.ts

Lines changed: 71 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,8 @@ interface AnimationOnFinishCallback {
3030
o?: AnimationCallbackOptions;
3131
}
3232

33+
type AnimationOnStopCallback = AnimationOnFinishCallback;
34+
3335
export const createAnimation = (animationId?: string): Animation => {
3436
let _delay: number | undefined;
3537
let _duration: number | undefined;
@@ -63,6 +65,7 @@ export const createAnimation = (animationId?: string): Animation => {
6365
const id: string | undefined = animationId;
6466
const onFinishCallbacks: AnimationOnFinishCallback[] = [];
6567
const onFinishOneTimeCallbacks: AnimationOnFinishCallback[] = [];
68+
const onStopOneTimeCallbacks: AnimationOnStopCallback[] = [];
6669
const elements: HTMLElement[] = [];
6770
const childAnimations: Animation[] = [];
6871
const stylesheets: HTMLElement[] = [];
@@ -134,6 +137,35 @@ export const createAnimation = (animationId?: string): Animation => {
134137
return numAnimationsRunning !== 0 && !paused;
135138
};
136139

140+
/**
141+
* @internal
142+
* Remove a callback from a chosen callback array
143+
* @param callbackToRemove: A reference to the callback that should be removed
144+
* @param callbackObjects: An array of callbacks that callbackToRemove should be removed from.
145+
*/
146+
const clearCallback = (
147+
callbackToRemove: AnimationLifecycle,
148+
callbackObjects: AnimationOnFinishCallback[] | AnimationOnStopCallback[]
149+
) => {
150+
const index = callbackObjects.findIndex((callbackObject) => callbackObject.c === callbackToRemove);
151+
152+
if (index > -1) {
153+
callbackObjects.splice(index, 1);
154+
}
155+
};
156+
157+
/**
158+
* @internal
159+
* Add a callback to be fired when an animation is stopped/cancelled.
160+
* @param callback: A reference to the callback that should be fired
161+
* @param opts: Any options associated with this particular callback
162+
*/
163+
const onStop = (callback: AnimationLifecycle, opts?: AnimationCallbackOptions) => {
164+
onStopOneTimeCallbacks.push({ c: callback, o: opts });
165+
166+
return ani;
167+
};
168+
137169
const onFinish = (callback: AnimationLifecycle, opts?: AnimationCallbackOptions) => {
138170
const callbacks = opts?.oneTimeCallback ? onFinishOneTimeCallbacks : onFinishCallbacks;
139171
callbacks.push({ c: callback, o: opts });
@@ -953,7 +985,34 @@ export const createAnimation = (animationId?: string): Animation => {
953985
shouldCalculateNumAnimations = false;
954986
}
955987

956-
onFinish(() => resolve(), { oneTimeCallback: true });
988+
/**
989+
* When one of these callbacks fires we
990+
* need to clear the other's callback otherwise
991+
* you can potentially get these callbacks
992+
* firing multiple times if the play method
993+
* is subsequently called.
994+
* Example:
995+
* animation.play() (onStop and onFinish callbacks are registered)
996+
* animation.stop() (onStop callback is fired, onFinish is not)
997+
* animation.play() (onStop and onFinish callbacks are registered)
998+
* Total onStop callbacks: 1
999+
* Total onFinish callbacks: 2
1000+
*/
1001+
const onStopCallback = () => {
1002+
clearCallback(onFinishCallback, onFinishOneTimeCallbacks);
1003+
resolve();
1004+
};
1005+
const onFinishCallback = () => {
1006+
clearCallback(onStopCallback, onStopOneTimeCallbacks);
1007+
resolve();
1008+
};
1009+
1010+
/**
1011+
* The play method resolves when an animation
1012+
* run either finishes or is cancelled.
1013+
*/
1014+
onFinish(onFinishCallback, { oneTimeCallback: true });
1015+
onStop(onStopCallback, { oneTimeCallback: true });
9571016

9581017
childAnimations.forEach((animation) => {
9591018
animation.play();
@@ -969,6 +1028,14 @@ export const createAnimation = (animationId?: string): Animation => {
9691028
});
9701029
};
9711030

1031+
/**
1032+
* Stops an animation and resets it state to the
1033+
* beginning. This does not fire any onFinish
1034+
* callbacks because the animation did not finish.
1035+
* However, since the animation was not destroyed
1036+
* (i.e. the animation could run again) we do not
1037+
* clear the onFinish callbacks.
1038+
*/
9721039
const stop = () => {
9731040
childAnimations.forEach((animation) => {
9741041
animation.stop();
@@ -980,6 +1047,9 @@ export const createAnimation = (animationId?: string): Animation => {
9801047
}
9811048

9821049
resetFlags();
1050+
1051+
onStopOneTimeCallbacks.forEach((onStopCallback) => onStopCallback.c(0, ani));
1052+
onStopOneTimeCallbacks.length = 0;
9831053
};
9841054

9851055
const from = (property: string, value: any) => {

core/src/utils/animation/test/animation.spec.ts

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,24 @@ import { processKeyframes } from '../animation-utils';
44
import { getTimeGivenProgression } from '../cubic-bezier';
55

66
describe('Animation Class', () => {
7+
describe('play()', () => {
8+
it('should resolve when the animation is cancelled', async () => {
9+
// Tell Jest to expect 1 assertion for async code
10+
expect.assertions(1);
11+
const el = document.createElement('div');
12+
const animation = createAnimation()
13+
.addElement(el)
14+
.fromTo('transform', 'translateX(0px)', 'translateX(100px)')
15+
.duration(100000);
16+
17+
const animationPromise = animation.play();
18+
19+
animation.stop();
20+
21+
// Expect that the promise resolves and returns undefined
22+
expect(animationPromise).resolves.toEqual(undefined);
23+
});
24+
});
725
describe('isRunning()', () => {
826
let animation: Animation;
927
beforeEach(() => {

0 commit comments

Comments
 (0)