feat(overlay): make overlays synchronous

Author: jelbourn

src/components/dialog/dialog-container.ts

@@ -1,11 +1,10 @@
-import {Component, ComponentRef, ViewChild, AfterViewInit} from '@angular/core';
+import {Component, ComponentRef, ViewChild} from '@angular/core';
 import {
   BasePortalHost,
   ComponentPortal,
   TemplatePortal
 } from '@angular2-material/core/portal/portal';
 import {PortalHostDirective} from '@angular2-material/core/portal/portal-directives';
-import {PromiseCompleter} from '@angular2-material/core/async/promise-completer';
 import {MdDialogConfig} from './dialog-config';
 import {MdDialogContentAlreadyAttachedError} from './dialog-errors';
 
@@ -23,63 +22,23 @@ import {MdDialogContentAlreadyAttachedError} from './dialog-errors';
     '[attr.role]': 'dialogConfig?.role'
   }
 })
-export class MdDialogContainer extends BasePortalHost implements AfterViewInit {
+export class MdDialogContainer extends BasePortalHost {
   /** The portal host inside of this container into which the dialog content will be loaded. */
   @ViewChild(PortalHostDirective) _portalHost: PortalHostDirective;
 
-  /**
-   * Completer used to resolve the promise for cases when a portal is attempted to be attached,
-   * but AfterViewInit has not yet occured.
-   */
-  private _deferredAttachCompleter: PromiseCompleter<ComponentRef<any>>;
-
-  /** Portal to be attached upon AfterViewInit. */
-  private _deferredAttachPortal: ComponentPortal<any>;
-
   /** The dialog configuration. */
   dialogConfig: MdDialogConfig;
 
-  /** TODO: internal */
-  ngAfterViewInit() {
-    // If there was an attempted call to `attachComponentPortal` before this lifecycle stage,
-    // we actually perform the attachment now that the `@ViewChild` is resolved.
-    if (this._deferredAttachCompleter) {
-      this.attachComponentPortal(this._deferredAttachPortal).then(componentRef => {
-        this._deferredAttachCompleter.resolve(componentRef);
-
-        this._deferredAttachPortal = null;
-        this._deferredAttachCompleter = null;
-      }, () => {
-        this._deferredAttachCompleter.reject();
-        this._deferredAttachCompleter = null;
-        this._deferredAttachPortal = null;
-      });
-    }
-  }
-
   /** Attach a portal as content to this dialog container. */
-  attachComponentPortal<T>(portal: ComponentPortal<T>): Promise<ComponentRef<T>> {
-    if (this._portalHost) {
-      if (this._portalHost.hasAttached()) {
-        throw new MdDialogContentAlreadyAttachedError();
-      }
-
-      return this._portalHost.attachComponentPortal(portal);
-    } else {
-      // The @ViewChild query for the portalHost is not resolved until AfterViewInit, but this
-      // function may be called before this lifecycle event. As such, we defer the attachment of
-      // the portal until AfterViewInit.
-      if (this._deferredAttachCompleter) {
-        throw new MdDialogContentAlreadyAttachedError();
-      }
-
-      this._deferredAttachPortal = portal;
-      this._deferredAttachCompleter = new PromiseCompleter();
-      return this._deferredAttachCompleter.promise;
+  attachComponentPortal<T>(portal: ComponentPortal<T>): ComponentRef<T> {
+    if (this._portalHost.hasAttached()) {
+      throw new MdDialogContentAlreadyAttachedError();
     }
+
+    return this._portalHost.attachComponentPortal(portal);
   }
 
-  attachTemplatePortal(portal: TemplatePortal): Promise<Map<string, any>> {
+  attachTemplatePortal(portal: TemplatePortal): Map<string, any> {
     throw Error('Not yet implemented');
   }
 }

src/components/dialog/dialog.spec.ts

@@ -1,4 +1,4 @@
-import {inject, fakeAsync, async, ComponentFixture, TestBed} from '@angular/core/testing';
+import {inject, async, ComponentFixture, TestBed} from '@angular/core/testing';
 import {NgModule, Component, Directive, ViewChild, ViewContainerRef} from '@angular/core';
 import {MdDialog, MdDialogModule} from './dialog';
 import {OverlayContainer} from '@angular2-material/core/overlay/overlay-container';
@@ -27,9 +27,9 @@ describe('MdDialog', () => {
     TestBed.compileComponents();
   }));
 
-  beforeEach(inject([MdDialog], fakeAsync((d: MdDialog) => {
+  beforeEach(inject([MdDialog], (d: MdDialog) => {
     dialog = d;
-  })));
+  }));
 
   beforeEach(() => {
     viewContainerFixture = TestBed.createComponent(ComponentWithChildViewContainer);
@@ -38,72 +38,58 @@ describe('MdDialog', () => {
     testViewContainerRef = viewContainerFixture.componentInstance.childViewContainer;
   });
 
-  it('should open a dialog with a component', async(() => {
+  it('should open a dialog with a component', () => {
     let config = new MdDialogConfig();
     config.viewContainerRef = testViewContainerRef;
 
-    dialog.open(PizzaMsg, config).then(dialogRef => {
-      expect(overlayContainerElement.textContent).toContain('Pizza');
-      expect(dialogRef.componentInstance).toEqual(jasmine.any(PizzaMsg));
-      expect(dialogRef.componentInstance.dialogRef).toBe(dialogRef);
+    let dialogRef = dialog.open(PizzaMsg, config);
 
-      viewContainerFixture.detectChanges();
-      let dialogContainerElement = overlayContainerElement.querySelector('md-dialog-container');
-      expect(dialogContainerElement.getAttribute('role')).toBe('dialog');
-    });
+    viewContainerFixture.detectChanges();
 
-    detectChangesForDialogOpen(viewContainerFixture);
-  }));
+    expect(overlayContainerElement.textContent).toContain('Pizza');
+    expect(dialogRef.componentInstance).toEqual(jasmine.any(PizzaMsg));
+    expect(dialogRef.componentInstance.dialogRef).toBe(dialogRef);
+
+    viewContainerFixture.detectChanges();
+    let dialogContainerElement = overlayContainerElement.querySelector('md-dialog-container');
+    expect(dialogContainerElement.getAttribute('role')).toBe('dialog');
+  });
 
-  it('should apply the configured role to the dialog element', async(() => {
+  it('should apply the configured role to the dialog element', () => {
     let config = new MdDialogConfig();
     config.viewContainerRef = testViewContainerRef;
     config.role = 'alertdialog';
 
-    dialog.open(PizzaMsg, config).then(dialogRef => {
-      viewContainerFixture.detectChanges();
+    dialog.open(PizzaMsg, config);
 
-      let dialogContainerElement = overlayContainerElement.querySelector('md-dialog-container');
-      expect(dialogContainerElement.getAttribute('role')).toBe('alertdialog');
-    });
+    viewContainerFixture.detectChanges();
 
-    detectChangesForDialogOpen(viewContainerFixture);
-  }));
+    let dialogContainerElement = overlayContainerElement.querySelector('md-dialog-container');
+    expect(dialogContainerElement.getAttribute('role')).toBe('alertdialog');
+  });
 
-  it('should close a dialog and get back a result', async(() => {
+  it('should close a dialog and get back a result', () => {
     let config = new MdDialogConfig();
     config.viewContainerRef = testViewContainerRef;
 
-    dialog.open(PizzaMsg, config).then(dialogRef => {
-      viewContainerFixture.detectChanges();
+    let dialogRef = dialog.open(PizzaMsg, config);
 
-      let afterCloseResult: string;
-      dialogRef.afterClosed().subscribe(result => {
-        afterCloseResult = result;
-      });
+    viewContainerFixture.detectChanges();
 
-      dialogRef.close('Charmander');
+    viewContainerFixture.detectChanges();
 
-      viewContainerFixture.whenStable().then(() => {
-        expect(afterCloseResult).toBe('Charmander');
-        expect(overlayContainerElement.childNodes.length).toBe(0);
-      });
+    let afterCloseResult: string;
+    dialogRef.afterClosed().subscribe(result => {
+      afterCloseResult = result;
     });
 
-    detectChangesForDialogOpen(viewContainerFixture);
-  }));
-});
+    dialogRef.close('Charmander');
 
+    expect(afterCloseResult).toBe('Charmander');
+    expect(overlayContainerElement.querySelector('md-dialog-container')).toBeNull();
+  });
+});
 
-/** Runs the necessary detectChanges for a dialog to complete its opening. */
-function detectChangesForDialogOpen(fixture: ComponentFixture<ComponentWithChildViewContainer>) {
-  // TODO(jelbourn): figure out why the test zone is "stable" when there are still pending
-  // tasks, such that we have to use `setTimeout` to run the second round of change detection.
-  // Two rounds of change detection are necessary: one to *create* the dialog container, and
-  // another to cause the lifecycle events of the container to run and load the dialog content.
-  fixture.detectChanges();
-  setTimeout(() => fixture.detectChanges(), 50);
-}
 
 @Directive({selector: 'dir-with-view-container'})
 class DirectiveWithViewContainer {
@@ -123,10 +109,7 @@ class ComponentWithChildViewContainer {
 }
 
 /** Simple component for testing ComponentPortal. */
-@Component({
-  selector: 'pizza-msg',
-  template: '<p>Pizza</p>',
-})
+@Component({template: '<p>Pizza</p>'})
 class PizzaMsg {
   constructor(public dialogRef: MdDialogRef<PizzaMsg>) { }
 }

src/components/dialog/dialog.ts

@@ -39,21 +39,21 @@ export class MdDialog {
    * @param component Type of the component to load into the load.
    * @param config
    */
-  open<T>(component: ComponentType<T>, config: MdDialogConfig): Promise<MdDialogRef<T>> {
-    let overlayRef: OverlayRef;
+  open<T>(component: ComponentType<T>, config: MdDialogConfig): MdDialogRef<T> {
+    let overlayRef = this._createOverlay(config);
+    let dialogContainer = this._attachDialogContainer(overlayRef, config);
 
-    return this._createOverlay(config)
-        .then(overlay => overlayRef = overlay)
-        .then(overlay => this._attachDialogContainer(overlay, config))
-        .then(containerRef => this._attachDialogContent(component, containerRef, overlayRef));
+    // TODO: probably need to wait for dialogContainer ngOnInit before attaching.
+
+    return this._attachDialogContent(component, dialogContainer, overlayRef);
   }
 
   /**
    * Creates the overlay into which the dialog will be loaded.
    * @param dialogConfig The dialog configuration.
    * @returns A promise resolving to the OverlayRef for the created overlay.
    */
-  private _createOverlay(dialogConfig: MdDialogConfig): Promise<OverlayRef> {
+  private _createOverlay(dialogConfig: MdDialogConfig): OverlayRef {
     let overlayState = this._getOverlayState(dialogConfig);
     return this._overlay.create(overlayState);
   }
@@ -64,43 +64,41 @@ export class MdDialog {
    * @param config The dialog configuration.
    * @returns A promise resolving to a ComponentRef for the attached container.
    */
-  private _attachDialogContainer(overlay: OverlayRef, config: MdDialogConfig):
-      Promise<ComponentRef<MdDialogContainer>> {
+  private _attachDialogContainer(overlay: OverlayRef, config: MdDialogConfig): MdDialogContainer {
     let containerPortal = new ComponentPortal(MdDialogContainer, config.viewContainerRef);
-    return overlay.attach(containerPortal).then((containerRef: ComponentRef<MdDialogContainer>) => {
-      // Pass the config directly to the container so that it can consume any relevant settings.
-      containerRef.instance.dialogConfig = config;
-      return containerRef;
-    });
+
+    let containerRef: ComponentRef<MdDialogContainer> = overlay.attach(containerPortal);
+    containerRef.instance.dialogConfig = config;
+
+    return containerRef.instance;
   }
 
   /**
    * Attaches the user-provided component to the already-created MdDialogContainer.
    * @param component The type of component being loaded into the dialog.
-   * @param containerRef Reference to the wrapping MdDialogContainer.
+   * @param dialogContainer Reference to the wrapping MdDialogContainer.
    * @param overlayRef Reference to the overlay in which the dialog resides.
    * @returns A promise resolving to the MdDialogRef that should be returned to the user.
    */
   private _attachDialogContent<T>(
       component: ComponentType<T>,
-      containerRef: ComponentRef<MdDialogContainer>,
-      overlayRef: OverlayRef): Promise<MdDialogRef<T>> {
-    let dialogContainer = containerRef.instance;
-
+      dialogContainer: MdDialogContainer,
+      overlayRef: OverlayRef): MdDialogRef<T> {
     // Create a reference to the dialog we're creating in order to give the user a handle
     // to modify and close it.
-    let dialogRef = new MdDialogRef(overlayRef);
+    let dialogRef = <MdDialogRef<T>> new MdDialogRef(overlayRef);
 
     // We create an injector specifically for the component we're instantiating so that it can
     // inject the MdDialogRef. This allows a component loaded inside of a dialog to close itself
     // and, optionally, to return a value.
     let dialogInjector = new DialogInjector(dialogRef, this._injector);
 
     let contentPortal = new ComponentPortal(component, null, dialogInjector);
-    return dialogContainer.attachComponentPortal(contentPortal).then(contentRef => {
-      dialogRef.componentInstance = contentRef.instance;
-      return dialogRef;
-    });
+
+    let contentRef = dialogContainer.attachComponentPortal(contentPortal);
+    dialogRef.componentInstance = contentRef.instance;
+
+    return dialogRef;
   }
 
   /**

src/components/menu/menu-trigger.ts

@@ -58,21 +58,21 @@ export class MdMenuTrigger implements AfterViewInit, OnDestroy {
   get menuOpen(): boolean { return this._menuOpen; }
 
   @HostListener('click')
-  toggleMenu(): Promise<void> {
+  toggleMenu(): void {
     return this._menuOpen ? this.closeMenu() : this.openMenu();
   }
 
-  openMenu(): Promise<void> {
-    return this._createOverlay()
-      .then(() => this._overlayRef.attach(this._portal))
-      .then(() => this._setIsMenuOpen(true));
+  openMenu(): void {
+    this._createOverlay();
+    this._overlayRef.attach(this._portal);
+    this._setIsMenuOpen(true);
   }
 
-  closeMenu(): Promise<void> {
-    if (!this._overlayRef) { return Promise.resolve(); }
-
-    return this._overlayRef.detach()
-        .then(() => this._setIsMenuOpen(false));
+  closeMenu(): void {
+    if (this._overlayRef) {
+      this._overlayRef.detach();
+      this._setIsMenuOpen(false);
+    }
   }
 
   destroyMenu(): void {
@@ -103,12 +103,11 @@ export class MdMenuTrigger implements AfterViewInit, OnDestroy {
    *  This method creates the overlay from the provided menu's template and saves its
    *  OverlayRef so that it can be attached to the DOM when openMenu is called.
    */
-  private _createOverlay(): Promise<any> {
-    if (this._overlayRef) { return Promise.resolve(); }
-
-    this._portal = new TemplatePortal(this.menu.templateRef, this._viewContainerRef);
-    return this._overlay.create(this._getOverlayConfig())
-        .then(overlay => this._overlayRef = overlay);
+  private _createOverlay(): void {
+    if (!this._overlayRef) {
+      this._portal = new TemplatePortal(this.menu.templateRef, this._viewContainerRef);
+      this._overlayRef = this._overlay.create(this._getOverlayConfig());
+    }
   }
 
   /**