fix(dialog): better handling of custom ViewContainerRef with OnPush change detection (#6164)

* Adds a `markForCheck` call after starting the dialog animation, in order to ensure that it animates away if it was opened from a component with OnPush change detection.
* Since we can't know whether the animation will start right after the `markForCheck`, these changes switch to starting the backdrop animation together with the dialog animation. This avoids some slight timing issues where the backdrop can start its animation a little too early.
* Simplifies the dialog animation streams to avoid having to subscribe to the `completed` callback in the `MdDialogRef`.
* Fixes the demo app sidenav jumping to the bottom when it is opened from the homepage.
* Fixes some alignment in the dialog demo that got thrown off by the new input width.

The dialog changes should solve issues like #5118.

Author: crisbeto

src/demo-app/demo-app/demo-app.html

@@ -14,12 +14,13 @@
       <hr>
 
       <a md-list-item
+         tabindex="-1"
          (click)="start.close()"
          [routerLink]="['/baseline']">
         Baseline
       </a>
     </md-nav-list>
-    <button md-button (click)="start.close()">CLOSE</button>
+    <button md-button tabindex="-1" (click)="start.close()">CLOSE</button>
   </md-sidenav>
   <div>
     <md-toolbar color="primary">

src/demo-app/dialog/dialog-demo.scss

@@ -1,8 +1,4 @@
-.demo-dialog {
-  color: rebeccapurple;
-}
-
 .demo-dialog-card {
-  max-width: 350px;
+  max-width: 405px;
   margin: 20px 0;
 }

src/lib/datepicker/datepicker.ts

@@ -288,7 +288,8 @@ export class MdDatepicker<D> implements OnDestroy {
   /** Open the calendar as a dialog. */
   private _openAsDialog(): void {
     this._dialogRef = this._dialog.open(MdDatepickerContent, {
-      direction: this._dir ? this._dir.value : 'ltr'
+      direction: this._dir ? this._dir.value : 'ltr',
+      viewContainerRef: this._viewContainerRef,
     });
     this._dialogRef.afterClosed().subscribe(() => this.close());
     this._dialogRef.componentInstance.datepicker = this;

src/lib/dialog/dialog-container.ts

@@ -16,6 +16,7 @@ import {
   EventEmitter,
   Inject,
   Optional,
+  ChangeDetectorRef,
 } from '@angular/core';
 import {
   animate,
@@ -68,7 +69,7 @@ export function throwMdDialogContentAlreadyAttachedError() {
     '[attr.aria-labelledby]': '_ariaLabelledBy',
     '[attr.aria-describedby]': '_config?.ariaDescribedBy || null',
     '[@slideDialog]': '_state',
-    '(@slideDialog.start)': 'this._isAnimating = true',
+    '(@slideDialog.start)': '_onAnimationStart($event)',
     '(@slideDialog.done)': '_onAnimationDone($event)',
   },
 })
@@ -82,17 +83,14 @@ export class MdDialogContainer extends BasePortalHost {
   /** Element that was focused before the dialog was opened. Save this to restore upon close. */
   private _elementFocusedBeforeDialogWasOpened: HTMLElement | null = null;
 
-  /** Reference to the global document object. */
-  private _document: Document;
-
   /** The dialog configuration. */
   _config: MdDialogConfig;
 
   /** State of the dialog animation. */
   _state: 'void' | 'enter' | 'exit' = 'enter';
 
-  /** Emits the current animation state whenever it changes. */
-  _onAnimationStateChange = new EventEmitter<AnimationEvent>();
+  /** Emits when an animation state changes. */
+  _animationStateChanged = new EventEmitter<AnimationEvent>();
 
   /** ID of the element that should be considered as the dialog's label. */
   _ariaLabelledBy: string | null = null;
@@ -104,10 +102,10 @@ export class MdDialogContainer extends BasePortalHost {
     private _ngZone: NgZone,
     private _elementRef: ElementRef,
     private _focusTrapFactory: FocusTrapFactory,
-    @Optional() @Inject(DOCUMENT) _document: any) {
+    private _changeDetectorRef: ChangeDetectorRef,
+    @Optional() @Inject(DOCUMENT) private _document: any) {
 
     super();
-    this._document = _document;
   }
 
   /**
@@ -171,15 +169,28 @@ export class MdDialogContainer extends BasePortalHost {
 
   /** Callback, invoked whenever an animation on the host completes. */
   _onAnimationDone(event: AnimationEvent) {
-    this._onAnimationStateChange.emit(event);
-
     if (event.toState === 'enter') {
       this._trapFocus();
     } else if (event.toState === 'exit') {
       this._restoreFocus();
-      this._onAnimationStateChange.complete();
     }
 
+    this._animationStateChanged.emit(event);
     this._isAnimating = false;
   }
+
+  /** Callback, invoked when an animation on the host starts. */
+  _onAnimationStart(event: AnimationEvent) {
+    this._isAnimating = true;
+    this._animationStateChanged.emit(event);
+  }
+
+  /** Starts the dialog exit animation. */
+  _startExitAnimation(): void {
+    this._state = 'exit';
+
+    // Mark the container for check so it can react if the
+    // view container is using OnPush change detection.
+    this._changeDetectorRef.markForCheck();
+  }
 }

src/lib/dialog/dialog-ref.ts

@@ -7,12 +7,11 @@
  */
 
 import {OverlayRef, GlobalPositionStrategy} from '../core';
-import {AnimationEvent} from '@angular/animations';
 import {DialogPosition} from './dialog-config';
 import {Observable} from 'rxjs/Observable';
 import {Subject} from 'rxjs/Subject';
 import {MdDialogContainer} from './dialog-container';
-import {filter} from '../core/rxjs/index';
+import {RxChain, first, filter} from '../core/rxjs/index';
 
 
 // TODO(jelbourn): resizing
@@ -36,9 +35,11 @@ export class MdDialogRef<T> {
   private _result: any;
 
   constructor(private _overlayRef: OverlayRef, private _containerInstance: MdDialogContainer) {
-    filter.call(_containerInstance._onAnimationStateChange,
-      (event: AnimationEvent) => event.toState === 'exit')
-      .subscribe(() => this._overlayRef.dispose(), undefined, () => {
+    RxChain.from(_containerInstance._animationStateChanged)
+      .call(filter, event => event.phaseName === 'done' && event.toState === 'exit')
+      .call(first)
+      .subscribe(() => {
+        this._overlayRef.dispose();
         this._afterClosed.next(this._result);
         this._afterClosed.complete();
         this.componentInstance = null!;
@@ -51,8 +52,14 @@ export class MdDialogRef<T> {
    */
   close(dialogResult?: any): void {
     this._result = dialogResult;
-    this._containerInstance._state = 'exit';
-    this._overlayRef.detachBackdrop(); // Transition the backdrop in parallel with the dialog.
+
+    // Transition the backdrop in parallel to the dialog.
+    RxChain.from(this._containerInstance._animationStateChanged)
+      .call(filter, event => event.phaseName === 'start')
+      .call(first)
+      .subscribe(() => this._overlayRef.detachBackdrop());
+
+    this._containerInstance._startExitAnimation();
   }
 
   /**

src/lib/dialog/dialog.spec.ts

@@ -15,6 +15,7 @@ import {
   ViewContainerRef,
   Injector,
   Inject,
+  ChangeDetectionStrategy,
 } from '@angular/core';
 import {By} from '@angular/platform-browser';
 import {NoopAnimationsModule} from '@angular/platform-browser/animations';
@@ -153,6 +154,31 @@ describe('MdDialog', () => {
     });
   }));
 
+  it('should close from a ViewContainerRef with OnPush change detection', fakeAsync(() => {
+    const onPushFixture = TestBed.createComponent(ComponentWithOnPushViewContainer);
+
+    onPushFixture.detectChanges();
+
+    const dialogRef = dialog.open(PizzaMsg, {
+      viewContainerRef: onPushFixture.componentInstance.viewContainerRef
+    });
+
+    flushMicrotasks();
+    onPushFixture.detectChanges();
+    flushMicrotasks();
+
+    expect(overlayContainerElement.querySelectorAll('md-dialog-container').length)
+        .toBe(1, 'Expected one open dialog.');
+
+    dialogRef.close();
+    flushMicrotasks();
+    onPushFixture.detectChanges();
+    tick(500);
+
+    expect(overlayContainerElement.querySelectorAll('md-dialog-container').length)
+        .toBe(0, 'Expected no open dialogs.');
+  }));
+
   it('should close when clicking on the overlay backdrop', async(() => {
     dialog.open(PizzaMsg, {
       viewContainerRef: testViewContainerRef
@@ -385,14 +411,25 @@ describe('MdDialog', () => {
 
   it('should have the componentInstance available in the afterClosed callback', fakeAsync(() => {
     let dialogRef = dialog.open(PizzaMsg);
+    let spy = jasmine.createSpy('afterClosed spy');
+
+    flushMicrotasks();
+    viewContainerFixture.detectChanges();
+    flushMicrotasks();
 
     dialogRef.afterClosed().subscribe(() => {
+      spy();
       expect(dialogRef.componentInstance).toBeTruthy('Expected component instance to be defined.');
     });
 
     dialogRef.close();
-    tick(500);
+
+    flushMicrotasks();
     viewContainerFixture.detectChanges();
+    tick(500);
+
+    // Ensure that the callback actually fires.
+    expect(spy).toHaveBeenCalled();
   }));
 
   describe('passing in data', () => {
@@ -566,22 +603,22 @@ describe('MdDialog', () => {
       document.body.appendChild(button);
       button.focus();
 
-      let dialogRef = dialog.open(PizzaMsg, {
-        viewContainerRef: testViewContainerRef
-      });
+      let dialogRef = dialog.open(PizzaMsg, { viewContainerRef: testViewContainerRef });
 
+      flushMicrotasks();
       viewContainerFixture.detectChanges();
       flushMicrotasks();
+
       expect(document.activeElement.id)
           .not.toBe('dialog-trigger', 'Expected the focus to change when dialog was opened.');
 
       dialogRef.close();
       expect(document.activeElement.id).not.toBe('dialog-trigger',
           'Expcted the focus not to have changed before the animation finishes.');
 
-      tick(500);
-      viewContainerFixture.detectChanges();
       flushMicrotasks();
+      viewContainerFixture.detectChanges();
+      tick(500);
 
       expect(document.activeElement.id).toBe('dialog-trigger',
           'Expected that the trigger was refocused after the dialog is closed.');
@@ -603,9 +640,12 @@ describe('MdDialog', () => {
 
       let dialogRef = dialog.open(PizzaMsg, { viewContainerRef: testViewContainerRef });
 
-      dialogRef.afterClosed().subscribe(() => input.focus());
+      tick(500);
+      viewContainerFixture.detectChanges();
 
+      dialogRef.afterClosed().subscribe(() => input.focus());
       dialogRef.close();
+
       tick(500);
       viewContainerFixture.detectChanges();
       flushMicrotasks();
@@ -775,6 +815,14 @@ class DirectiveWithViewContainer {
   constructor(public viewContainerRef: ViewContainerRef) { }
 }
 
+@Component({
+  changeDetection: ChangeDetectionStrategy.OnPush,
+  template: 'hello',
+})
+class ComponentWithOnPushViewContainer {
+  constructor(public viewContainerRef: ViewContainerRef) { }
+}
+
 @Component({
   selector: 'arbitrary-component',
   template: `<dir-with-view-container></dir-with-view-container>`,
@@ -829,6 +877,7 @@ const TEST_DIRECTIVES = [
   ComponentWithChildViewContainer,
   PizzaMsg,
   DirectiveWithViewContainer,
+  ComponentWithOnPushViewContainer,
   ContentElementDialog,
   DialogWithInjectedData
 ];