feat(overlay): make overlays synchronous (#1079)

Author: jelbourn

src/demo-app/dialog/dialog-demo.ts

@@ -21,13 +21,11 @@ export class DialogDemo {
     let config = new MdDialogConfig();
     config.viewContainerRef = this.viewContainerRef;
 
-    this.dialog.open(JazzDialog, config).then(ref => {
-      this.dialogRef = ref;
+    this.dialogRef = this.dialog.open(JazzDialog, config);
 
-      this.dialogRef.afterClosed().subscribe(result => {
-        this.lastCloseResult = result;
-        this.dialogRef = null;
-      });
+    this.dialogRef.afterClosed().subscribe(result => {
+      this.lastCloseResult = result;
+      this.dialogRef = null;
     });
   }
 }

src/demo-app/overlay/overlay-demo.ts

@@ -44,9 +44,8 @@ export class OverlayDemo {
 
     this.nextPosition += 30;
 
-    this.overlay.create(config).then(ref => {
-      ref.attach(new ComponentPortal(RotiniPanel, this.viewContainerRef));
-    });
+    let overlayRef = this.overlay.create(config);
+    overlayRef.attach(new ComponentPortal(RotiniPanel, this.viewContainerRef));
   }
 
   openFusilliPanel() {
@@ -59,9 +58,8 @@ export class OverlayDemo {
 
     this.nextPosition += 30;
 
-    this.overlay.create(config).then(ref => {
-      ref.attach(this.templatePortals.first);
-    });
+    let overlayRef = this.overlay.create(config);
+    overlayRef.attach(this.templatePortals.first);
   }
 
   openSpaghettiPanel() {
@@ -75,9 +73,8 @@ export class OverlayDemo {
     let config = new OverlayState();
     config.positionStrategy = strategy;
 
-    this.overlay.create(config).then(ref => {
-      ref.attach(new ComponentPortal(SpagettiPanel, this.viewContainerRef));
-    });
+    let overlayRef = this.overlay.create(config);
+    overlayRef.attach(new ComponentPortal(SpagettiPanel, this.viewContainerRef));
   }
 }
 

src/lib/core/overlay/overlay-directives.ts

@@ -93,10 +93,8 @@ export class ConnectedOverlayDirective implements OnInit, OnDestroy {
             {originX: this.positions[0].overlayX, originY: this.positions[0].originY},
             {overlayX: this.positions[0].overlayX, overlayY: this.positions[0].overlayY});
 
-    this._overlay.create(overlayConfig).then(ref => {
-      this._overlayRef = ref;
-      this._overlayRef.attach(this._templatePortal);
-    });
+    this._overlayRef = this._overlay.create(overlayConfig);
+    this._overlayRef.attach(this._templatePortal);
   }
 
   /** Destroys the overlay created by this directive. */

src/lib/core/overlay/overlay-ref.ts

@@ -11,16 +11,11 @@ export class OverlayRef implements PortalHost {
       private _pane: HTMLElement,
       private _state: OverlayState) { }
 
-  attach(portal: Portal<any>): Promise<any> {
-    let attachPromise = this._portalHost.attach(portal);
+  attach(portal: Portal<any>): any {
+    let attachResult = this._portalHost.attach(portal);
+    this.updatePosition();
 
-    // Don't chain the .then() call in the return because we want the result of portalHost.attach
-    // to be returned from this method.
-    attachPromise.then(() => {
-      this.updatePosition();
-    });
-
-    return attachPromise;
+    return attachResult;
   }
 
   detach(): Promise<any> {

src/lib/core/overlay/overlay.spec.ts

@@ -1,10 +1,9 @@
-import {inject, fakeAsync, flushMicrotasks, TestBed, async} from '@angular/core/testing';
+import {inject, TestBed, async} from '@angular/core/testing';
 import {NgModule, Component, ViewChild, ViewContainerRef} from '@angular/core';
 import {TemplatePortalDirective, PortalModule} from '../portal/portal-directives';
 import {TemplatePortal, ComponentPortal} from '../portal/portal';
 import {Overlay} from './overlay';
 import {OverlayContainer} from './overlay-container';
-import {OverlayRef} from './overlay-ref';
 import {OverlayState} from './overlay-state';
 import {PositionStrategy} from './position/position-strategy';
 import {OverlayModule} from './overlay-directives';
@@ -30,68 +29,43 @@ describe('Overlay', () => {
     TestBed.compileComponents();
   }));
 
-  beforeEach(fakeAsync(inject([Overlay], (o: Overlay) => {
+  beforeEach(inject([Overlay], (o: Overlay) => {
     overlay = o;
 
     let fixture = TestBed.createComponent(TestComponentWithTemplatePortals);
     fixture.detectChanges();
     templatePortal = fixture.componentInstance.templatePortal;
     componentPortal = new ComponentPortal(PizzaMsg, fixture.componentInstance.viewContainerRef);
+  }));
 
-    flushMicrotasks();
-  })));
-
-  it('should load a component into an overlay', fakeAsync(() => {
-    let overlayRef: OverlayRef;
-
-    overlay.create().then(ref => {
-      overlayRef = ref;
-      overlayRef.attach(componentPortal);
-    });
-
-    flushMicrotasks();
+  it('should load a component into an overlay', () => {
+    let overlayRef = overlay.create();
+    overlayRef.attach(componentPortal);
 
     expect(overlayContainerElement.textContent).toContain('Pizza');
 
     overlayRef.dispose();
     expect(overlayContainerElement.childNodes.length).toBe(0);
     expect(overlayContainerElement.textContent).toBe('');
-  }));
-
-  it('should load a template portal into an overlay', fakeAsync(() => {
-    let overlayRef: OverlayRef;
-
-    overlay.create().then(ref => {
-      overlayRef = ref;
-      overlayRef.attach(templatePortal);
-    });
+  });
 
-    flushMicrotasks();
+  it('should load a template portal into an overlay', () => {
+    let overlayRef = overlay.create();
+    overlayRef.attach(templatePortal);
 
     expect(overlayContainerElement.textContent).toContain('Cake');
 
     overlayRef.dispose();
     expect(overlayContainerElement.childNodes.length).toBe(0);
     expect(overlayContainerElement.textContent).toBe('');
-  }));
-
-  it('should open multiple overlays', fakeAsync(() => {
-    let pizzaOverlayRef: OverlayRef;
-    let cakeOverlayRef: OverlayRef;
-
-    overlay.create().then(ref => {
-      pizzaOverlayRef = ref;
-      pizzaOverlayRef.attach(componentPortal);
-    });
-
-    flushMicrotasks();
+  });
 
-    overlay.create().then(ref => {
-      cakeOverlayRef = ref;
-      cakeOverlayRef.attach(templatePortal);
-    });
+  it('should open multiple overlays', () => {
+    let pizzaOverlayRef = overlay.create();
+    pizzaOverlayRef.attach(componentPortal);
 
-    flushMicrotasks();
+    let cakeOverlayRef = overlay.create();
+    cakeOverlayRef.attach(templatePortal);
 
     expect(overlayContainerElement.childNodes.length).toBe(2);
     expect(overlayContainerElement.textContent).toContain('Pizza');
@@ -104,7 +78,7 @@ describe('Overlay', () => {
     cakeOverlayRef.dispose();
     expect(overlayContainerElement.childNodes.length).toBe(0);
     expect(overlayContainerElement.textContent).toBe('');
-  }));
+  });
 
   describe('applyState', () => {
     let state: OverlayState;
@@ -113,17 +87,13 @@ describe('Overlay', () => {
       state = new OverlayState();
     });
 
-    it('should apply the positioning strategy', fakeAsync(() => {
+    it('should apply the positioning strategy', () => {
       state.positionStrategy = new FakePositionStrategy();
 
-      overlay.create(state).then(ref => {
-        ref.attach(componentPortal);
-      });
-
-      flushMicrotasks();
+      overlay.create(state).attach(componentPortal);
 
       expect(overlayContainerElement.querySelectorAll('.fake-positioned').length).toBe(1);
-    }));
+    });
   });
 });
 

src/lib/core/overlay/overlay.ts

@@ -36,8 +36,8 @@ export class Overlay {
    * @param state State to apply to the overlay.
    * @returns A reference to the created overlay.
    */
-  create(state: OverlayState = defaultState): Promise<OverlayRef> {
-    return this._createPaneElement().then(pane => this._createOverlayRef(pane, state));
+  create(state: OverlayState = defaultState): OverlayRef {
+    return this._createOverlayRef(this._createPaneElement(), state);
   }
 
   /**
@@ -52,14 +52,14 @@ export class Overlay {
    * Creates the DOM element for an overlay and appends it to the overlay container.
    * @returns Promise resolving to the created element.
    */
-  private _createPaneElement(): Promise<HTMLElement> {
+  private _createPaneElement(): HTMLElement {
     var pane = document.createElement('div');
     pane.id = `md-overlay-${nextUniqueId++}`;
     pane.classList.add('md-overlay-pane');
 
     this._overlayContainer.getContainerElement().appendChild(pane);
 
-    return Promise.resolve(pane);
+    return pane;
   }
 
   /**

src/lib/core/portal/dom-portal-host.ts

@@ -17,7 +17,7 @@ export class DomPortalHost extends BasePortalHost {
   }
 
   /** Attach the given ComponentPortal to DOM element using the ComponentFactoryResolver. */
-  attachComponentPortal<T>(portal: ComponentPortal<T>): Promise<ComponentRef<T>> {
+  attachComponentPortal<T>(portal: ComponentPortal<T>): ComponentRef<T> {
     if (portal.viewContainerRef == null) {
       throw new MdComponentPortalAttachedToDomWithoutOriginError();
     }
@@ -32,10 +32,10 @@ export class DomPortalHost extends BasePortalHost {
     this._hostDomElement.appendChild(hostView.rootNodes[0]);
     this.setDisposeFn(() => ref.destroy());
 
-    return Promise.resolve(ref);
+    return ref;
   }
 
-  attachTemplatePortal(portal: TemplatePortal): Promise<Map<string, any>> {
+  attachTemplatePortal(portal: TemplatePortal): Map<string, any> {
     let viewContainer = portal.viewContainerRef;
     let viewRef = viewContainer.createEmbeddedView(portal.templateRef);
 
@@ -49,7 +49,7 @@ export class DomPortalHost extends BasePortalHost {
     }));
 
     // TODO(jelbourn): Return locals from view.
-    return Promise.resolve(new Map<string, any>());
+    return new Map<string, any>();
   }
 
   dispose(): void {

src/lib/core/portal/portal-directives.ts

@@ -59,7 +59,7 @@ export class PortalHostDirective extends BasePortalHost {
   }
 
   /** Attach the given ComponentPortal to this PortlHost using the ComponentFactoryResolver. */
-  attachComponentPortal<T>(portal: ComponentPortal<T>): Promise<ComponentRef<T>> {
+  attachComponentPortal<T>(portal: ComponentPortal<T>): ComponentRef<T> {
     portal.setAttachedHost(this);
 
     // If the portal specifies an origin, use that as the logical location of the component
@@ -75,30 +75,30 @@ export class PortalHostDirective extends BasePortalHost {
         portal.injector || viewContainerRef.parentInjector);
 
     this.setDisposeFn(() => ref.destroy());
-    return Promise.resolve(ref);
+    return ref;
   }
 
   /** Attach the given TemplatePortal to this PortlHost as an embedded View. */
-  attachTemplatePortal(portal: TemplatePortal): Promise<Map<string, any>> {
+  attachTemplatePortal(portal: TemplatePortal): Map<string, any> {
     portal.setAttachedHost(this);
 
     this._viewContainerRef.createEmbeddedView(portal.templateRef);
     this.setDisposeFn(() => this._viewContainerRef.clear());
 
     // TODO(jelbourn): return locals from view
-    return Promise.resolve(new Map<string, any>());
+    return new Map<string, any>();
   }
 
   /** Detatches the currently attached Portal (if there is one) and attaches the given Portal. */
   private _replaceAttachedPortal(p: Portal<any>): void {
-    let maybeDetach = this.hasAttached() ? this.detach() : Promise.resolve(null);
-
-    maybeDetach.then(() => {
-      if (p) {
-        this.attach(p);
-        this._portal = p;
-      }
-    });
+    if (this.hasAttached()) {
+      this.detach();
+    }
+
+    if (p) {
+      this.attach(p);
+      this._portal = p;
+    }
   }
 }
 

src/lib/core/portal/portal.spec.ts

@@ -186,10 +186,7 @@ describe('Portals', () => {
     it('should attach and detach a component portal', fakeAsync(() => {
       let portal = new ComponentPortal(PizzaMsg, someViewContainerRef);
 
-      let componentInstance: PizzaMsg;
-      portal.attach(host).then(ref => {
-        componentInstance = ref.instance;
-      });
+      let componentInstance: PizzaMsg = portal.attach(host).instance;
 
       flushMicrotasks();
 
@@ -210,10 +207,7 @@ describe('Portals', () => {
       let chocolateInjector = new ChocolateInjector(someInjector);
       let portal = new ComponentPortal(PizzaMsg, someViewContainerRef, chocolateInjector);
 
-      let componentInstance: PizzaMsg;
-      portal.attach(host).then(ref => {
-        componentInstance = ref.instance;
-      });
+      let componentInstance: PizzaMsg = portal.attach(host).instance;
 
       flushMicrotasks();
       fixture.detectChanges();