Skip to content

Commit 3b60a1d

Browse files
brandyscarneybrandyscarney
authored
fix(modal): dismiss top-most overlay when multiple IDs match (#30883)
Issue number: resolves #30030 --------- ## What is the current behavior? When modals are presented one after another with matching IDs and then dismissed by ID it will dismiss the first presented modal. ## What is the new behavior? - When modals are presented one after another with matching IDs and then dismissed by ID it will dismiss the last (top-most) presented modal. - Added e2e tests to verify this behavior works the same as the default dismiss (not passing an ID). ## Does this introduce a breaking change? - [ ] Yes - [x] No ## Other information [Modal: Dismiss Behavior](https://ionic-framework-git-fw-7016-ionic1.vercel.app/src/components/modal/test/dismiss-behavior) --------- Co-authored-by: Brandy Smith <[email protected]>
1 parent 8573bf8 commit 3b60a1d

File tree

4 files changed

+176
-2
lines changed

4 files changed

+176
-2
lines changed

‎core/src/components/modal/test/dismiss-behavior/index.html‎

Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,97 @@
1+
<!DOCTYPE html>
2+
<html lang="en" dir="ltr">
3+
<head>
4+
<meta charset="UTF-8" />
5+
<title>Modal - Dismiss Behavior</title>
6+
<meta
7+
name="viewport"
8+
content="width=device-width, initial-scale=1.0, minimum-scale=1.0, maximum-scale=1.0, user-scalable=no"
9+
/>
10+
<link href="../../../../../css/ionic.bundle.css" rel="stylesheet" />
11+
<link href="../../../../../scripts/testing/styles.css" rel="stylesheet" />
12+
<script src="../../../../../scripts/testing/scripts.js"></script>
13+
<script nomodule src="../../../../../dist/ionic/ionic.js"></script>
14+
<script type="module" src="../../../../../dist/ionic/ionic.esm.js"></script>
15+
</head>
16+
17+
<body>
18+
<ion-app>
19+
<div class="ion-page">
20+
<ion-header>
21+
<ion-toolbar>
22+
<ion-title>Modal - Dismiss Behavior</ion-title>
23+
</ion-toolbar>
24+
</ion-header>
25+
26+
<ion-content class="ion-padding">
27+
<button id="present-first-modal" onclick="presentFirstModal()">Present Modal</button>
28+
</ion-content>
29+
</div>
30+
</ion-app>
31+
32+
<script type="module">
33+
import { modalController } from '../../../../../dist/ionic/index.esm.js';
34+
35+
window.modalController = modalController;
36+
37+
const sharedId = 'shared-modal-id';
38+
const maxModals = 5;
39+
let modalCount = 0;
40+
41+
function createModalComponent(modalNumber) {
42+
const element = document.createElement('div');
43+
const canPresentNext = modalNumber < maxModals;
44+
const presentNextButton = canPresentNext
45+
? `<ion-button id="present-next-modal" onclick="presentNextModal(${modalNumber + 1})">Present Modal ${
46+
modalNumber + 1
47+
}</ion-button>`
48+
: '';
49+
element.innerHTML = `
50+
<ion-header>
51+
<ion-toolbar>
52+
<ion-title>Modal ${modalNumber}</ion-title>
53+
</ion-toolbar>
54+
</ion-header>
55+
<ion-content class="ion-padding">
56+
<p>This is modal number ${modalNumber}</p>
57+
${presentNextButton}
58+
<ion-button class="dismiss-by-id">Dismiss By ID</ion-button>
59+
<ion-button class="dismiss-default">Dismiss Default</ion-button>
60+
</ion-content>
61+
`;
62+
return element;
63+
}
64+
65+
window.presentFirstModal = async () => {
66+
modalCount = 0;
67+
await presentNextModal(1);
68+
};
69+
70+
window.presentNextModal = async (modalNumber) => {
71+
if (modalNumber > maxModals) {
72+
return;
73+
}
74+
modalCount = Math.max(modalCount, modalNumber);
75+
const element = createModalComponent(modalNumber);
76+
const modal = await modalController.create({
77+
component: element,
78+
htmlAttributes: {
79+
id: sharedId,
80+
'data-testid': `modal-${modalNumber}`,
81+
},
82+
});
83+
await modal.present();
84+
85+
const dismissByIdButton = element.querySelector('ion-button.dismiss-by-id');
86+
dismissByIdButton.addEventListener('click', () => {
87+
modalController.dismiss(undefined, undefined, sharedId);
88+
});
89+
90+
const dismissDefaultButton = element.querySelector('ion-button.dismiss-default');
91+
dismissDefaultButton.addEventListener('click', () => {
92+
modalController.dismiss();
93+
});
94+
};
95+
</script>
96+
</body>
97+
</html>

‎core/src/components/modal/test/dismiss-behavior/modal.e2e.ts‎

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
import { expect } from '@playwright/test';
2+
import { configs, test } from '@utils/test/playwright';
3+
4+
configs({ modes: ['ios'], directions: ['ltr'] }).forEach(({ title, config }) => {
5+
test.describe(title('modal: dismiss behavior'), () => {
6+
test.describe(title('modal: default dismiss'), () => {
7+
test('should dismiss the last presented modal when the default dismiss button is clicked', async ({ page }) => {
8+
await page.goto('/src/components/modal/test/dismiss-behavior', config);
9+
10+
const ionModalDidPresent = await page.spyOnEvent('ionModalDidPresent');
11+
const ionModalDidDismiss = await page.spyOnEvent('ionModalDidDismiss');
12+
13+
await page.click('#present-first-modal');
14+
await ionModalDidPresent.next();
15+
const firstModal = page.locator('ion-modal[data-testid="modal-1"]');
16+
await expect(firstModal).toBeVisible();
17+
18+
await page.click('#present-next-modal');
19+
await ionModalDidPresent.next();
20+
const secondModal = page.locator('ion-modal[data-testid="modal-2"]');
21+
await expect(secondModal).toBeVisible();
22+
23+
await page.click('ion-modal[data-testid="modal-2"] ion-button.dismiss-default');
24+
await ionModalDidDismiss.next();
25+
await secondModal.waitFor({ state: 'detached' });
26+
27+
await expect(firstModal).toBeVisible();
28+
await expect(secondModal).toBeHidden();
29+
});
30+
});
31+
32+
test.describe(title('modal: dismiss by id'), () => {
33+
test('should dismiss the last presented modal when the dismiss by id button is clicked', async ({ page }) => {
34+
await page.goto('/src/components/modal/test/dismiss-behavior', config);
35+
36+
const ionModalDidPresent = await page.spyOnEvent('ionModalDidPresent');
37+
const ionModalDidDismiss = await page.spyOnEvent('ionModalDidDismiss');
38+
39+
await page.click('#present-first-modal');
40+
await ionModalDidPresent.next();
41+
const firstModal = page.locator('ion-modal[data-testid="modal-1"]');
42+
await expect(firstModal).toBeVisible();
43+
44+
await page.click('#present-next-modal');
45+
await ionModalDidPresent.next();
46+
const secondModal = page.locator('ion-modal[data-testid="modal-2"]');
47+
await expect(secondModal).toBeVisible();
48+
49+
await page.click('ion-modal[data-testid="modal-2"] ion-button.dismiss-by-id');
50+
await ionModalDidDismiss.next();
51+
await secondModal.waitFor({ state: 'detached' });
52+
53+
await expect(firstModal).toBeVisible();
54+
await expect(secondModal).toBeHidden();
55+
});
56+
});
57+
});
58+
});

‎core/src/components/modal/test/modal-id.spec.tsx‎

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
1+
import { h } from '@stencil/core';
12
import { newSpecPage } from '@stencil/core/testing';
23

34
import { Modal } from '../modal';
4-
import { h } from '@stencil/core';
55

66
describe('modal: id', () => {
77
it('modal should be assigned an incrementing id', async () => {
@@ -52,4 +52,21 @@ describe('modal: id', () => {
5252
const alert = page.body.querySelector('ion-modal')!;
5353
expect(alert.id).toBe(id);
5454
});
55+
56+
it('should allow multiple modals with the same id', async () => {
57+
const sharedId = 'shared-modal-id';
58+
59+
const page = await newSpecPage({
60+
components: [Modal],
61+
template: () => [
62+
<ion-modal id={sharedId} overlayIndex={1} is-open={true}></ion-modal>,
63+
<ion-modal id={sharedId} overlayIndex={2} is-open={true}></ion-modal>,
64+
],
65+
});
66+
67+
const modals = page.body.querySelectorAll('ion-modal');
68+
expect(modals.length).toBe(2);
69+
expect(modals[0].id).toBe(sharedId);
70+
expect(modals[1].id).toBe(sharedId);
71+
});
5572
});

‎core/src/utils/overlays.ts‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -473,7 +473,9 @@ export const getPresentedOverlay = (
473473
id?: string
474474
): HTMLIonOverlayElement | undefined => {
475475
const overlays = getPresentedOverlays(doc, overlayTag);
476-
return id === undefined ? overlays[overlays.length - 1] : overlays.find((o) => o.id === id);
476+
// If no id is provided, return the last presented overlay
477+
// Otherwise, return the last overlay with the given id
478+
return (id === undefined ? overlays : overlays.filter((o: HTMLIonOverlayElement) => o.id === id)).slice(-1)[0];
477479
};
478480

479481
/**

0 commit comments

Comments
 (0)