fix(cdk/dialog): not emitting closed event on external detachments

Fixes that the CDK dialog wasn't emitting to the `closed` event when it is detached externally, e.g. by a scroll strategy or a navigation. We had unit tests for this on the Material side, but we had special logic to handle it there.

Fixes #26581.

Author: crisbeto

src/cdk/dialog/BUILD.bazel

@@ -45,6 +45,7 @@ ng_test_library(
         "//src/cdk/testing/private",
         "@npm//@angular/common",
         "@npm//@angular/platform-browser",
+        "@npm//rxjs",
     ],
 )
 

src/cdk/dialog/dialog-config.ts

@@ -132,6 +132,12 @@ export class DialogConfig<D = unknown, R = unknown, C extends BasePortalOutlet =
    */
   closeOnDestroy?: boolean = true;
 
+  /**
+   * Whether the dialog should close when the underlying overlay is disposed. This is useful if
+   * another service is wrapping the dialog and is managing the destruction instead.
+   */
+  closeOnOverlayDetachments?: boolean = true;
+
   /** Alternate `ComponentFactoryResolver` to use when resolving the associated component. */
   componentFactoryResolver?: ComponentFactoryResolver;
 

src/cdk/dialog/dialog-ref.ts

@@ -72,6 +72,13 @@ export class DialogRef<R = unknown, C = unknown> {
         this.close(undefined, {focusOrigin: 'mouse'});
       }
     });
+
+    overlayRef.detachments().subscribe(() => {
+      // Check specifically for `false`, because we want `undefined` to be treated like `true`.
+      if (config.closeOnOverlayDetachments !== false) {
+        this._finishClose();
+      }
+    });
   }
 
   /**
@@ -80,16 +87,8 @@ export class DialogRef<R = unknown, C = unknown> {
    * @param options Additional options to customize the closing behavior.
    */
   close(result?: R, options?: DialogCloseOptions): void {
-    if (this.containerInstance) {
-      const closedSubject = this.closed as Subject<R | undefined>;
-      this.containerInstance._closeInteractionType = options?.focusOrigin || 'program';
-      this.overlayRef.dispose();
-      closedSubject.next(result);
-      closedSubject.complete();
-      (this as {componentInstance: C}).componentInstance = (
-        this as {containerInstance: BasePortalOutlet}
-      ).containerInstance = null!;
-    }
+    this._finishClose(result, options);
+    this.overlayRef.dispose();
   }
 
   /** Updates the position of the dialog based on the current position strategy. */
@@ -119,4 +118,17 @@ export class DialogRef<R = unknown, C = unknown> {
     this.overlayRef.removePanelClass(classes);
     return this;
   }
+
+  /** Finalizes the closing sequence once the overlay is detached. */
+  private _finishClose(result?: R, options?: DialogCloseOptions) {
+    if (this.containerInstance) {
+      const closedSubject = this.closed as Subject<R | undefined>;
+      this.containerInstance._closeInteractionType = options?.focusOrigin || 'program';
+      closedSubject.next(result);
+      closedSubject.complete();
+      (this as {componentInstance: C}).componentInstance = (
+        this as {containerInstance: BasePortalOutlet}
+      ).containerInstance = null!;
+    }
+  }
 }

src/cdk/dialog/dialog.spec.ts

@@ -23,14 +23,15 @@ import {By} from '@angular/platform-browser';
 import {Location} from '@angular/common';
 import {SpyLocation} from '@angular/common/testing';
 import {Directionality} from '@angular/cdk/bidi';
-import {Overlay, OverlayContainer} from '@angular/cdk/overlay';
+import {Overlay, OverlayContainer, ScrollDispatcher} from '@angular/cdk/overlay';
 import {A, ESCAPE} from '@angular/cdk/keycodes';
 import {_supportsShadowDom} from '@angular/cdk/platform';
 import {
   dispatchKeyboardEvent,
   createKeyboardEvent,
   dispatchEvent,
 } from '@angular/cdk/testing/private';
