From 950fa40c5597c81d5cbaeb9276b09adfea5e79fb Mon Sep 17 00:00:00 2001
From: Shawn Taylor <shawn@ionic.io>
Date: Wed, 31 Jan 2024 10:14:57 -0500
Subject: [PATCH] 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>
---
 core/src/components/modal/modal.tsx           |  5 ---
 .../modal/test/animations/modal.e2e.ts        | 45 +++++++++++++++++++
 core/src/utils/overlays.ts                    |  6 +++
 3 files changed, 51 insertions(+), 5 deletions(-)
 create mode 100644 core/src/components/modal/test/animations/modal.e2e.ts

diff --git a/core/src/components/modal/modal.tsx b/core/src/components/modal/modal.tsx
index bae8648d765..d32e092b195 100644
--- a/core/src/components/modal/modal.tsx
+++ b/core/src/components/modal/modal.tsx
@@ -10,7 +10,6 @@ import { Style as StatusBarStyle, StatusBar } from '@utils/native/status-bar';
 import {
   GESTURE,
   BACKDROP,
-  activeAnimations,
   dismiss,
   eventMethod,
   prepareOverlay,
@@ -705,8 +704,6 @@ export class Modal implements ComponentInterface, OverlayInterface {
       this.keyboardOpenCallback = undefined;
     }
 
-    const enteringAnimation = activeAnimations.get(this) || [];
-
     const dismissed = await dismiss<ModalDismissOptions>(
       this,
       data,
@@ -733,8 +730,6 @@ export class Modal implements ComponentInterface, OverlayInterface {
       if (this.gesture) {
         this.gesture.destroy();
       }
-
-      enteringAnimation.forEach((ani) => ani.destroy());
     }
     this.currentBreakpoint = undefined;
     this.animation = undefined;
diff --git a/core/src/components/modal/test/animations/modal.e2e.ts b/core/src/components/modal/test/animations/modal.e2e.ts
new file mode 100644
index 00000000000..8d541003ff8
--- /dev/null
+++ b/core/src/components/modal/test/animations/modal.e2e.ts
@@ -0,0 +1,45 @@
+import { expect } from '@playwright/test';
+import { configs, test } from '@utils/test/playwright';
+
+configs({ modes: ['ios'], directions: ['ltr'] }).forEach(({ config, title }) => {
+  test.describe(title('modal: animations'), () => {
+    test.beforeEach(async ({ page }) => {
+      await page.setContent(
+        `
+        <ion-modal is-open="true" trigger="open-modal"></ion-modal>
+      `,
+        config
+      );
+    });
+    test('card modal should clean up animations on dismiss', async ({ page }, testInfo) => {
+      testInfo.annotations.push({
+        type: 'issue',
+        description: 'https://github.com/ionic-team/ionic-framework/issues/28352',
+      });
+
+      const ionModalDidDismiss = await page.spyOnEvent('ionModalDidDismiss');
+
+      const modal = page.locator('ion-modal');
+
+      const initialAnimations = await modal.evaluate((el: HTMLIonModalElement) => {
+        return el.shadowRoot!.getAnimations();
+      });
+
+      // While the modal is open, it should have animations
+      expect(initialAnimations.length).toBeGreaterThan(0);
+
+      await modal.evaluate((el: HTMLIonModalElement) => {
+        el.dismiss();
+      });
+
+      await ionModalDidDismiss.next();
+
+      const currentAnimations = await modal.evaluate((el: HTMLIonModalElement) => {
+        return el.shadowRoot!.getAnimations();
+      });
+
+      // Once the modal has finished closing, there should be no animations
+      expect(currentAnimations.length).toBe(0);
+    });
+  });
+});
diff --git a/core/src/utils/overlays.ts b/core/src/utils/overlays.ts
index bd56ac36844..bead295cd0b 100644
--- a/core/src/utils/overlays.ts
+++ b/core/src/utils/overlays.ts
@@ -577,6 +577,12 @@ export const dismiss = async <OverlayDismissOptions>(
     overlay.didDismiss.emit({ data, role });
     overlay.didDismissShorthand?.emit({ data, role });
 
+    // Get a reference to all animations currently assigned to this overlay
+    // Then tear them down to return the overlay to its initial visual state
+    const animations = activeAnimations.get(overlay) || [];
+
+    animations.forEach((ani) => ani.destroy());
+
     activeAnimations.delete(overlay);
 
     /**