Skip to content

Commit f59030e

Browse files
authored
feat(dialog): don't require a ViewContainerRef (#1704)
1 parent 1e86066 commit f59030e

File tree

8 files changed

+121
-45
lines changed

8 files changed

+121
-45
lines changed

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

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import {Component, ViewContainerRef} from '@angular/core';
2-
import {MdDialog, MdDialogConfig, MdDialogRef} from '@angular/material';
2+
import {MdDialog, MdDialogRef} from '@angular/material';
33

44
@Component({
55
moduleId: module.id,
@@ -16,10 +16,7 @@ export class DialogDemo {
1616
public viewContainerRef: ViewContainerRef) { }
1717

1818
open() {
19-
let config = new MdDialogConfig();
20-
config.viewContainerRef = this.viewContainerRef;
21-
22-
this.dialogRef = this.dialog.open(JazzDialog, config);
19+
this.dialogRef = this.dialog.open(JazzDialog);
2320

2421
this.dialogRef.afterClosed().subscribe(result => {
2522
this.lastCloseResult = result;
@@ -34,8 +31,11 @@ export class DialogDemo {
3431
template: `
3532
<p>It's Jazz!</p>
3633
<p><label>How much? <input #howMuch></label></p>
34+
<p> {{ jazzMessage }} </p>
3735
<button type="button" (click)="dialogRef.close(howMuch.value)">Close dialog</button>`
3836
})
3937
export class JazzDialog {
38+
jazzMessage = 'Jazzy jazz jazz';
39+
4040
constructor(public dialogRef: MdDialogRef<JazzDialog>) { }
4141
}

src/lib/core/overlay/overlay.ts

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,4 @@
1-
import {
2-
ComponentFactoryResolver,
3-
Injectable,
4-
} from '@angular/core';
1+
import {ComponentFactoryResolver, Injectable, ApplicationRef, Injector} from '@angular/core';
52
import {OverlayState} from './overlay-state';
63
import {DomPortalHost} from '../portal/dom-portal-host';
74
import {OverlayRef} from './overlay-ref';
@@ -29,7 +26,9 @@ let defaultState = new OverlayState();
2926
export class Overlay {
3027
constructor(private _overlayContainer: OverlayContainer,
3128
private _componentFactoryResolver: ComponentFactoryResolver,
32-
private _positionBuilder: OverlayPositionBuilder) {}
29+
private _positionBuilder: OverlayPositionBuilder,
30+
private _appRef: ApplicationRef,
31+
private _injector: Injector) {}
3332

3433
/**
3534
* Creates an overlay.
@@ -53,7 +52,7 @@ export class Overlay {
5352
* @returns Promise resolving to the created element.
5453
*/
5554
private _createPaneElement(): HTMLElement {
56-
var pane = document.createElement('div');
55+
let pane = document.createElement('div');
5756
pane.id = `md-overlay-${nextUniqueId++}`;
5857
pane.classList.add('md-overlay-pane');
5958

@@ -68,7 +67,7 @@ export class Overlay {
6867
* @returns A portal host for the given DOM element.
6968
*/
7069
private _createPortalHost(pane: HTMLElement): DomPortalHost {
71-
return new DomPortalHost(pane, this._componentFactoryResolver);
70+
return new DomPortalHost(pane, this._componentFactoryResolver, this._appRef, this._injector);
7271
}
7372

7473
/**

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

Lines changed: 58 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,11 @@
1-
import {ComponentFactoryResolver, ComponentRef, EmbeddedViewRef} from '@angular/core';
1+
import {
2+
ComponentFactoryResolver,
3+
ComponentRef,
4+
EmbeddedViewRef,
5+
ApplicationRef,
6+
Injector,
7+
} from '@angular/core';
28
import {BasePortalHost, ComponentPortal, TemplatePortal} from './portal';
3-
import {MdComponentPortalAttachedToDomWithoutOriginError} from './portal-errors';
49

510

611
/**
@@ -12,27 +17,60 @@ import {MdComponentPortalAttachedToDomWithoutOriginError} from './portal-errors'
1217
export class DomPortalHost extends BasePortalHost {
1318
constructor(
1419
private _hostDomElement: Element,
15-
private _componentFactoryResolver: ComponentFactoryResolver) {
20+
private _componentFactoryResolver: ComponentFactoryResolver,
21+
private _appRef: ApplicationRef,
22+
private _defaultInjector: Injector) {
1623
super();
1724
}
1825

1926
/** Attach the given ComponentPortal to DOM element using the ComponentFactoryResolver. */
2027
attachComponentPortal<T>(portal: ComponentPortal<T>): ComponentRef<T> {
21-
if (portal.viewContainerRef == null) {
22-
throw new MdComponentPortalAttachedToDomWithoutOriginError();
23-
}
24-
2528
let componentFactory = this._componentFactoryResolver.resolveComponentFactory(portal.component);
26-
let ref = portal.viewContainerRef.createComponent(
27-
componentFactory,
28-
portal.viewContainerRef.length,
29-
portal.injector || portal.viewContainerRef.parentInjector);
29+
let componentRef: ComponentRef<T>;
30+
31+
// If the portal specifies a ViewContainerRef, we will use that as the attachment point
32+
// for the component (in terms of Angular's component tree, not rendering).
33+
// When the ViewContainerRef is missing, we use the factory to create the component directly
34+
// and then manually attach the ChangeDetector for that component to the application (which
35+
// happens automatically when using a ViewContainer).
36+
if (portal.viewContainerRef) {
37+
componentRef = portal.viewContainerRef.createComponent(
38+
componentFactory,
39+
portal.viewContainerRef.length,
40+
portal.injector || portal.viewContainerRef.parentInjector);
41+
42+
this.setDisposeFn(() => componentRef.destroy());
43+
} else {
44+
componentRef = componentFactory.create(portal.injector || this._defaultInjector);
45+
46+
// When creating a component outside of a ViewContainer, we need to manually register
47+
// its ChangeDetector with the application. This API is unfortunately not yet published
48+
// in Angular core. The change detector must also be deregistered when the component
49+
// is destroyed to prevent memory leaks.
50+
//
51+
// See https://github.com/angular/angular/pull/12674
52+
let changeDetectorRef = componentRef.changeDetectorRef;
53+
(this._appRef as any).registerChangeDetector(changeDetectorRef);
54+
55+
this.setDisposeFn(() => {
56+
(this._appRef as any).unregisterChangeDetector(changeDetectorRef);
3057

31-
let hostView = <EmbeddedViewRef<any>> ref.hostView;
32-
this._hostDomElement.appendChild(hostView.rootNodes[0]);
33-
this.setDisposeFn(() => ref.destroy());
58+
// Normally the ViewContainer will remove the component's nodes from the DOM.
59+
// Without a ViewContainer, we need to manually remove the nodes.
60+
let componentRootNode = this._getComponentRootNode(componentRef);
61+
if (componentRootNode.parentNode) {
62+
componentRootNode.parentNode.removeChild(componentRootNode);
63+
}
3464

35-
return ref;
65+
componentRef.destroy();
66+
});
67+
}
68+
69+
// At this point the component has been instantiated, so we move it to the location in the DOM
70+
// where we want it to be rendered.
71+
this._hostDomElement.appendChild(this._getComponentRootNode(componentRef));
72+
73+
return componentRef;
3674
}
3775

3876
attachTemplatePortal(portal: TemplatePortal): Map<string, any> {
@@ -58,4 +96,9 @@ export class DomPortalHost extends BasePortalHost {
5896
this._hostDomElement.parentNode.removeChild(this._hostDomElement);
5997
}
6098
}
99+
100+
/** Gets the root HTMLElement for an instantiated component. */
101+
private _getComponentRootNode(componentRef: ComponentRef<any>): HTMLElement {
102+
return (componentRef.hostView as EmbeddedViewRef<any>).rootNodes[0] as HTMLElement;
103+
}
61104
}

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

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,5 @@
11
import {MdError} from '../errors/error';
22

3-
/** Exception thrown when a ComponentPortal is attached to a DomPortalHost without an origin. */
4-
export class MdComponentPortalAttachedToDomWithoutOriginError extends MdError {
5-
constructor() {
6-
super(
7-
'A ComponentPortal must have an origin set when attached to a DomPortalHost ' +
8-
'because the DOM element is not part of the Angular application context.');
9-
}
10-
}
11-
123
/** Exception thrown when attempting to attach a null portal to a host. */
134
export class MdNullPortalError extends MdError {
145
constructor() {

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

Lines changed: 33 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import {
88
ComponentFactoryResolver,
99
Optional,
1010
Injector,
11+
ApplicationRef,
1112
} from '@angular/core';
1213
import {TemplatePortalDirective, PortalModule} from './portal-directives';
1314
import {Portal, ComponentPortal} from './portal';
@@ -134,20 +135,26 @@ describe('Portals', () => {
134135
let componentFactoryResolver: ComponentFactoryResolver;
135136
let someViewContainerRef: ViewContainerRef;
136137
let someInjector: Injector;
138+
let someFixture: ComponentFixture<any>;
137139
let someDomElement: HTMLElement;
138140
let host: DomPortalHost;
141+
let injector: Injector;
142+
let appRef: ApplicationRef;
139143

140-
beforeEach(inject([ComponentFactoryResolver], (dcl: ComponentFactoryResolver) => {
144+
let deps = [ComponentFactoryResolver, Injector, ApplicationRef];
145+
beforeEach(inject(deps, (dcl: ComponentFactoryResolver, i: Injector, ar: ApplicationRef) => {
141146
componentFactoryResolver = dcl;
147+
injector = i;
148+
appRef = ar;
142149
}));
143150

144151
beforeEach(() => {
145152
someDomElement = document.createElement('div');
146-
host = new DomPortalHost(someDomElement, componentFactoryResolver);
153+
host = new DomPortalHost(someDomElement, componentFactoryResolver, appRef, injector);
147154

148-
let fixture = TestBed.createComponent(ArbitraryViewContainerRefComponent);
149-
someViewContainerRef = fixture.componentInstance.viewContainerRef;
150-
someInjector = fixture.componentInstance.injector;
155+
someFixture = TestBed.createComponent(ArbitraryViewContainerRefComponent);
156+
someViewContainerRef = someFixture.componentInstance.viewContainerRef;
157+
someInjector = someFixture.componentInstance.injector;
151158
});
152159

153160
it('should attach and detach a component portal', () => {
@@ -238,6 +245,27 @@ describe('Portals', () => {
238245

239246
expect(someDomElement.textContent).toContain('Pizza');
240247
});
248+
249+
it('should attach and detach a component portal without a ViewContainerRef', () => {
250+
let portal = new ComponentPortal(PizzaMsg);
251+
252+
let componentInstance: PizzaMsg = portal.attach(host).instance;
253+
254+
expect(componentInstance)
255+
.toEqual(jasmine.any(PizzaMsg), 'Expected a PizzaMsg component to be created');
256+
expect(someDomElement.textContent)
257+
.toContain('Pizza', 'Expected the static string "Pizza" in the DomPortalHost.');
258+
259+
componentInstance.snack = new Chocolate();
260+
someFixture.detectChanges();
261+
expect(someDomElement.textContent)
262+
.toContain('Chocolate', 'Expected the bound string "Chocolate" in the DomPortalHost');
263+
264+
host.detach();
265+
266+
expect(someDomElement.innerHTML)
267+
.toBe('', 'Expected the DomPortalHost to be empty after detach');
268+
});
241269
});
242270
});
243271

src/lib/dialog/dialog-config.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,10 +9,10 @@ export type DialogRole = 'dialog' | 'alertdialog'
99
* Configuration for opening a modal dialog with the MdDialog service.
1010
*/
1111
export class MdDialogConfig {
12-
viewContainerRef: ViewContainerRef;
12+
viewContainerRef?: ViewContainerRef;
1313

1414
/** The ARIA role of the dialog element. */
15-
role: DialogRole = 'dialog';
15+
role?: DialogRole = 'dialog';
1616

1717
// TODO(jelbourn): add configuration for size, clickOutsideToClose, lifecycle hooks,
1818
// ARIA labelling.

src/lib/dialog/dialog.spec.ts

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,20 @@ describe('MdDialog', () => {
6464
expect(dialogContainerElement.getAttribute('role')).toBe('dialog');
6565
});
6666

67+
it('should open a dialog with a component and no ViewContainerRef', () => {
68+
let dialogRef = dialog.open(PizzaMsg);
69+
70+
viewContainerFixture.detectChanges();
71+
72+
expect(overlayContainerElement.textContent).toContain('Pizza');
73+
expect(dialogRef.componentInstance).toEqual(jasmine.any(PizzaMsg));
74+
expect(dialogRef.componentInstance.dialogRef).toBe(dialogRef);
75+
76+
viewContainerFixture.detectChanges();
77+
let dialogContainerElement = overlayContainerElement.querySelector('md-dialog-container');
78+
expect(dialogContainerElement.getAttribute('role')).toBe('dialog');
79+
});
80+
6781
it('should apply the configured role to the dialog element', () => {
6882
let config = new MdDialogConfig();
6983
config.viewContainerRef = testViewContainerRef;

src/lib/dialog/dialog.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ export class MdDialog {
4040
* @param component Type of the component to load into the load.
4141
* @param config
4242
*/
43-
open<T>(component: ComponentType<T>, config: MdDialogConfig): MdDialogRef<T> {
43+
open<T>(component: ComponentType<T>, config = new MdDialogConfig()): MdDialogRef<T> {
4444
let overlayRef = this._createOverlay(config);
4545
let dialogContainer = this._attachDialogContainer(overlayRef, config);
4646

@@ -64,7 +64,8 @@ export class MdDialog {
6464
* @returns A promise resolving to a ComponentRef for the attached container.
6565
*/
6666
private _attachDialogContainer(overlay: OverlayRef, config: MdDialogConfig): MdDialogContainer {
67-
let containerPortal = new ComponentPortal(MdDialogContainer, config.viewContainerRef);
67+
let viewContainer = config ? config.viewContainerRef : null;
68+
let containerPortal = new ComponentPortal(MdDialogContainer, viewContainer);
6869

6970
let containerRef: ComponentRef<MdDialogContainer> = overlay.attach(containerPortal);
7071
containerRef.instance.dialogConfig = config;

0 commit comments

Comments
 (0)