From 1320948b245be3defe8610b9f049e781a4903a6e Mon Sep 17 00:00:00 2001
From: Amanda Johnston <90629384+amandaejohnston@users.noreply.github.com>
Date: Wed, 26 Oct 2022 13:59:43 -0500
Subject: [PATCH] fix(modal, popover): remove trigger click listeners when
 overlay is unmounted (#26167)

---
 core/src/components/modal/modal.tsx           | 13 +++++++++--
 .../modal/test/trigger/modal.e2e.ts           | 23 +++++++++++++++++--
 core/src/components/popover/popover.tsx       | 15 +++++++++---
 .../popover/test/trigger/popover.e2e.ts       | 18 ++++++++++++++-
 4 files changed, 61 insertions(+), 8 deletions(-)

diff --git a/core/src/components/modal/modal.tsx b/core/src/components/modal/modal.tsx
index 5ad51aefb3..b4500948ed 100644
--- a/core/src/components/modal/modal.tsx
+++ b/core/src/components/modal/modal.tsx
@@ -332,7 +332,17 @@ export class Modal implements ComponentInterface, OverlayInterface {
   }
 
   connectedCallback() {
-    prepareOverlay(this.el);
+    const { configureTriggerInteraction, el } = this;
+    prepareOverlay(el);
+    configureTriggerInteraction();
+  }
+
+  disconnectedCallback() {
+    const { destroyTriggerInteraction } = this;
+
+    if (destroyTriggerInteraction) {
+      destroyTriggerInteraction();
+    }
   }
 
   componentWillLoad() {
@@ -371,7 +381,6 @@ export class Modal implements ComponentInterface, OverlayInterface {
       raf(() => this.present());
     }
     this.breakpointsChanged(this.breakpoints);
-    this.configureTriggerInteraction();
   }
 
   private configureTriggerInteraction = () => {
diff --git a/core/src/components/modal/test/trigger/modal.e2e.ts b/core/src/components/modal/test/trigger/modal.e2e.ts
index 492d85098d..adcc76dc41 100644
--- a/core/src/components/modal/test/trigger/modal.e2e.ts
+++ b/core/src/components/modal/test/trigger/modal.e2e.ts
@@ -2,15 +2,34 @@ import { expect } from '@playwright/test';
 import { test } from '@utils/test/playwright';
 
 test.describe('modal: trigger', () => {
-  test('should open modal by left clicking on trigger', async ({ page }) => {
+  test.beforeEach(async ({ page, skip }) => {
+    skip.rtl();
+    skip.mode('ios');
     await page.goto('/src/components/modal/test/trigger');
+  });
+
+  test('should open modal by left clicking on trigger', async ({ page }) => {
     const ionModalDidPresent = await page.spyOnEvent('ionModalDidPresent');
 
     await page.click('#left-click-trigger');
 
     await ionModalDidPresent.next();
 
-    const modal = await page.locator('.left-click-modal');
+    const modal = page.locator('.left-click-modal');
+    await expect(modal).toBeVisible();
+  });
+
+  test('should still open modal when it has been removed and re-added to DOM', async ({ page }) => {
+    const button = page.locator('#left-click-trigger');
+    const modal = page.locator('ion-modal');
+
+    await modal.evaluate((modal: HTMLIonModalElement) => {
+      modal.remove();
+      document.querySelector('ion-button')?.insertAdjacentElement('afterend', modal);
+    });
+    await page.waitForChanges();
+
+    await button.click();
     await expect(modal).toBeVisible();
   });
 });
diff --git a/core/src/components/popover/popover.tsx b/core/src/components/popover/popover.tsx
index e9d6bacfc9..79c0ba9c34 100644
--- a/core/src/components/popover/popover.tsx
+++ b/core/src/components/popover/popover.tsx
@@ -311,7 +311,18 @@ export class Popover implements ComponentInterface, PopoverInterface {
   @Event({ eventName: 'didDismiss' }) didDismissShorthand!: EventEmitter<OverlayEventDetail>;
 
   connectedCallback() {
-    prepareOverlay(this.el);
+    const { configureTriggerInteraction, el } = this;
+
+    prepareOverlay(el);
+    configureTriggerInteraction();
+  }
+
+  disconnectedCallback() {
+    const { destroyTriggerInteraction } = this;
+
+    if (destroyTriggerInteraction) {
+      destroyTriggerInteraction();
+    }
   }
 
   componentWillLoad() {
@@ -344,8 +355,6 @@ export class Popover implements ComponentInterface, PopoverInterface {
         this.dismiss(undefined, undefined, false);
       });
     }
-
-    this.configureTriggerInteraction();
   }
 
   /**
diff --git a/core/src/components/popover/test/trigger/popover.e2e.ts b/core/src/components/popover/test/trigger/popover.e2e.ts
index 2f691f1841..b9a4c232a0 100644
--- a/core/src/components/popover/test/trigger/popover.e2e.ts
+++ b/core/src/components/popover/test/trigger/popover.e2e.ts
@@ -5,7 +5,9 @@ import { test } from '@utils/test/playwright';
 import { openPopover } from '../test.utils';
 
 test.describe('popover: trigger', async () => {
-  test.beforeEach(async ({ page }) => {
+  test.beforeEach(async ({ page, skip }) => {
+    skip.rtl();
+    skip.mode('ios');
     await page.goto('/src/components/popover/test/trigger');
   });
 
@@ -32,6 +34,20 @@ test.describe('popover: trigger', async () => {
 
     await checkPopover(page, '.hover-popover');
   });
+
+  test('should still open popover when it has been removed and re-added to DOM', async ({ page }) => {
+    const button = page.locator('#left-click-trigger');
+    const popover = page.locator('.left-click-popover');
+
+    await popover.evaluate((popover: HTMLIonPopoverElement) => {
+      popover.remove();
+      document.querySelector('ion-button')?.insertAdjacentElement('afterend', popover);
+    });
+    await page.waitForChanges();
+
+    await button.click();
+    await expect(popover).toBeVisible();
+  });
 });
 
 const checkPopover = async (page: E2EPage, popoverSelector: string) => {