fix(dialog): multiple dialogs not opening with disabled animations

With SHA 36f708c, second dialogs can be only opened if the first dialog finished animating.

Due to the recent update to Angular 4.3, the animation trigger callbacks are firing in a wrong order (done callback fires before start), which causes the _isAnimating property to be set to true while the animation already finished.

@crisbeto Not too happy how the tests ended up.

Fixes #6719

Author: devversion

src/lib/datepicker/datepicker.spec.ts

@@ -1,5 +1,5 @@
 import {Component, ViewChild} from '@angular/core';
-import {async, ComponentFixture, TestBed, inject} from '@angular/core/testing';
+import {async, ComponentFixture, TestBed, inject, fakeAsync, flush} from '@angular/core/testing';
 import {NoopAnimationsModule} from '@angular/platform-browser/animations';
 import {FormControl, FormsModule, ReactiveFormsModule} from '@angular/forms';
 import {By} from '@angular/platform-browser';
@@ -152,7 +152,7 @@ describe('MdDatepicker', () => {
             .toBe(true, 'Expected default ESCAPE action to be prevented.');
       });
 
-      it('close should close dialog', () => {
+      it('close should close dialog', fakeAsync(() => {
         testComponent.touch = true;
         fixture.detectChanges();
 
@@ -163,13 +163,13 @@ describe('MdDatepicker', () => {
 
         testComponent.datepicker.close();
         fixture.detectChanges();
+        flush();
 
-        fixture.whenStable().then(() => {
-          expect(document.querySelector('md-dialog-container')).toBeNull();
-        });
-      });
+        expect(document.querySelector('md-dialog-container')).toBeNull();
+        flush();
+      }));
 
-      it('setting selected should update input and close calendar', () => {
+      it('setting selected should update input and close calendar', fakeAsync(() => {
         testComponent.touch = true;
         fixture.detectChanges();
 
@@ -182,12 +182,12 @@ describe('MdDatepicker', () => {
         let cells = document.querySelectorAll('.mat-calendar-body-cell');
         dispatchMouseEvent(cells[1], 'click');
         fixture.detectChanges();
+        flush();
 
-        fixture.whenStable().then(() => {
-          expect(document.querySelector('md-dialog-container')).toBeNull();
-          expect(testComponent.datepickerInput.value).toEqual(new Date(2020, JAN, 2));
-        });
-      });
+        expect(document.querySelector('md-dialog-container')).toBeNull();
+        expect(testComponent.datepickerInput.value).toEqual(new Date(2020, JAN, 2));
+        flush();
+      }));
 
       it('clicking the currently selected date should close the calendar ' +
           'without firing selectedChanged', () => {

src/lib/dialog/dialog-container.ts

@@ -182,8 +182,13 @@ export class MdDialogContainer extends BasePortalHost {
       this._restoreFocus();
     }
 
-    this._animationStateChanged.emit(event);
-    this._isAnimating = false;
+    // Note: as of Angular 4.3, the animations module seems to fire the `start` callback before
+    // the end if animations are disabled. Make this call async to ensure that it still fires
+    // at the appropriate time.
+    Promise.resolve().then(() => {
+      this._animationStateChanged.emit(event);
+      this._isAnimating = false;
+    });
   }
 
   /** Callback, invoked when an animation on the host starts. */

src/lib/dialog/dialog.spec.ts

@@ -6,6 +6,7 @@ import {
   ComponentFixture,
   TestBed,
   tick,
+  flush,
 } from '@angular/core/testing';
 import {
   NgModule,
@@ -149,21 +150,21 @@ describe('MdDialog', () => {
     expect(dialogContainerElement.getAttribute('aria-describedby')).toBe('description-element');
   });
 
-  it('should close a dialog and get back a result', async(() => {
+  it('should close a dialog and get back a result', fakeAsync(() => {
     let dialogRef = dialog.open(PizzaMsg, { viewContainerRef: testViewContainerRef });
     let afterCloseCallback = jasmine.createSpy('afterClose callback');
 
     dialogRef.afterClosed().subscribe(afterCloseCallback);
     dialogRef.close('Charmander');
     viewContainerFixture.detectChanges();
+    flush();
 
-    viewContainerFixture.whenStable().then(() => {
-      expect(afterCloseCallback).toHaveBeenCalledWith('Charmander');
-      expect(overlayContainerElement.querySelector('md-dialog-container')).toBeNull();
-    });
+    expect(afterCloseCallback).toHaveBeenCalledWith('Charmander');
+    expect(overlayContainerElement.querySelector('md-dialog-container')).toBeNull();
+    flush();
   }));
 
-  it('should close a dialog and get back a result before it is closed', async(() => {
+  it('should close a dialog and get back a result before it is closed', fakeAsync(() => {
     const dialogRef = dialog.open(PizzaMsg, {viewContainerRef: testViewContainerRef});
 
     // beforeClose should emit before dialog container is destroyed
@@ -175,24 +176,24 @@ describe('MdDialog', () => {
     dialogRef.beforeClose().subscribe(beforeCloseHandler);
     dialogRef.close('Bulbasaurus');
     viewContainerFixture.detectChanges();
+    flush();
 
-    viewContainerFixture.whenStable().then(() => {
-      expect(beforeCloseHandler).toHaveBeenCalledWith('Bulbasaurus');
-      expect(overlayContainerElement.querySelector('md-dialog-container')).toBeNull();
-    });
+    expect(beforeCloseHandler).toHaveBeenCalledWith('Bulbasaurus');
+    expect(overlayContainerElement.querySelector('md-dialog-container')).toBeNull();
+    flush();
   }));
 
-  it('should close a dialog via the escape key', async(() => {
+  it('should close a dialog via the escape key', fakeAsync(() => {
     dialog.open(PizzaMsg, {
       viewContainerRef: testViewContainerRef
     });
 
     dispatchKeyboardEvent(document, 'keydown', ESCAPE);
     viewContainerFixture.detectChanges();
+    flush();
 
-    viewContainerFixture.whenStable().then(() => {
-      expect(overlayContainerElement.querySelector('md-dialog-container')).toBeNull();
-    });
+    expect(overlayContainerElement.querySelector('md-dialog-container')).toBeNull();
+    flush();
   }));
 
   it('should close from a ViewContainerRef with OnPush change detection', fakeAsync(() => {
@@ -220,7 +221,7 @@ describe('MdDialog', () => {
         .toBe(0, 'Expected no open dialogs.');
   }));
 
-  it('should close when clicking on the overlay backdrop', async(() => {
+  it('should close when clicking on the overlay backdrop', fakeAsync(() => {
     dialog.open(PizzaMsg, {
       viewContainerRef: testViewContainerRef
     });
@@ -231,13 +232,13 @@ describe('MdDialog', () => {
 
     backdrop.click();
     viewContainerFixture.detectChanges();
+    flush();
 
-    viewContainerFixture.whenStable().then(() => {
-      expect(overlayContainerElement.querySelector('md-dialog-container')).toBeFalsy();
-    });
+    expect(overlayContainerElement.querySelector('md-dialog-container')).toBeFalsy();
+    flush();
   }));
 
-  it('should emit the backdropClick stream when clicking on the overlay backdrop', async(() => {
+  it('should emit the backdropClick stream when clicking on the overlay backdrop', fakeAsync(() => {
     const dialogRef = dialog.open(PizzaMsg, {
       viewContainerRef: testViewContainerRef
     });
@@ -253,12 +254,12 @@ describe('MdDialog', () => {
     expect(spy).toHaveBeenCalledTimes(1);
 
     viewContainerFixture.detectChanges();
+    flush();
 
-    viewContainerFixture.whenStable().then(() => {
-      // Additional clicks after the dialog has closed should not be emitted
-      backdrop.click();
-      expect(spy).toHaveBeenCalledTimes(1);
-    });
+    // Additional clicks after the dialog has closed should not be emitted
+    backdrop.click();
+    expect(spy).toHaveBeenCalledTimes(1);
+    flush();
   }));
 
   it('should notify the observers if a dialog has been opened', () => {
@@ -269,7 +270,7 @@ describe('MdDialog', () => {
     });
   });
 
-  it('should notify the observers if all open dialogs have finished closing', async(() => {
+  it('should notify the observers if all open dialogs have finished closing', fakeAsync(() => {
     const ref1 = dialog.open(PizzaMsg, { viewContainerRef: testViewContainerRef });
     const ref2 = dialog.open(ContentElementDialog, { viewContainerRef: testViewContainerRef });
     const spy = jasmine.createSpy('afterAllClosed spy');
@@ -278,14 +279,16 @@ describe('MdDialog', () => {
 
     ref1.close();
     viewContainerFixture.detectChanges();
+    flush();
 
-    viewContainerFixture.whenStable().then(() => {
-      expect(spy).not.toHaveBeenCalled();
+    expect(spy).not.toHaveBeenCalled();
 
-      ref2.close();
-      viewContainerFixture.detectChanges();
-      viewContainerFixture.whenStable().then(() => expect(spy).toHaveBeenCalled());
-    });
+    ref2.close();
+    viewContainerFixture.detectChanges();
+    flush();
+
+    expect(spy).toHaveBeenCalled();
+    flush();
   }));
 
   it('should emit the afterAllClosed stream on subscribe if there are no open dialogs', () => {
@@ -418,7 +421,7 @@ describe('MdDialog', () => {
     expect(overlayPane.getAttribute('dir')).toBe('rtl');
   });
 
-  it('should close all of the dialogs', async(() => {
+  it('should close all of the dialogs', fakeAsync(() => {
     dialog.open(PizzaMsg);
     dialog.open(PizzaMsg);
     dialog.open(PizzaMsg);
@@ -427,10 +430,10 @@ describe('MdDialog', () => {
 
     dialog.closeAll();
     viewContainerFixture.detectChanges();
+    flush();
 
-    viewContainerFixture.whenStable().then(() => {
-      expect(overlayContainerElement.querySelectorAll('md-dialog-container').length).toBe(0);
-    });
+    expect(overlayContainerElement.querySelectorAll('md-dialog-container').length).toBe(0);
+    flush();
   }));
 
   it('should set the proper animation states', () => {
@@ -445,32 +448,32 @@ describe('MdDialog', () => {
     expect(dialogContainer._state).toBe('exit');
   });
 
-  it('should close all dialogs when the user goes forwards/backwards in history', async(() => {
+  it('should close all dialogs when the user goes forwards/backwards in history', fakeAsync(() => {
     dialog.open(PizzaMsg);
     dialog.open(PizzaMsg);
 
     expect(overlayContainerElement.querySelectorAll('md-dialog-container').length).toBe(2);
 
     mockLocation.simulateUrlPop('');
     viewContainerFixture.detectChanges();
+    flush();
 
-    viewContainerFixture.whenStable().then(() => {
-      expect(overlayContainerElement.querySelectorAll('md-dialog-container').length).toBe(0);
-    });
+    expect(overlayContainerElement.querySelectorAll('md-dialog-container').length).toBe(0);
+    flush();
   }));
 
-  it('should close all open dialogs when the location hash changes', async(() => {
+  it('should close all open dialogs when the location hash changes', fakeAsync(() => {
     dialog.open(PizzaMsg);
     dialog.open(PizzaMsg);
 
     expect(overlayContainerElement.querySelectorAll('md-dialog-container').length).toBe(2);
 
     mockLocation.simulateHashChange('');
     viewContainerFixture.detectChanges();
+    flush();
 
-    viewContainerFixture.whenStable().then(() => {
-      expect(overlayContainerElement.querySelectorAll('md-dialog-container').length).toBe(0);
-    });
+    expect(overlayContainerElement.querySelectorAll('md-dialog-container').length).toBe(0);
+    flush();
   }));
 
   it('should have the componentInstance available in the afterClosed callback', fakeAsync(() => {
@@ -519,17 +522,17 @@ describe('MdDialog', () => {
     });
   });
 
-  it('should not keep a reference to the component after the dialog is closed', async(() => {
+  it('should not keep a reference to the component after the dialog is closed', fakeAsync(() => {
     let dialogRef = dialog.open(PizzaMsg);
 
     expect(dialogRef.componentInstance).toBeTruthy();
 
     dialogRef.close();
     viewContainerFixture.detectChanges();
+    flush();
 
-    viewContainerFixture.whenStable().then(() => {
-      expect(dialogRef.componentInstance).toBeFalsy('Expected reference to have been cleared.');
-    });
+    expect(dialogRef.componentInstance).toBeFalsy('Expected reference to have been cleared.');
+    flush();
   }));
 
   it('should assign a unique id to each dialog', () => {
@@ -583,7 +586,7 @@ describe('MdDialog', () => {
       expect(overlayContainerElement.querySelector('md-dialog-container')).toBeTruthy();
     });
 
-    it('should allow for the disableClose option to be updated while open', async(() => {
+    it('should allow for the disableClose option to be updated while open', fakeAsync(() => {
       let dialogRef = dialog.open(PizzaMsg, {
         disableClose: true,
         viewContainerRef: testViewContainerRef
@@ -600,9 +603,10 @@ describe('MdDialog', () => {
       backdrop.click();
 
       viewContainerFixture.detectChanges();
-      viewContainerFixture.whenStable().then(() => {
-        expect(overlayContainerElement.querySelector('md-dialog-container')).toBeFalsy();
-      });
+      flush();
+
+      expect(overlayContainerElement.querySelector('md-dialog-container')).toBeFalsy();
+      flush();
     }));
   });
 
@@ -863,7 +867,7 @@ describe('MdDialog with a parent MdDialog', () => {
   });
 
   it('should close dialogs opened by a parent when calling closeAll on a child MdDialog',
-    async(() => {
+    fakeAsync(() => {
       parentDialog.open(PizzaMsg);
       fixture.detectChanges();
 
@@ -872,15 +876,15 @@ describe('MdDialog with a parent MdDialog', () => {
 
       childDialog.closeAll();
       fixture.detectChanges();
+      flush();
 
-      fixture.whenStable().then(() => {
-        expect(overlayContainerElement.textContent!.trim())
-            .toBe('', 'Expected closeAll on child MdDialog to close dialog opened by parent');
-      });
+      expect(overlayContainerElement.textContent!.trim())
+        .toBe('', 'Expected closeAll on child MdDialog to close dialog opened by parent');
+      flush();
     }));
 
   it('should close dialogs opened by a child when calling closeAll on a parent MdDialog',
-    async(() => {
+    fakeAsync(() => {
       childDialog.open(PizzaMsg);
       fixture.detectChanges();
 
@@ -889,22 +893,22 @@ describe('MdDialog with a parent MdDialog', () => {
 
       parentDialog.closeAll();
       fixture.detectChanges();
+      flush();
 
-      fixture.whenStable().then(() => {
-        expect(overlayContainerElement.textContent!.trim())
-            .toBe('', 'Expected closeAll on parent MdDialog to close dialog opened by child');
-      });
+      expect(overlayContainerElement.textContent!.trim())
+        .toBe('', 'Expected closeAll on parent MdDialog to close dialog opened by child');
+      flush();
     }));
 
-  it('should close the top dialog via the escape key', async(() => {
+  it('should close the top dialog via the escape key', fakeAsync(() => {
     childDialog.open(PizzaMsg);
 
     dispatchKeyboardEvent(document, 'keydown', ESCAPE);
     fixture.detectChanges();
+    flush();
 
-    fixture.whenStable().then(() => {
-      expect(overlayContainerElement.querySelector('md-dialog-container')).toBeNull();
-    });
+    expect(overlayContainerElement.querySelector('md-dialog-container')).toBeNull();
+    flush();
   }));
 });