Skip to content

Commit f8d32fe

Browse files
crisbetoandrewseguin
authored andcommitted
fix(bottom-sheet): DOM nodes not cleaned up if view container is destroyed mid-animation (#16349)
Along the same lines as #16309. Currently the bottom sheet's cleanup logic is tied to its exit animation completing. This usually isn't a problem since by default the bottom sheet 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.
1 parent d4ab49a commit f8d32fe

File tree

2 files changed

+28
-1
lines changed

2 files changed

+28
-1
lines changed

src/material/bottom-sheet/bottom-sheet-ref.ts

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,9 @@ export class MatBottomSheetRef<T = any, R = any> {
3939
/** Result to be passed down to the `afterDismissed` stream. */
4040
private _result: R | undefined;
4141

42+
/** Handle to the timeout that's running as a fallback in case the exit animation doesn't fire. */
43+
private _closeFallbackTimeout: number;
44+
4245
constructor(
4346
containerInstance: MatBottomSheetContainer,
4447
private _overlayRef: OverlayRef,
@@ -61,6 +64,7 @@ export class MatBottomSheetRef<T = any, R = any> {
6164
containerInstance._animationStateChanged
6265
.pipe(filter(event => event.phaseName === 'done' && event.toState === 'hidden'), take(1))
6366
.subscribe(() => {
67+
clearTimeout(this._closeFallbackTimeout);
6468
_overlayRef.dispose();
6569
});
6670

@@ -91,7 +95,18 @@ export class MatBottomSheetRef<T = any, R = any> {
9195
this.containerInstance._animationStateChanged.pipe(
9296
filter(event => event.phaseName === 'start'),
9397
take(1)
94-
).subscribe(() => this._overlayRef.detachBackdrop());
98+
).subscribe(event => {
99+
// The logic that disposes of the overlay depends on the exit animation completing, however
100+
// it isn't guaranteed if the parent view is destroyed while it's running. Add a fallback
101+
// timeout which will clean everything up if the animation hasn't fired within the specified
102+
// amount of time plus 100ms. We don't need to run this outside the NgZone, because for the
103+
// vast majority of cases the timeout will have been cleared before it has fired.
104+
this._closeFallbackTimeout = setTimeout(() => {
105+
this._overlayRef.dispose();
106+
}, event.totalTime + 100);
107+
108+
this._overlayRef.detachBackdrop();
109+
});
95110

96111
this._result = result;
97112
this.containerInstance.exit();

src/material/bottom-sheet/bottom-sheet.spec.ts

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -192,6 +192,18 @@ describe('MatBottomSheet', () => {
192192
expect(overlayContainerElement.querySelector('mat-bottom-sheet-container')).toBeFalsy();
193193
}));
194194

195+
it('should dispose of bottom sheet if view container is destroyed while animating',
196+
fakeAsync(() => {
197+
const bottomSheetRef = bottomSheet.open(PizzaMsg, {viewContainerRef: testViewContainerRef});
198+
199+
bottomSheetRef.dismiss();
200+
viewContainerFixture.detectChanges();
201+
viewContainerFixture.destroy();
202+
flush();
203+
204+
expect(overlayContainerElement.querySelector('mat-dialog-container')).toBeNull();
205+
}));
206+
195207
it('should emit the backdropClick stream when clicking on the overlay backdrop', fakeAsync(() => {
196208
const bottomSheetRef = bottomSheet.open(PizzaMsg, {viewContainerRef: testViewContainerRef});
197209
const spy = jasmine.createSpy('backdropClick spy');

0 commit comments

Comments
 (0)