Skip to content

Commit a9a266a

Browse files
crisbetoandrewseguin
authored andcommitted
fix(dialog): DOM nodes not cleaned up if view container is destroyed mid-animation (#16309)
Currently the dialog's cleanup logic is tied to its exit animation completing. This usually isn't a problem since by default the dialog is attached to the application ref, however if the consumer has set a `viewContainerRef` and that ref is destroyed mid-animation, the exit animation event will never fire. These changes add a timeout as a fallback in case the animation doesn't finish in the specified time. Fixes #16284.
1 parent f8d32fe commit a9a266a

File tree

2 files changed

+29
-2
lines changed

2 files changed

+29
-2
lines changed

src/material/dialog/dialog-ref.ts

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,9 @@ export class MatDialogRef<T, R = any> {
4242
/** Result to be passed to afterClosed. */
4343
private _result: R | undefined;
4444

45+
/** Handle to the timeout that's running as a fallback in case the exit animation doesn't fire. */
46+
private _closeFallbackTimeout: number;
47+
4548
constructor(
4649
private _overlayRef: OverlayRef,
4750
public _containerInstance: MatDialogContainer,
@@ -66,7 +69,10 @@ export class MatDialogRef<T, R = any> {
6669
_containerInstance._animationStateChanged.pipe(
6770
filter(event => event.phaseName === 'done' && event.toState === 'exit'),
6871
take(1)
69-
).subscribe(() => this._overlayRef.dispose());
72+
).subscribe(() => {
73+
clearTimeout(this._closeFallbackTimeout);
74+
this._overlayRef.dispose();
75+
});
7076

7177
_overlayRef.detachments().subscribe(() => {
7278
this._beforeClosed.next(this._result);
@@ -99,10 +105,19 @@ export class MatDialogRef<T, R = any> {
99105
filter(event => event.phaseName === 'start'),
100106
take(1)
101107
)
102-
.subscribe(() => {
108+
.subscribe(event => {
103109
this._beforeClosed.next(dialogResult);
104110
this._beforeClosed.complete();
105111
this._overlayRef.detachBackdrop();
112+
113+
// The logic that disposes of the overlay depends on the exit animation completing, however
114+
// it isn't guaranteed if the parent view is destroyed while it's running. Add a fallback
115+
// timeout which will clean everything up if the animation hasn't fired within the specified
116+
// amount of time plus 100ms. We don't need to run this outside the NgZone, because for the
117+
// vast majority of cases the timeout will have been cleared before it has the chance to fire.
118+
this._closeFallbackTimeout = setTimeout(() => {
119+
this._overlayRef.dispose();
120+
}, event.totalTime + 100);
106121
});
107122

108123
this._containerInstance._startExitAnimation();

src/material/dialog/dialog.spec.ts

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -194,6 +194,17 @@ describe('MatDialog', () => {
194194
expect(overlayContainerElement.querySelector('mat-dialog-container')).toBeNull();
195195
}));
196196

197+
it('should dispose of dialog if view container is destroyed while animating', fakeAsync(() => {
198+
const dialogRef = dialog.open(PizzaMsg, {viewContainerRef: testViewContainerRef});
199+
200+
dialogRef.close();
201+
viewContainerFixture.detectChanges();
202+
viewContainerFixture.destroy();
203+
flush();
204+
205+
expect(overlayContainerElement.querySelector('mat-dialog-container')).toBeNull();
206+
}));
207+
197208
it('should dispatch the beforeClose and afterClose events when the ' +
198209
'overlay is detached externally', fakeAsync(inject([Overlay], (overlay: Overlay) => {
199210
const dialogRef = dialog.open(PizzaMsg, {
@@ -1064,6 +1075,7 @@ describe('MatDialog', () => {
10641075

10651076
document.body.removeChild(button);
10661077
document.body.removeChild(input);
1078+
flush();
10671079
}));
10681080

10691081
it('should move focus to the container if there are no focusable elements in the dialog',

0 commit comments

Comments
 (0)