From f59030e4dad974fcf1a3066667ac1ce931aa312c Mon Sep 17 00:00:00 2001 From: Jeremy Elbourn Date: Thu, 3 Nov 2016 15:51:01 -0700 Subject: [PATCH] feat(dialog): don't require a ViewContainerRef (#1704) --- src/demo-app/dialog/dialog-demo.ts | 10 ++-- src/lib/core/overlay/overlay.ts | 13 +++-- src/lib/core/portal/dom-portal-host.ts | 73 ++++++++++++++++++++------ src/lib/core/portal/portal-errors.ts | 9 ---- src/lib/core/portal/portal.spec.ts | 38 ++++++++++++-- src/lib/dialog/dialog-config.ts | 4 +- src/lib/dialog/dialog.spec.ts | 14 +++++ src/lib/dialog/dialog.ts | 5 +- 8 files changed, 121 insertions(+), 45 deletions(-) diff --git a/src/demo-app/dialog/dialog-demo.ts b/src/demo-app/dialog/dialog-demo.ts index 6fb4e994c7ef..1cb56c988b3f 100644 --- a/src/demo-app/dialog/dialog-demo.ts +++ b/src/demo-app/dialog/dialog-demo.ts @@ -1,5 +1,5 @@ import {Component, ViewContainerRef} from '@angular/core'; -import {MdDialog, MdDialogConfig, MdDialogRef} from '@angular/material'; +import {MdDialog, MdDialogRef} from '@angular/material'; @Component({ moduleId: module.id, @@ -16,10 +16,7 @@ export class DialogDemo { public viewContainerRef: ViewContainerRef) { } open() { - let config = new MdDialogConfig(); - config.viewContainerRef = this.viewContainerRef; - - this.dialogRef = this.dialog.open(JazzDialog, config); + this.dialogRef = this.dialog.open(JazzDialog); this.dialogRef.afterClosed().subscribe(result => { this.lastCloseResult = result; @@ -34,8 +31,11 @@ export class DialogDemo { template: `

It's Jazz!

+

{{ jazzMessage }}

