mirror of
https://github.com/ionic-team/ionic-framework.git
synced 2025-11-08 07:41:51 +08:00
fix(modal): dismiss modal when parent element is removed from DOM (#30544)
Issue number: resolves #30389 --------- <!-- 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. --> Currently, when the element an ion-modal was presented from is removed, the modal stays presented and can be broken depending on the framework. This is unlike #30540, where children of open modals were being kept open. In this case, specifically the DOM element is being removed for whatever reason and the modal is staying open. ## What is the new behavior? <!-- Please describe the behavior or changes that are being added by this PR. --> We're now identifying our parent component on load and watching it with a mutation observer to determine if it gets removed from the DOM. If it does, we trigger a dismiss. This, conveniently, works nicely with #30540 and will dismiss all children and grandchildren as well. ## 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/docs/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. --> The issue this resolves was already marked closed, but on closer inspection I determined that was a mistake. I believed this issue was related to another one I was dealing with and it is, but it wasn't quite the same. After this issue is merged, I believe we will have handled all avenues of possibly ending up with broken modals because of parent elements or modals being removed. [Relevant Test Page](https://ionic-framework-git-fix-remove-modal-when-parent-removed-ionic1.vercel.app/src/components/modal/test/inline) **Current dev build:** ``` 8.6.5-dev.11752329407.10f7fc80 ```
This commit is contained in:
@ -96,6 +96,11 @@ export class Modal implements ComponentInterface, OverlayInterface {
|
||||
private viewTransitionAnimation?: Animation;
|
||||
private resizeTimeout?: any;
|
||||
|
||||
// Mutation observer to watch for parent removal
|
||||
private parentRemovalObserver?: MutationObserver;
|
||||
// Cached original parent from before modal is moved to body during presentation
|
||||
private cachedOriginalParent?: HTMLElement;
|
||||
|
||||
lastFocus?: HTMLElement;
|
||||
animation?: Animation;
|
||||
|
||||
@ -398,6 +403,7 @@ export class Modal implements ComponentInterface, OverlayInterface {
|
||||
disconnectedCallback() {
|
||||
this.triggerController.removeClickListener();
|
||||
this.cleanupViewTransitionListener();
|
||||
this.cleanupParentRemovalObserver();
|
||||
}
|
||||
|
||||
componentWillLoad() {
|
||||
@ -407,6 +413,11 @@ export class Modal implements ComponentInterface, OverlayInterface {
|
||||
const attributesToInherit = ['aria-label', 'role'];
|
||||
this.inheritedAttributes = inheritAttributes(el, attributesToInherit);
|
||||
|
||||
// Cache original parent before modal gets moved to body during presentation
|
||||
if (el.parentNode) {
|
||||
this.cachedOriginalParent = el.parentNode as HTMLElement;
|
||||
}
|
||||
|
||||
/**
|
||||
* When using a controller modal you can set attributes
|
||||
* using the htmlAttributes property. Since the above attributes
|
||||
@ -642,6 +653,9 @@ export class Modal implements ComponentInterface, OverlayInterface {
|
||||
// Initialize view transition listener for iOS card modals
|
||||
this.initViewTransitionListener();
|
||||
|
||||
// Initialize parent removal observer
|
||||
this.initParentRemovalObserver();
|
||||
|
||||
unlock();
|
||||
}
|
||||
|
||||
@ -847,6 +861,7 @@ export class Modal implements ComponentInterface, OverlayInterface {
|
||||
this.gesture.destroy();
|
||||
}
|
||||
this.cleanupViewTransitionListener();
|
||||
this.cleanupParentRemovalObserver();
|
||||
}
|
||||
this.currentBreakpoint = undefined;
|
||||
this.animation = undefined;
|
||||
@ -1150,6 +1165,61 @@ export class Modal implements ComponentInterface, OverlayInterface {
|
||||
});
|
||||
}
|
||||
|
||||
private initParentRemovalObserver() {
|
||||
if (typeof MutationObserver === 'undefined') {
|
||||
return;
|
||||
}
|
||||
|
||||
// Only observe if we have a cached parent and are in browser environment
|
||||
if (typeof window === 'undefined' || !this.cachedOriginalParent) {
|
||||
return;
|
||||
}
|
||||
|
||||
// Don't observe document or fragment nodes as they can't be "removed"
|
||||
if (
|
||||
this.cachedOriginalParent.nodeType === Node.DOCUMENT_NODE ||
|
||||
this.cachedOriginalParent.nodeType === Node.DOCUMENT_FRAGMENT_NODE
|
||||
) {
|
||||
return;
|
||||
}
|
||||
|
||||
this.parentRemovalObserver = new MutationObserver((mutations) => {
|
||||
mutations.forEach((mutation) => {
|
||||
if (mutation.type === 'childList' && mutation.removedNodes.length > 0) {
|
||||
// Check if our cached original parent was removed
|
||||
const cachedParentWasRemoved = Array.from(mutation.removedNodes).some((node) => {
|
||||
const isDirectMatch = node === this.cachedOriginalParent;
|
||||
const isContainedMatch = this.cachedOriginalParent
|
||||
? (node as HTMLElement).contains?.(this.cachedOriginalParent)
|
||||
: false;
|
||||
return isDirectMatch || isContainedMatch;
|
||||
});
|
||||
|
||||
// Also check if parent is no longer connected to DOM
|
||||
const cachedParentDisconnected = this.cachedOriginalParent && !this.cachedOriginalParent.isConnected;
|
||||
|
||||
if (cachedParentWasRemoved || cachedParentDisconnected) {
|
||||
this.dismiss(undefined, 'parent-removed');
|
||||
// Release the reference to the cached original parent
|
||||
// so we don't have a memory leak
|
||||
this.cachedOriginalParent = undefined;
|
||||
}
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
// Observe document body with subtree to catch removals at any level
|
||||
this.parentRemovalObserver.observe(document.body, {
|
||||
childList: true,
|
||||
subtree: true,
|
||||
});
|
||||
}
|
||||
|
||||
private cleanupParentRemovalObserver() {
|
||||
this.parentRemovalObserver?.disconnect();
|
||||
this.parentRemovalObserver = undefined;
|
||||
}
|
||||
|
||||
render() {
|
||||
const {
|
||||
handle,
|
||||
|
||||
@ -22,9 +22,8 @@
|
||||
</ion-header>
|
||||
|
||||
<ion-content class="ion-padding">
|
||||
<button id="open-inline-modal" onclick="openModal(event)">Open Modal</button>
|
||||
|
||||
<div id="modal-container">
|
||||
<button id="open-inline-modal" onclick="openModal(event)">Open Modal</button>
|
||||
<ion-modal swipe-to-close="true">
|
||||
<ion-header>
|
||||
<ion-toolbar>
|
||||
@ -34,6 +33,9 @@
|
||||
<ion-content class="ion-padding">
|
||||
<p>This is my inline modal content!</p>
|
||||
<button id="open-child-modal" onclick="openChildModal(event)">Open Child Modal</button>
|
||||
<button id="remove-modal-container" onclick="removeModalContainer(event)">
|
||||
Remove Modal Container
|
||||
</button>
|
||||
|
||||
<ion-modal id="child-modal" swipe-to-close="true">
|
||||
<ion-header>
|
||||
@ -46,6 +48,9 @@
|
||||
<p>When the parent modal is dismissed, this child modal should also be dismissed automatically.</p>
|
||||
<button id="dismiss-parent" onclick="dismissParent(event)">Dismiss Parent Modal</button>
|
||||
<button id="dismiss-child" onclick="dismissChild(event)">Dismiss Child Modal</button>
|
||||
<button id="child-remove-modal-container" onclick="removeModalContainer(event)">
|
||||
Remove Modal Container
|
||||
</button>
|
||||
</ion-content>
|
||||
</ion-modal>
|
||||
</ion-content>
|
||||
@ -78,6 +83,14 @@
|
||||
childModal.isOpen = false;
|
||||
};
|
||||
|
||||
const removeModalContainer = () => {
|
||||
const container = document.querySelector('#modal-container');
|
||||
if (container) {
|
||||
container.remove();
|
||||
console.log('Modal container removed from DOM');
|
||||
}
|
||||
};
|
||||
|
||||
modal.addEventListener('didDismiss', () => {
|
||||
modal.isOpen = false;
|
||||
});
|
||||
|
||||
@ -122,5 +122,152 @@ configs({ modes: ['ios'], directions: ['ltr'] }).forEach(({ title, config }) =>
|
||||
await modal.evaluate((el: HTMLIonModalElement) => el.firstElementChild!.firstElementChild!.className)
|
||||
).not.toContain('ion-page');
|
||||
});
|
||||
|
||||
test('it should dismiss modal when parent container is removed from DOM', async ({ page }) => {
|
||||
await page.goto('/src/components/modal/test/inline', config);
|
||||
const ionModalDidPresent = await page.spyOnEvent('ionModalDidPresent');
|
||||
const ionModalDidDismiss = await page.spyOnEvent('ionModalDidDismiss');
|
||||
|
||||
const modal = page.locator('ion-modal').first();
|
||||
const modalContainer = page.locator('#modal-container');
|
||||
|
||||
// Open the modal
|
||||
await page.click('#open-inline-modal');
|
||||
await ionModalDidPresent.next();
|
||||
await expect(modal).toBeVisible();
|
||||
|
||||
// Remove the modal container from DOM
|
||||
await page.click('#remove-modal-container');
|
||||
|
||||
// Wait for modal to be dismissed
|
||||
const dismissEvent = await ionModalDidDismiss.next();
|
||||
|
||||
// Verify the modal was dismissed with the correct role
|
||||
expect(dismissEvent.detail.role).toBe('parent-removed');
|
||||
|
||||
// Verify the modal is no longer visible
|
||||
await expect(modal).toBeHidden();
|
||||
|
||||
// Verify the container was actually removed
|
||||
await expect(modalContainer).not.toBeAttached();
|
||||
});
|
||||
|
||||
test('it should dismiss both parent and child modals when parent container is removed from DOM', async ({
|
||||
page,
|
||||
}) => {
|
||||
await page.goto('/src/components/modal/test/inline', config);
|
||||
const ionModalDidPresent = await page.spyOnEvent('ionModalDidPresent');
|
||||
const ionModalDidDismiss = await page.spyOnEvent('ionModalDidDismiss');
|
||||
|
||||
const parentModal = page.locator('ion-modal').first();
|
||||
const childModal = page.locator('#child-modal');
|
||||
const modalContainer = page.locator('#modal-container');
|
||||
|
||||
// Open the parent modal
|
||||
await page.click('#open-inline-modal');
|
||||
await ionModalDidPresent.next();
|
||||
await expect(parentModal).toBeVisible();
|
||||
|
||||
// Open the child modal
|
||||
await page.click('#open-child-modal');
|
||||
await ionModalDidPresent.next();
|
||||
await expect(childModal).toBeVisible();
|
||||
|
||||
// Remove the modal container from DOM
|
||||
await page.click('#child-remove-modal-container');
|
||||
|
||||
// Wait for both modals to be dismissed
|
||||
const firstDismissEvent = await ionModalDidDismiss.next();
|
||||
const secondDismissEvent = await ionModalDidDismiss.next();
|
||||
|
||||
// Verify at least one modal was dismissed with 'parent-removed' role
|
||||
const dismissRoles = [firstDismissEvent.detail.role, secondDismissEvent.detail.role];
|
||||
expect(dismissRoles).toContain('parent-removed');
|
||||
|
||||
// Verify both modals are no longer visible
|
||||
await expect(parentModal).toBeHidden();
|
||||
await expect(childModal).toBeHidden();
|
||||
|
||||
// Verify the container was actually removed
|
||||
await expect(modalContainer).not.toBeAttached();
|
||||
});
|
||||
|
||||
test('it should dismiss modals when top-level ancestor is removed', async ({ page }) => {
|
||||
// We need to make sure we can close a modal when a much higher
|
||||
// element is removed from the DOM. This will be a common
|
||||
// use case in frameworks like Angular and React, where an entire
|
||||
// page container for much more than the modal might be swapped out.
|
||||
await page.setContent(
|
||||
`
|
||||
<ion-app>
|
||||
<div class="ion-page">
|
||||
<ion-header>
|
||||
<ion-toolbar>
|
||||
<ion-title>Top Level Removal Test</ion-title>
|
||||
</ion-toolbar>
|
||||
</ion-header>
|
||||
<ion-content class="ion-padding">
|
||||
<div id="top-level-container">
|
||||
<div id="nested-container">
|
||||
<button id="open-nested-modal">Open Nested Modal</button>
|
||||
<ion-modal id="nested-modal">
|
||||
<ion-header>
|
||||
<ion-toolbar>
|
||||
<ion-title>Nested Modal</ion-title>
|
||||
</ion-toolbar>
|
||||
</ion-header>
|
||||
<ion-content class="ion-padding">
|
||||
<p>This modal's original parent is deeply nested</p>
|
||||
<button id="remove-top-level">Remove Top Level Container</button>
|
||||
</ion-content>
|
||||
</ion-modal>
|
||||
</div>
|
||||
</div>
|
||||
</ion-content>
|
||||
</div>
|
||||
</ion-app>
|
||||
|
||||
<script>
|
||||
const nestedModal = document.querySelector('#nested-modal');
|
||||
nestedModal.presentingElement = document.querySelector('.ion-page');
|
||||
|
||||
document.getElementById('open-nested-modal').addEventListener('click', () => {
|
||||
nestedModal.isOpen = true;
|
||||
});
|
||||
|
||||
document.getElementById('remove-top-level').addEventListener('click', () => {
|
||||
document.querySelector('#top-level-container').remove();
|
||||
});
|
||||
</script>
|
||||
`,
|
||||
config
|
||||
);
|
||||
|
||||
const ionModalDidPresent = await page.spyOnEvent('ionModalDidPresent');
|
||||
const ionModalDidDismiss = await page.spyOnEvent('ionModalDidDismiss');
|
||||
|
||||
const nestedModal = page.locator('#nested-modal');
|
||||
const topLevelContainer = page.locator('#top-level-container');
|
||||
|
||||
// Open the nested modal
|
||||
await page.click('#open-nested-modal');
|
||||
await ionModalDidPresent.next();
|
||||
await expect(nestedModal).toBeVisible();
|
||||
|
||||
// Remove the top-level container
|
||||
await page.click('#remove-top-level');
|
||||
|
||||
// Wait for modal to be dismissed
|
||||
const dismissEvent = await ionModalDidDismiss.next();
|
||||
|
||||
// Verify the modal was dismissed with the correct role
|
||||
expect(dismissEvent.detail.role).toBe('parent-removed');
|
||||
|
||||
// Verify the modal is no longer visible
|
||||
await expect(nestedModal).toBeHidden();
|
||||
|
||||
// Verify the container was actually removed
|
||||
await expect(topLevelContainer).not.toBeAttached();
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user