From cdad90b773de0b7eb457f7160ad170bed1d35f73 Mon Sep 17 00:00:00 2001 From: Jeremy Elbourn Date: Fri, 19 Aug 2016 13:45:32 -0700 Subject: [PATCH] feat(overlay): make overlays synchronous (#1079) --- src/demo-app/dialog/dialog-demo.ts | 10 ++- src/demo-app/overlay/overlay-demo.ts | 15 ++-- src/lib/core/overlay/overlay-directives.ts | 6 +- src/lib/core/overlay/overlay-ref.ts | 13 ++-- src/lib/core/overlay/overlay.spec.ts | 70 ++++++------------ src/lib/core/overlay/overlay.ts | 8 +-- src/lib/core/portal/dom-portal-host.ts | 8 +-- src/lib/core/portal/portal-directives.ts | 24 +++---- src/lib/core/portal/portal.spec.ts | 10 +-- src/lib/core/portal/portal.ts | 24 +++---- src/lib/dialog/dialog-container.ts | 57 +++------------ src/lib/dialog/dialog.spec.ts | 83 +++++++++------------- src/lib/dialog/dialog.ts | 44 ++++++------ src/lib/menu/menu-trigger.ts | 31 ++++---- src/lib/tooltip/tooltip.spec.ts | 42 ++--------- src/lib/tooltip/tooltip.ts | 33 ++++----- 16 files changed, 167 insertions(+), 311 deletions(-) diff --git a/src/demo-app/dialog/dialog-demo.ts b/src/demo-app/dialog/dialog-demo.ts index a52b699cfde4..ac81c05091c5 100644 --- a/src/demo-app/dialog/dialog-demo.ts +++ b/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; }); } } diff --git a/src/demo-app/overlay/overlay-demo.ts b/src/demo-app/overlay/overlay-demo.ts index 9ef1ed561636..a3e2476d9459 100644 --- a/src/demo-app/overlay/overlay-demo.ts +++ b/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)); } } diff --git a/src/lib/core/overlay/overlay-directives.ts b/src/lib/core/overlay/overlay-directives.ts index c2233a0e65f2..b3926fc24533 100644 --- a/src/lib/core/overlay/overlay-directives.ts +++ b/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. */ diff --git a/src/lib/core/overlay/overlay-ref.ts b/src/lib/core/overlay/overlay-ref.ts index 5db00dbe0a77..0485083c52d9 100644 --- a/src/lib/core/overlay/overlay-ref.ts +++ b/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): Promise { - let attachPromise = this._portalHost.attach(portal); + attach(portal: Portal): 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 { diff --git a/src/lib/core/overlay/overlay.spec.ts b/src/lib/core/overlay/overlay.spec.ts index 961289f342fd..fc94b5b435f1 100644 --- a/src/lib/core/overlay/overlay.spec.ts +++ b/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); - })); + }); }); }); diff --git a/src/lib/core/overlay/overlay.ts b/src/lib/core/overlay/overlay.ts index 692d283bdc3c..0add059bd1da 100644 --- a/src/lib/core/overlay/overlay.ts +++ b/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 { - 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 { + 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; } /** diff --git a/src/lib/core/portal/dom-portal-host.ts b/src/lib/core/portal/dom-portal-host.ts index a4606cec7af9..413f2ee8e309 100644 --- a/src/lib/core/portal/dom-portal-host.ts +++ b/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(portal: ComponentPortal): Promise> { + attachComponentPortal(portal: ComponentPortal): ComponentRef { 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> { + attachTemplatePortal(portal: TemplatePortal): Map { 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()); + return new Map(); } dispose(): void { diff --git a/src/lib/core/portal/portal-directives.ts b/src/lib/core/portal/portal-directives.ts index eb6e45cfafe2..9f2fc42df004 100644 --- a/src/lib/core/portal/portal-directives.ts +++ b/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(portal: ComponentPortal): Promise> { + attachComponentPortal(portal: ComponentPortal): ComponentRef { 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> { + attachTemplatePortal(portal: TemplatePortal): Map { portal.setAttachedHost(this); this._viewContainerRef.createEmbeddedView(portal.templateRef); this.setDisposeFn(() => this._viewContainerRef.clear()); // TODO(jelbourn): return locals from view - return Promise.resolve(new Map()); + return new Map(); } /** Detatches the currently attached Portal (if there is one) and attaches the given Portal. */ private _replaceAttachedPortal(p: Portal): 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; + } } } diff --git a/src/lib/core/portal/portal.spec.ts b/src/lib/core/portal/portal.spec.ts index e8f0e919f3e3..0642d58dd672 100644 --- a/src/lib/core/portal/portal.spec.ts +++ b/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(); diff --git a/src/lib/core/portal/portal.ts b/src/lib/core/portal/portal.ts index 7317fa33321d..b75a69369135 100644 --- a/src/lib/core/portal/portal.ts +++ b/src/lib/core/portal/portal.ts @@ -25,7 +25,7 @@ export abstract class Portal { private _attachedHost: PortalHost; /** Attach this portal to a host. */ - attach(host: PortalHost): Promise { + attach(host: PortalHost): T { if (host == null) { throw new MdNullPortalHostError(); } @@ -35,11 +35,11 @@ export abstract class Portal { } this._attachedHost = host; - return > host.attach(this); + return host.attach(this); } /** Detach this portal from its host */ - detach(): Promise { + detach(): void { let host = this._attachedHost; if (host == null) { throw new MdNoPortalAttachedError(); @@ -121,12 +121,12 @@ export class TemplatePortal extends Portal> { return this.templateRef.elementRef; } - attach(host: PortalHost, locals?: Map): Promise> { + attach(host: PortalHost, locals?: Map): Map { this.locals = locals == null ? new Map() : locals; return super.attach(host); } - detach(): Promise { + detach(): void { this.locals = new Map(); return super.detach(); } @@ -137,9 +137,9 @@ export class TemplatePortal extends Portal> { * A `PortalHost` is an space that can contain a single `Portal`. */ export interface PortalHost { - attach(portal: Portal): Promise; + attach(portal: Portal): any; - detach(): Promise; + detach(): any; dispose(): void; @@ -166,7 +166,7 @@ export abstract class BasePortalHost implements PortalHost { return this._attachedPortal != null; } - attach(portal: Portal): Promise { + attach(portal: Portal): any { if (portal == null) { throw new MdNullPortalError(); } @@ -190,11 +190,11 @@ export abstract class BasePortalHost implements PortalHost { throw new MdUnknownPortalTypeError(); } - abstract attachComponentPortal(portal: ComponentPortal): Promise>; + abstract attachComponentPortal(portal: ComponentPortal): ComponentRef; - abstract attachTemplatePortal(portal: TemplatePortal): Promise>; + abstract attachTemplatePortal(portal: TemplatePortal): Map; - detach(): Promise { + detach(): void { if (this._attachedPortal) { this._attachedPortal.setAttachedHost(null); } this._attachedPortal = null; @@ -202,8 +202,6 @@ export abstract class BasePortalHost implements PortalHost { this._disposeFn(); this._disposeFn = null; } - - return Promise.resolve(null); } dispose() { diff --git a/src/lib/dialog/dialog-container.ts b/src/lib/dialog/dialog-container.ts index 49e7de09da8b..53237bd8fc9a 100644 --- a/src/lib/dialog/dialog-container.ts +++ b/src/lib/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'; @@ -22,63 +21,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>; - - /** Portal to be attached upon AfterViewInit. */ - private _deferredAttachPortal: ComponentPortal; - /** 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(portal: ComponentPortal): Promise> { - 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(portal: ComponentPortal): ComponentRef { + if (this._portalHost.hasAttached()) { + throw new MdDialogContentAlreadyAttachedError(); } + + return this._portalHost.attachComponentPortal(portal); } - attachTemplatePortal(portal: TemplatePortal): Promise> { + attachTemplatePortal(portal: TemplatePortal): Map { throw Error('Not yet implemented'); } } diff --git a/src/lib/dialog/dialog.spec.ts b/src/lib/dialog/dialog.spec.ts index 9e2bb2131907..22f80ff2e67f 100644 --- a/src/lib/dialog/dialog.spec.ts +++ b/src/lib/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) { - // 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: '

Pizza

', -}) +@Component({template: '

Pizza

'}) class PizzaMsg { constructor(public dialogRef: MdDialogRef) { } } diff --git a/src/lib/dialog/dialog.ts b/src/lib/dialog/dialog.ts index e27b6edeca86..18cbaad02409 100644 --- a/src/lib/dialog/dialog.ts +++ b/src/lib/dialog/dialog.ts @@ -39,13 +39,11 @@ export class MdDialog { * @param component Type of the component to load into the load. * @param config */ - open(component: ComponentType, config: MdDialogConfig): Promise> { - let overlayRef: OverlayRef; + open(component: ComponentType, config: MdDialogConfig): MdDialogRef { + 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)); + return this._attachDialogContent(component, dialogContainer, overlayRef); } /** @@ -53,7 +51,7 @@ export class MdDialog { * @param dialogConfig The dialog configuration. * @returns A promise resolving to the OverlayRef for the created overlay. */ - private _createOverlay(dialogConfig: MdDialogConfig): Promise { + private _createOverlay(dialogConfig: MdDialogConfig): OverlayRef { let overlayState = this._getOverlayState(dialogConfig); return this._overlay.create(overlayState); } @@ -64,32 +62,29 @@ 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> { + private _attachDialogContainer(overlay: OverlayRef, config: MdDialogConfig): MdDialogContainer { let containerPortal = new ComponentPortal(MdDialogContainer, config.viewContainerRef); - return overlay.attach(containerPortal).then((containerRef: ComponentRef) => { - // Pass the config directly to the container so that it can consume any relevant settings. - containerRef.instance.dialogConfig = config; - return containerRef; - }); + + let containerRef: ComponentRef = 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( component: ComponentType, - containerRef: ComponentRef, - overlayRef: OverlayRef): Promise> { - let dialogContainer = containerRef.instance; - + dialogContainer: MdDialogContainer, + overlayRef: OverlayRef): MdDialogRef { // 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 = > 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 @@ -97,10 +92,11 @@ export class MdDialog { 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; } /** diff --git a/src/lib/menu/menu-trigger.ts b/src/lib/menu/menu-trigger.ts index 0ed91cc974ca..13cbc178496f 100644 --- a/src/lib/menu/menu-trigger.ts +++ b/src/lib/menu/menu-trigger.ts @@ -58,21 +58,21 @@ export class MdMenuTrigger implements AfterViewInit, OnDestroy { get menuOpen(): boolean { return this._menuOpen; } @HostListener('click') - toggleMenu(): Promise { + toggleMenu(): void { return this._menuOpen ? this.closeMenu() : this.openMenu(); } - openMenu(): Promise { - 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 { - 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 { - 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()); + } } /** diff --git a/src/lib/tooltip/tooltip.spec.ts b/src/lib/tooltip/tooltip.spec.ts index ccb4be3255e3..e34e1a54626d 100644 --- a/src/lib/tooltip/tooltip.spec.ts +++ b/src/lib/tooltip/tooltip.spec.ts @@ -30,54 +30,26 @@ describe('MdTooltip', () => { let buttonElement: HTMLButtonElement; let tooltipDirective: MdTooltip; - beforeEach(async(() => { + beforeEach(() => { fixture = TestBed.createComponent(BasicTooltipDemo); fixture.detectChanges(); buttonDebugElement = fixture.debugElement.query(By.css('button')); buttonElement = buttonDebugElement.nativeElement; tooltipDirective = buttonDebugElement.injector.get(MdTooltip); - })); + }); - it('should show/hide on mouse enter/leave', async(() => { + it('should show/hide on mouse enter/leave', () => { expect(tooltipDirective.visible).toBeFalsy(); tooltipDirective._handleMouseEnter(null); expect(tooltipDirective.visible).toBeTruthy(); fixture.detectChanges(); - whenStable([ - () => { - expect(overlayContainerElement.textContent).toBe('some message'); - tooltipDirective._handleMouseLeave(null); - }, - () => { - expect(overlayContainerElement.textContent).toBe(''); - } - ]); - })); + expect(overlayContainerElement.textContent).toBe('some message'); - /** - * Utility function to make it easier to use multiple `whenStable` checks. - * Accepts an array of callbacks, each to wait for stability before running. - * TODO: Remove the `setTimeout()` when a viable alternative is available - * @param callbacks - */ - function whenStable(callbacks: Array) { - if (callbacks.length) { - fixture.detectChanges(); - fixture.whenStable().then(() => { - // 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 tooltip, - // and another to cause the lifecycle events of the tooltip to run and load the tooltip - // content. - setTimeout(() => { - callbacks[0](); - whenStable(callbacks.slice(1)); - }, 50); - }); - } - } + tooltipDirective._handleMouseLeave(null); + expect(overlayContainerElement.textContent).toBe(''); + }); }); }); diff --git a/src/lib/tooltip/tooltip.ts b/src/lib/tooltip/tooltip.ts index d59e8c7dac0e..552783276806 100644 --- a/src/lib/tooltip/tooltip.ts +++ b/src/lib/tooltip/tooltip.ts @@ -1,7 +1,6 @@ import { NgModule, Component, - ComponentRef, Directive, Input, ElementRef, @@ -74,7 +73,8 @@ export class MdTooltip { if (this._overlayRef) { if (this.visible) { // if visible, hide before destroying - this.hide().then(() => this._createOverlay()); + this.hide(); + this._createOverlay(); } else { // if not visible, dispose and recreate this._overlayRef.dispose(); @@ -87,9 +87,8 @@ export class MdTooltip { let strategy = this._overlay.position().connectedTo(this._elementRef, origin, position); let config = new OverlayState(); config.positionStrategy = strategy; - this._overlay.create(config).then(ref => { - this._overlayRef = ref; - }); + + this._overlayRef = this._overlay.create(config); } } @@ -136,37 +135,35 @@ export class MdTooltip { /** * Shows the tooltip and returns a promise that will resolve when the tooltip is visible */ - show(): Promise { + show(): void { if (!this.visible && this._overlayRef && !this._overlayRef.hasAttached()) { this.visible = true; - let promise = this._overlayRef.attach(new ComponentPortal(TooltipComponent, - this._viewContainerRef)); - promise.then((ref: ComponentRef) => { - ref.instance.message = this.message; - this._updatePosition(); - }); - return promise; + + let portal = new ComponentPortal(TooltipComponent, this._viewContainerRef); + let tooltipRef = this._overlayRef.attach(portal); + tooltipRef.instance.message = this.message; + this._updatePosition(); } } /** * Hides the tooltip and returns a promise that will resolve when the tooltip is hidden */ - hide(): Promise { + hide(): void { if (this.visible && this._overlayRef && this._overlayRef.hasAttached()) { this.visible = false; - return this._overlayRef.detach(); + this._overlayRef.detach(); } } /** * Shows/hides the tooltip and returns a promise that will resolve when it is done */ - toggle(): Promise { + toggle(): void { if (this.visible) { - return this.hide(); + this.hide(); } else { - return this.show(); + this.show(); } }