` }) export class JazzDialog { + jazzMessage = 'Jazzy jazz jazz'; + constructor(public dialogRef: MdDialogRef) { } } diff --git a/src/lib/core/overlay/overlay.ts b/src/lib/core/overlay/overlay.ts index 236a5c258c88..00b3dae201b8 100644 --- a/src/lib/core/overlay/overlay.ts +++ b/src/lib/core/overlay/overlay.ts @@ -1,7 +1,4 @@ -import { - ComponentFactoryResolver, - Injectable, -} from '@angular/core'; +import {ComponentFactoryResolver, Injectable, ApplicationRef, Injector} from '@angular/core'; import {OverlayState} from './overlay-state'; import {DomPortalHost} from '../portal/dom-portal-host'; import {OverlayRef} from './overlay-ref'; @@ -29,7 +26,9 @@ let defaultState = new OverlayState(); export class Overlay { constructor(private _overlayContainer: OverlayContainer, private _componentFactoryResolver: ComponentFactoryResolver, - private _positionBuilder: OverlayPositionBuilder) {} + private _positionBuilder: OverlayPositionBuilder, + private _appRef: ApplicationRef, + private _injector: Injector) {} /** * Creates an overlay. @@ -53,7 +52,7 @@ export class Overlay { * @returns Promise resolving to the created element. */ private _createPaneElement(): HTMLElement { - var pane = document.createElement('div'); + let pane = document.createElement('div'); pane.id = `md-overlay-${nextUniqueId++}`; pane.classList.add('md-overlay-pane'); @@ -68,7 +67,7 @@ export class Overlay { * @returns A portal host for the given DOM element. */ private _createPortalHost(pane: HTMLElement): DomPortalHost { - return new DomPortalHost(pane, this._componentFactoryResolver); + return new DomPortalHost(pane, this._componentFactoryResolver, this._appRef, this._injector); } /** diff --git a/src/lib/core/portal/dom-portal-host.ts b/src/lib/core/portal/dom-portal-host.ts index 413f2ee8e309..2a0acab45a98 100644 --- a/src/lib/core/portal/dom-portal-host.ts +++ b/src/lib/core/portal/dom-portal-host.ts @@ -1,6 +1,11 @@ -import {ComponentFactoryResolver, ComponentRef, EmbeddedViewRef} from '@angular/core'; +import { + ComponentFactoryResolver, + ComponentRef, + EmbeddedViewRef, + ApplicationRef, + Injector, +} from '@angular/core'; import {BasePortalHost, ComponentPortal, TemplatePortal} from './portal'; -import {MdComponentPortalAttachedToDomWithoutOriginError} from './portal-errors'; /** @@ -12,27 +17,60 @@ import {MdComponentPortalAttachedToDomWithoutOriginError} from './portal-errors' export class DomPortalHost extends BasePortalHost { constructor( private _hostDomElement: Element, - private _componentFactoryResolver: ComponentFactoryResolver) { + private _componentFactoryResolver: ComponentFactoryResolver, + private _appRef: ApplicationRef, + private _defaultInjector: Injector) { super(); } /** Attach the given ComponentPortal to DOM element using the ComponentFactoryResolver. */ attachComponentPortal(portal: ComponentPortal): ComponentRef { - if (portal.viewContainerRef == null) { - throw new MdComponentPortalAttachedToDomWithoutOriginError(); - } - let componentFactory = this._componentFactoryResolver.resolveComponentFactory(portal.component); - let ref = portal.viewContainerRef.createComponent( - componentFactory, - portal.viewContainerRef.length, - portal.injector || portal.viewContainerRef.parentInjector); + let componentRef: ComponentRef; + + // If the portal specifies a ViewContainerRef, we will use that as the attachment point + // for the component (in terms of Angular's component tree, not rendering). + // When the ViewContainerRef is missing, we use the factory to create the component directly + // and then manually attach the ChangeDetector for that component to the application (which + // happens automatically when using a ViewContainer). + if (portal.viewContainerRef) { + componentRef = portal.viewContainerRef.createComponent( + componentFactory, + portal.viewContainerRef.length, + portal.injector || portal.viewContainerRef.parentInjector); + + this.setDisposeFn(() => componentRef.destroy()); + } else { + componentRef = componentFactory.create(portal.injector || this._defaultInjector); + + // When creating a component outside of a ViewContainer, we need to manually register + // its ChangeDetector with the application. This API is unfortunately not yet published + // in Angular core. The change detector must also be deregistered when the component + // is destroyed to prevent memory leaks. + // + // See https://github.com/angular/angular/pull/12674 + let changeDetectorRef = componentRef.changeDetectorRef; + (this._appRef as any).registerChangeDetector(changeDetectorRef); + + this.setDisposeFn(() => { + (this._appRef as any).unregisterChangeDetector(changeDetectorRef); - let hostView = > ref.hostView; - this._hostDomElement.appendChild(hostView.rootNodes[0]); - this.setDisposeFn(() => ref.destroy()); + // Normally the ViewContainer will remove the component's nodes from the DOM. + // Without a ViewContainer, we need to manually remove the nodes. + let componentRootNode = this._getComponentRootNode(componentRef); + if (componentRootNode.parentNode) { + componentRootNode.parentNode.removeChild(componentRootNode); + } - return ref; + componentRef.destroy(); + }); + } + + // At this point the component has been instantiated, so we move it to the location in the DOM + // where we want it to be rendered. + this._hostDomElement.appendChild(this._getComponentRootNode(componentRef)); + + return componentRef; } attachTemplatePortal(portal: TemplatePortal): Map { @@ -58,4 +96,9 @@ export class DomPortalHost extends BasePortalHost { this._hostDomElement.parentNode.removeChild(this._hostDomElement); } } + + /** Gets the root HTMLElement for an instantiated component. */ + private _getComponentRootNode(componentRef: ComponentRef): HTMLElement { + return (componentRef.hostView as EmbeddedViewRef).rootNodes[0] as HTMLElement; + } } diff --git a/src/lib/core/portal/portal-errors.ts b/src/lib/core/portal/portal-errors.ts index 5ef00508cc5e..e5617fc4fc4e 100644 --- a/src/lib/core/portal/portal-errors.ts +++ b/src/lib/core/portal/portal-errors.ts @@ -1,14 +1,5 @@ import {MdError} from '../errors/error'; -/** Exception thrown when a ComponentPortal is attached to a DomPortalHost without an origin. */ -export class MdComponentPortalAttachedToDomWithoutOriginError extends MdError { - constructor() { - super( - 'A ComponentPortal must have an origin set when attached to a DomPortalHost ' + - 'because the DOM element is not part of the Angular application context.'); - } -} - /** Exception thrown when attempting to attach a null portal to a host. */ export class MdNullPortalError extends MdError { constructor() { diff --git a/src/lib/core/portal/portal.spec.ts b/src/lib/core/portal/portal.spec.ts index 291a9e36f6b0..6f9c176f3c8a 100644 --- a/src/lib/core/portal/portal.spec.ts +++ b/src/lib/core/portal/portal.spec.ts @@ -8,6 +8,7 @@ import { ComponentFactoryResolver, Optional, Injector, + ApplicationRef, } from '@angular/core'; import {TemplatePortalDirective, PortalModule} from './portal-directives'; import {Portal, ComponentPortal} from './portal'; @@ -134,20 +135,26 @@ describe('Portals', () => { let componentFactoryResolver: ComponentFactoryResolver; let someViewContainerRef: ViewContainerRef; let someInjector: Injector; + let someFixture: ComponentFixture; let someDomElement: HTMLElement; let host: DomPortalHost; + let injector: Injector; + let appRef: ApplicationRef; - beforeEach(inject([ComponentFactoryResolver], (dcl: ComponentFactoryResolver) => { + let deps = [ComponentFactoryResolver, Injector, ApplicationRef]; + beforeEach(inject(deps, (dcl: ComponentFactoryResolver, i: Injector, ar: ApplicationRef) => { componentFactoryResolver = dcl; + injector = i; + appRef = ar; })); beforeEach(() => { someDomElement = document.createElement('div'); - host = new DomPortalHost(someDomElement, componentFactoryResolver); + host = new DomPortalHost(someDomElement, componentFactoryResolver, appRef, injector); - let fixture = TestBed.createComponent(ArbitraryViewContainerRefComponent); - someViewContainerRef = fixture.componentInstance.viewContainerRef; - someInjector = fixture.componentInstance.injector; + someFixture = TestBed.createComponent(ArbitraryViewContainerRefComponent); + someViewContainerRef = someFixture.componentInstance.viewContainerRef; + someInjector = someFixture.componentInstance.injector; }); it('should attach and detach a component portal', () => { @@ -238,6 +245,27 @@ describe('Portals', () => { expect(someDomElement.textContent).toContain('Pizza'); }); + + it('should attach and detach a component portal without a ViewContainerRef', () => { + let portal = new ComponentPortal(PizzaMsg); + + let componentInstance: PizzaMsg = portal.attach(host).instance; + + expect(componentInstance) + .toEqual(jasmine.any(PizzaMsg), 'Expected a PizzaMsg component to be created'); + expect(someDomElement.textContent) + .toContain('Pizza', 'Expected the static string "Pizza" in the DomPortalHost.'); + + componentInstance.snack = new Chocolate(); + someFixture.detectChanges(); + expect(someDomElement.textContent) + .toContain('Chocolate', 'Expected the bound string "Chocolate" in the DomPortalHost'); + + host.detach(); + + expect(someDomElement.innerHTML) + .toBe('', 'Expected the DomPortalHost to be empty after detach'); + }); }); }); diff --git a/src/lib/dialog/dialog-config.ts b/src/lib/dialog/dialog-config.ts index fa6d74fb6e26..155f2a665bdf 100644 --- a/src/lib/dialog/dialog-config.ts +++ b/src/lib/dialog/dialog-config.ts @@ -9,10 +9,10 @@ export type DialogRole = 'dialog' | 'alertdialog' * Configuration for opening a modal dialog with the MdDialog service. */ export class MdDialogConfig { - viewContainerRef: ViewContainerRef; + viewContainerRef?: ViewContainerRef; /** The ARIA role of the dialog element. */ - role: DialogRole = 'dialog'; + role?: DialogRole = 'dialog'; // TODO(jelbourn): add configuration for size, clickOutsideToClose, lifecycle hooks, // ARIA labelling. diff --git a/src/lib/dialog/dialog.spec.ts b/src/lib/dialog/dialog.spec.ts index 6ff5b8651ee8..133cab843674 100644 --- a/src/lib/dialog/dialog.spec.ts +++ b/src/lib/dialog/dialog.spec.ts @@ -64,6 +64,20 @@ describe('MdDialog', () => { expect(dialogContainerElement.getAttribute('role')).toBe('dialog'); }); + it('should open a dialog with a component and no ViewContainerRef', () => { + let dialogRef = dialog.open(PizzaMsg); + + viewContainerFixture.detectChanges(); + + 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', () => { let config = new MdDialogConfig(); config.viewContainerRef = testViewContainerRef; diff --git a/src/lib/dialog/dialog.ts b/src/lib/dialog/dialog.ts index 05384daecc78..59c3dbdcbbdf 100644 --- a/src/lib/dialog/dialog.ts +++ b/src/lib/dialog/dialog.ts @@ -40,7 +40,7 @@ export class MdDialog { * @param component Type of the component to load into the load. * @param config */ - open(component: ComponentType, config: MdDialogConfig): MdDialogRef { + open(component: ComponentType, config = new MdDialogConfig()): MdDialogRef { let overlayRef = this._createOverlay(config); let dialogContainer = this._attachDialogContainer(overlayRef, config); @@ -64,7 +64,8 @@ export class MdDialog { * @returns A promise resolving to a ComponentRef for the attached container. */ private _attachDialogContainer(overlay: OverlayRef, config: MdDialogConfig): MdDialogContainer { - let containerPortal = new ComponentPortal(MdDialogContainer, config.viewContainerRef); + let viewContainer = config ? config.viewContainerRef : null; + let containerPortal = new ComponentPortal(MdDialogContainer, viewContainer); let containerRef: ComponentRef = overlay.attach(containerPortal); containerRef.instance.dialogConfig = config;