Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(dialog): don't require a ViewContainerRef #1704

Merged
merged 1 commit into from
Nov 3, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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