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

…26608)

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.

(cherry picked from commit 8f29413)

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,14 @@ export class DialogConfig<D = unknown, R = unknown, C extends BasePortalOutlet =
    */
   closeOnDestroy?: boolean = true;
 
+  /**
+   * Whether the dialog should close when the underlying overlay is detached. This is useful if
+   * another service is wrapping the dialog and is managing the destruction instead. E.g. an
+   * external detachment can happen as a result of a scroll strategy triggering it or when the
+   * browser location changes.
+   */
+  closeOnOverlayDetachments?: boolean = true;
+
   /** Alternate `ComponentFactoryResolver` to use when resolving the associated component. */
   componentFactoryResolver?: ComponentFactoryResolver;
 

src/cdk/dialog/dialog-ref.ts

@@ -8,7 +8,7 @@
 
 import {OverlayRef} from '@angular/cdk/overlay';
 import {ESCAPE, hasModifierKey} from '@angular/cdk/keycodes';
-import {Observable, Subject} from 'rxjs';
+import {Observable, Subject, Subscription} from 'rxjs';
 import {DialogConfig} from './dialog-config';
 import {FocusOrigin} from '@angular/cdk/a11y';
 import {BasePortalOutlet} from '@angular/cdk/portal';
@@ -50,6 +50,9 @@ export class DialogRef<R = unknown, C = unknown> {
   /** Unique ID for the dialog. */
   readonly id: string;
 
+  /** Subscription to external detachments of the dialog. */
+  private _detachSubscription: Subscription;
+
   constructor(
     readonly overlayRef: OverlayRef,
     readonly config: DialogConfig<any, DialogRef<R, C>, BasePortalOutlet>,
@@ -72,6 +75,13 @@ export class DialogRef<R = unknown, C = unknown> {
         this.close(undefined, {focusOrigin: 'mouse'});
       }
     });
+
+    this._detachSubscription = overlayRef.detachments().subscribe(() => {
+      // Check specifically for `false`, because we want `undefined` to be treated like `true`.
+      if (config.closeOnOverlayDetachments !== false) {
+        this.close();
+      }
+    });
   }
 
   /**
@@ -83,6 +93,9 @@ export class DialogRef<R = unknown, C = unknown> {
     if (this.containerInstance) {
       const closedSubject = this.closed as Subject<R | undefined>;
       this.containerInstance._closeInteractionType = options?.focusOrigin || 'program';
+      // Drop the detach subscription first since it can be triggered by the
+      // `dispose` call and override the result of this closing sequence.
+      this._detachSubscription.unsubscribe();
       this.overlayRef.dispose();
       closedSubject.next(result);
       closedSubject.complete();

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,9 @@ 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.
+      // The Material dialog ref handles this manually.
+      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>;