Skip to content

Commit 950fa40

Browse files
fix(overlays): tear down animations after dismiss (#28907)
Issue number: resolves #28352 --------- <!-- 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. --> Not all animations are getting properly destroyed after they run. This means that styles can get stuck at the end value of their animation. Specifically for this reproduction (as reported in the bug ticket), if the modal animates from 1 to 0 opacity and then the modal gets reopened with a different animation where the opacity should be 1 throughout (i.e. the opacity isn't supposed to animate at all), the modal is invisible because the opacity got stuck at 0 and never got reset to the default value of 1. This bug is probably causing some incorrect behavior on other edge cases with overlays, but this is the only one I've identified. ### Reproduction steps Note: you cannot reproduce this when using a modalController 1. Open a modal, e.g. [this one](http://localhost:3333/src/components/modal/test/card-nav?ionic:mode=ios) in `ios` mode on a screen wider than 768px 1. Close the modal 1. Open the same modal on a screen narrower than 768px 1. See that the modal does not appear ## What is the new behavior? <!-- Please describe the behavior or changes that are being added by this PR. --> - Animations are properly destroyed after the animation completes - The modal now appears as expected after following the reproduction steps above ## Does this introduce a breaking change? - [ ] Yes - [x] No <!-- If this introduces a breaking change: 1. Describe the impact and migration path for existing applications below. 2. Update the BREAKING.md file with the breaking change. 3. Add "BREAKING CHANGE: [...]" to the commit description when merging. See https://github.com/ionic-team/ionic-framework/blob/main/.github/CONTRIBUTING.md#footer for more information. --> <!-- ## Other information Any other information that is important to this PR such as screenshots of how the component looks before and after the change. --> --------- Co-authored-by: Liam DeBeasi <liamdebeasi@users.noreply.github.com>
1 parent d5dc901 commit 950fa40

File tree

3 files changed

+51
-5
lines changed

3 files changed

+51
-5
lines changed

core/src/components/modal/modal.tsx

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@ import { Style as StatusBarStyle, StatusBar } from '@utils/native/status-bar';
1010
import {
1111
GESTURE,
1212
BACKDROP,
13-
activeAnimations,
1413
dismiss,
1514
eventMethod,
1615
prepareOverlay,
@@ -705,8 +704,6 @@ export class Modal implements ComponentInterface, OverlayInterface {
705704
this.keyboardOpenCallback = undefined;
706705
}
707706

708-
const enteringAnimation = activeAnimations.get(this) || [];
709-
710707
const dismissed = await dismiss<ModalDismissOptions>(
711708
this,
712709
data,
@@ -733,8 +730,6 @@ export class Modal implements ComponentInterface, OverlayInterface {
733730
if (this.gesture) {
734731
this.gesture.destroy();
735732
}
736-
737-
enteringAnimation.forEach((ani) => ani.destroy());
738733
}
739734
this.currentBreakpoint = undefined;
740735
this.animation = undefined;
Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
import { expect } from '@playwright/test';
2+
import { configs, test } from '@utils/test/playwright';
3+
4+
configs({ modes: ['ios'], directions: ['ltr'] }).forEach(({ config, title }) => {
5+
test.describe(title('modal: animations'), () => {
6+
test.beforeEach(async ({ page }) => {
7+
await page.setContent(
8+
`
9+
<ion-modal is-open="true" trigger="open-modal"></ion-modal>
10+
`,
11+
config
12+
);
13+
});
14+
test('card modal should clean up animations on dismiss', async ({ page }, testInfo) => {
15+
testInfo.annotations.push({
16+
type: 'issue',
17+
description: 'https://github.com/ionic-team/ionic-framework/issues/28352',
18+
});
19+
20+
const ionModalDidDismiss = await page.spyOnEvent('ionModalDidDismiss');
21+
22+
const modal = page.locator('ion-modal');
23+
24+
const initialAnimations = await modal.evaluate((el: HTMLIonModalElement) => {
25+
return el.shadowRoot!.getAnimations();
26+
});
27+
28+
// While the modal is open, it should have animations
29+
expect(initialAnimations.length).toBeGreaterThan(0);
30+
31+
await modal.evaluate((el: HTMLIonModalElement) => {
32+
el.dismiss();
33+
});
34+
35+
await ionModalDidDismiss.next();
36+
37+
const currentAnimations = await modal.evaluate((el: HTMLIonModalElement) => {
38+
return el.shadowRoot!.getAnimations();
39+
});
40+
41+
// Once the modal has finished closing, there should be no animations
42+
expect(currentAnimations.length).toBe(0);
43+
});
44+
});
45+
});

core/src/utils/overlays.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -577,6 +577,12 @@ export const dismiss = async <OverlayDismissOptions>(
577577
overlay.didDismiss.emit({ data, role });
578578
overlay.didDismissShorthand?.emit({ data, role });
579579

580+
// Get a reference to all animations currently assigned to this overlay
581+
// Then tear them down to return the overlay to its initial visual state
582+
const animations = activeAnimations.get(overlay) || [];
583+
584+
animations.forEach((ani) => ani.destroy());
585+
580586
activeAnimations.delete(overlay);
581587

582588
/**

0 commit comments

Comments
 (0)