fix(modal, popover): remove trigger click listeners when overlay is u…

…nmounted (#26167)
ionic-team · Oct 26, 2022 · 1320948 · 1320948
1 parent a7e15ba
commit 1320948
Show file tree

Hide file tree

Showing 4 changed files with 61 additions and 8 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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) => {