Skip to content

Commit

Permalink
feat(dialog): don't require a ViewContainerRef (#1704)
Browse files Browse the repository at this point in the history
  • Loading branch information
jelbourn authored Nov 3, 2016
1 parent 1e86066 commit f59030e
Show file tree
Hide file tree
Showing 8 changed files with 121 additions and 45 deletions.
10 changes: 5 additions & 5 deletions src/demo-app/dialog/dialog-demo.ts
Original file line number Diff line number Diff line change
@@ -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,
Expand All @@ -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;
Expand All @@ -34,8 +31,11 @@ export class DialogDemo {
template: `
<p>It's Jazz!</p>
<p><label>How much? <input #howMuch></label></p>
<p> {{ jazzMessage }} </p>
<button type="button" (click)="dialogRef.close(howMuch.value)">Close dialog</button>`
})
export class JazzDialog {
jazzMessage = 'Jazzy jazz jazz';

constructor(public dialogRef: MdDialogRef<JazzDialog>) { }
}
13 changes: 6 additions & 7 deletions src/lib/core/overlay/overlay.ts
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -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.
Expand All @@ -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');

Expand All @@ -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);
}

/**
Expand Down
73 changes: 58 additions & 15 deletions src/lib/core/portal/dom-portal-host.ts
Original file line number Diff line number Diff line change
@@ -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';


/**
Expand All @@ -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<T>(portal: ComponentPortal<T>): ComponentRef<T> {
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<T>;

// 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 = <EmbeddedViewRef<any>> 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<string, any> {
Expand All @@ -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<any>): HTMLElement {
return (componentRef.hostView as EmbeddedViewRef<any>).rootNodes[0] as HTMLElement;
}
}
9 changes: 0 additions & 9 deletions src/lib/core/portal/portal-errors.ts
Original file line number Diff line number Diff line change
@@ -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() {
Expand Down
38 changes: 33 additions & 5 deletions src/lib/core/portal/portal.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import {
ComponentFactoryResolver,
Optional,
Injector,
ApplicationRef,
} from '@angular/core';
import {TemplatePortalDirective, PortalModule} from './portal-directives';
import {Portal, ComponentPortal} from './portal';
Expand Down Expand Up @@ -134,20 +135,26 @@ describe('Portals', () => {
let componentFactoryResolver: ComponentFactoryResolver;
let someViewContainerRef: ViewContainerRef;
let someInjector: Injector;
let someFixture: ComponentFixture<any>;
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', () => {
Expand Down Expand Up @@ -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');
});
});
});

Expand Down
4 changes: 2 additions & 2 deletions src/lib/dialog/dialog-config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
14 changes: 14 additions & 0 deletions src/lib/dialog/dialog.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
5 changes: 3 additions & 2 deletions src/lib/dialog/dialog.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ export class MdDialog {
* @param component Type of the component to load into the load.
* @param config
*/
open<T>(component: ComponentType<T>, config: MdDialogConfig): MdDialogRef<T> {
open<T>(component: ComponentType<T>, config = new MdDialogConfig()): MdDialogRef<T> {
let overlayRef = this._createOverlay(config);
let dialogContainer = this._attachDialogContainer(overlayRef, config);

Expand All @@ -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<MdDialogContainer> = overlay.attach(containerPortal);
containerRef.instance.dialogConfig = config;
Expand Down

0 comments on commit f59030e

Please sign in to comment.