+import {Subject} from 'rxjs';
 import {DIALOG_DATA, Dialog, DialogModule, DialogRef} from './index';
 
 describe('Dialog', () => {
@@ -41,6 +42,7 @@ describe('Dialog', () => {
   let viewContainerFixture: ComponentFixture<ComponentWithChildViewContainer>;
   let mockLocation: SpyLocation;
   let overlay: Overlay;
+  let scrolledSubject = new Subject();
 
   beforeEach(fakeAsync(() => {
     TestBed.configureTestingModule({
@@ -59,6 +61,10 @@ describe('Dialog', () => {
       providers: [
         {provide: Location, useClass: SpyLocation},
         {provide: TEMPLATE_INJECTOR_TEST_TOKEN, useValue: 'hello from test module'},
+        {
+          provide: ScrollDispatcher,
+          useFactory: () => ({scrolled: () => scrolledSubject}),
+        },
       ],
     });
 
@@ -504,24 +510,28 @@ describe('Dialog', () => {
   }));
 
   it('should close all dialogs when the user goes forwards/backwards in history', fakeAsync(() => {
-    dialog.open(PizzaMsg);
+    const closeSpy = jasmine.createSpy('closed');
+    dialog.open(PizzaMsg).closed.subscribe(closeSpy);
     viewContainerFixture.detectChanges();
-    dialog.open(PizzaMsg);
+    dialog.open(PizzaMsg).closed.subscribe(closeSpy);
     viewContainerFixture.detectChanges();
 
     expect(overlayContainerElement.querySelectorAll('cdk-dialog-container').length).toBe(2);
+    expect(closeSpy).not.toHaveBeenCalled();
 
     mockLocation.simulateUrlPop('');
     viewContainerFixture.detectChanges();
     flush();
 
     expect(overlayContainerElement.querySelectorAll('cdk-dialog-container').length).toBe(0);
+    expect(closeSpy).toHaveBeenCalledTimes(2);
   }));
 
   it('should close all open dialogs when the location hash changes', fakeAsync(() => {
-    dialog.open(PizzaMsg);
+    const closeSpy = jasmine.createSpy('closed');
+    dialog.open(PizzaMsg).closed.subscribe(closeSpy);
     viewContainerFixture.detectChanges();
-    dialog.open(PizzaMsg);
+    dialog.open(PizzaMsg).closed.subscribe(closeSpy);
     viewContainerFixture.detectChanges();
 
     expect(overlayContainerElement.querySelectorAll('cdk-dialog-container').length).toBe(2);
@@ -533,6 +543,28 @@ describe('Dialog', () => {
     expect(overlayContainerElement.querySelectorAll('cdk-dialog-container').length).toBe(0);
   }));
 
+  it('should close the dialog when detached externally', fakeAsync(() => {
+    const closeSpy = jasmine.createSpy('closed');
+    dialog
+      .open(PizzaMsg, {scrollStrategy: overlay.scrollStrategies.close()})
+      .closed.subscribe(closeSpy);
+    viewContainerFixture.detectChanges();
+    dialog
+      .open(PizzaMsg, {scrollStrategy: overlay.scrollStrategies.close()})
+      .closed.subscribe(closeSpy);
+    viewContainerFixture.detectChanges();
+
+    expect(overlayContainerElement.querySelectorAll('cdk-dialog-container').length).toBe(2);
+    expect(closeSpy).not.toHaveBeenCalled();
+
+    scrolledSubject.next();
+    viewContainerFixture.detectChanges();
+    flush();
+
+    expect(overlayContainerElement.querySelectorAll('cdk-dialog-container').length).toBe(0);
+    expect(closeSpy).toHaveBeenCalledTimes(2);
+  }));
+
   it('should have the componentInstance available in the afterClosed callback', fakeAsync(() => {
     let dialogRef = dialog.open(PizzaMsg);
     let spy = jasmine.createSpy('afterClosed spy');

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

@@ -95,6 +95,8 @@ export class MatBottomSheet implements OnDestroy {
       ..._config,
       // Disable closing since we need to sync it up to the animation ourselves.
       disableClose: true,
+      // Disable closing on detachments so that we can sync up the animation.
+      closeOnOverlayDetachments: false,
       maxWidth: '100%',
       container: MatBottomSheetContainer,
       scrollStrategy: _config.scrollStrategy || this._overlay.scrollStrategies.block(),

src/material/dialog/dialog.ts

@@ -170,6 +170,8 @@ export abstract class _MatDialogBase<C extends _MatDialogContainerBase> implemen
       // We want to do the cleanup here, rather than the CDK service, because the CDK destroys
       // the dialogs immediately whereas we want it to wait for the animations to finish.
       closeOnDestroy: false,
+      // Disable closing on detachments so that we can sync up the animation.
+      closeOnOverlayDetachments: false,
       container: {
         type: this._dialogContainerType,
         providers: () => [

tools/public_api_guard/cdk/dialog.md

@@ -127,6 +127,7 @@ export class DialogConfig<D = unknown, R = unknown, C extends BasePortalOutlet =
     backdropClass?: string | string[];
     closeOnDestroy?: boolean;
     closeOnNavigation?: boolean;
+    closeOnOverlayDetachments?: boolean;
     componentFactoryResolver?: ComponentFactoryResolver;
     container?: Type<C> | {
         type: Type<C>;