fix(dialog): incorrect dialog state for close animation (#19034)

The dialog currently sets its state to `CLOSED` if the
close animation is still in progress. This happens due to a
misplaced state update assignment.

Author: devversion

src/material/dialog/dialog-ref.ts

@@ -74,7 +74,7 @@ export class MatDialogRef<T, R = any> {
       take(1)
     ).subscribe(() => {
       clearTimeout(this._closeFallbackTimeout);
-      this._overlayRef.dispose();
+      this._finishDialogClose();
     });
 
     _overlayRef.detachments().subscribe(() => {
@@ -119,17 +119,15 @@ export class MatDialogRef<T, R = any> {
     .subscribe(event => {
       this._beforeClosed.next(dialogResult);
       this._beforeClosed.complete();
-      this._state = MatDialogState.CLOSED;
       this._overlayRef.detachBackdrop();
 
       // The logic that disposes of the overlay depends on the exit animation completing, however
       // it isn't guaranteed if the parent view is destroyed while it's running. Add a fallback
       // timeout which will clean everything up if the animation hasn't fired within the specified
       // amount of time plus 100ms. We don't need to run this outside the NgZone, because for the
       // vast majority of cases the timeout will have been cleared before it has the chance to fire.
-      this._closeFallbackTimeout = setTimeout(() => {
-        this._overlayRef.dispose();
-      }, event.totalTime + 100);
+      this._closeFallbackTimeout = setTimeout(() => this._finishDialogClose(),
+          event.totalTime + 100);
     });
 
     this._containerInstance._startExitAnimation();
@@ -223,6 +221,15 @@ export class MatDialogRef<T, R = any> {
     return this._state;
   }
 
+  /**
+   * Finishes the dialog close by updating the state of the dialog
+   * and disposing the overlay.
+   */
+  private _finishDialogClose() {
+    this._state = MatDialogState.CLOSED;
+    this._overlayRef.dispose();
+  }
+
   /** Fetches the position strategy object from the overlay ref. */
   private _getPositionStrategy(): GlobalPositionStrategy {
     return this._overlayRef.getConfig().positionStrategy as GlobalPositionStrategy;

src/material/dialog/dialog.spec.ts

@@ -20,7 +20,7 @@ import {
   ComponentFactoryResolver
 } from '@angular/core';
 import {By} from '@angular/platform-browser';
-import {NoopAnimationsModule} from '@angular/platform-browser/animations';
+import {BrowserAnimationsModule, NoopAnimationsModule} from '@angular/platform-browser/animations';
 import {Location} from '@angular/common';
 import {SpyLocation} from '@angular/common/testing';
 import {Directionality} from '@angular/cdk/bidi';
@@ -777,19 +777,6 @@ describe('MatDialog', () => {
       expect(resolver.resolveComponentFactory).toHaveBeenCalled();
     }));
 
-  it('should return the current state of the dialog', fakeAsync(() => {
-    const dialogRef = dialog.open(PizzaMsg, {viewContainerRef: testViewContainerRef});
-
-    expect(dialogRef.getState()).toBe(MatDialogState.OPEN);
-    dialogRef.close();
-    viewContainerFixture.detectChanges();
-
-    expect(dialogRef.getState()).toBe(MatDialogState.CLOSING);
-    flush();
-
-    expect(dialogRef.getState()).toBe(MatDialogState.CLOSED);
-  }));
-
   describe('passing in data', () => {
     it('should be able to pass in data', () => {
       let config = {
@@ -1579,6 +1566,59 @@ describe('MatDialog with default options', () => {
 });
 
 
+describe('MatDialog with animations enabled', () => {
+  let dialog: MatDialog;
+  let overlayContainer: OverlayContainer;
+
+  let testViewContainerRef: ViewContainerRef;
+  let viewContainerFixture: ComponentFixture<ComponentWithChildViewContainer>;
+
+  beforeEach(fakeAsync(() => {
+    TestBed.configureTestingModule({
+      imports: [MatDialogModule, DialogTestModule, BrowserAnimationsModule],
+    });
+
+    TestBed.compileComponents();
+  }));
+
+  beforeEach(inject([MatDialog, OverlayContainer], (d: MatDialog, oc: OverlayContainer) => {
+    dialog = d;
+    overlayContainer = oc;
+
+    viewContainerFixture = TestBed.createComponent(ComponentWithChildViewContainer);
+    viewContainerFixture.detectChanges();
+    testViewContainerRef = viewContainerFixture.componentInstance.childViewContainer;
+  }));
+
+  afterEach(() => {
+    overlayContainer.ngOnDestroy();
+  });
+
+  it('should return the current state of the dialog', fakeAsync(() => {
+    const dialogRef = dialog.open(PizzaMsg, {viewContainerRef: testViewContainerRef});
+    // Duration of the close animation in milliseconds. Extracted from the
+    // Angular animations definition of the dialog.
+    const dialogCloseDuration = 75;
+
+    expect(dialogRef.getState()).toBe(MatDialogState.OPEN);
+    dialogRef.close();
+    viewContainerFixture.detectChanges();
+
+    expect(dialogRef.getState()).toBe(MatDialogState.CLOSING);
+
+    // Ensure that the closing state is still set if half of the animation has
+    // passed by. The dialog state should be only set to `closed` when the dialog
+    // finished the close animation.
+    tick(dialogCloseDuration / 2);
+    expect(dialogRef.getState()).toBe(MatDialogState.CLOSING);
+
+    // Flush the remaining duration of the closing animation. We flush all other remaining
+    // tasks (e.g. the fallback close timeout) to avoid fakeAsync pending timer failures.
+    flush();
+    expect(dialogRef.getState()).toBe(MatDialogState.CLOSED);
+  }));
+});
+
 @Directive({selector: 'dir-with-view-container'})
 class DirectiveWithViewContainer {
   constructor(public viewContainerRef: ViewContainerRef) { }