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(portal): support context in TemplatePortal #6408

Merged
merged 4 commits into from
Aug 21, 2017
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
2 changes: 1 addition & 1 deletion src/cdk/overlay/overlay-directives.ts
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ export class OverlayOrigin {
})
export class ConnectedOverlayDirective implements OnDestroy, OnChanges {
private _overlayRef: OverlayRef;
private _templatePortal: TemplatePortal;
private _templatePortal: TemplatePortal<any>;
private _hasBackdrop = false;
private _backdropSubscription: Subscription | null;
private _positionSubscription: Subscription;
Expand Down
2 changes: 1 addition & 1 deletion src/cdk/overlay/overlay.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import {
describe('Overlay', () => {
let overlay: Overlay;
let componentPortal: ComponentPortal<PizzaMsg>;
let templatePortal: TemplatePortal;
let templatePortal: TemplatePortal<any>;
let overlayContainerElement: HTMLElement;
let viewContainerFixture: ComponentFixture<TestComponentWithTemplatePortals>;

Expand Down
6 changes: 3 additions & 3 deletions src/cdk/portal/dom-portal-host.ts
Original file line number Diff line number Diff line change
Expand Up @@ -69,9 +69,9 @@ export class DomPortalHost extends BasePortalHost {
* Attaches a template portal to the DOM as an embedded view.
* @param portal Portal to be attached.
*/
attachTemplatePortal(portal: TemplatePortal): Map<string, any> {
attachTemplatePortal<C>(portal: TemplatePortal<C>): EmbeddedViewRef<C> {
let viewContainer = portal.viewContainerRef;
let viewRef = viewContainer.createEmbeddedView(portal.templateRef);
let viewRef = viewContainer.createEmbeddedView(portal.templateRef, portal.context);
viewRef.detectChanges();

// The method `createEmbeddedView` will add the view as a child of the viewContainer.
Expand All @@ -87,7 +87,7 @@ export class DomPortalHost extends BasePortalHost {
}));

// TODO(jelbourn): Return locals from view.
return new Map<string, any>();
return viewRef;
}

/**
Expand Down
11 changes: 5 additions & 6 deletions src/cdk/portal/portal-directives.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import {
NgModule,
ComponentRef,
Directive,
EmbeddedViewRef,
TemplateRef,
ComponentFactoryResolver,
ViewContainerRef,
Expand All @@ -32,7 +33,7 @@ import {Portal, TemplatePortal, ComponentPortal, BasePortalHost} from './portal'
selector: '[cdk-portal], [cdkPortal], [portal]',
exportAs: 'cdkPortal',
})
export class TemplatePortalDirective extends TemplatePortal {
export class TemplatePortalDirective extends TemplatePortal<any> {
constructor(templateRef: TemplateRef<any>, viewContainerRef: ViewContainerRef) {
super(templateRef, viewContainerRef);
}
Expand Down Expand Up @@ -117,16 +118,14 @@ export class PortalHostDirective extends BasePortalHost implements OnDestroy {
* Attach the given TemplatePortal to this PortlHost as an embedded View.
* @param portal Portal to be attached.
*/
attachTemplatePortal(portal: TemplatePortal): Map<string, any> {
attachTemplatePortal<C>(portal: TemplatePortal<C>): EmbeddedViewRef<C> {
portal.setAttachedHost(this);

this._viewContainerRef.createEmbeddedView(portal.templateRef);
const viewRef = this._viewContainerRef.createEmbeddedView(portal.templateRef, portal.context);
super.setDisposeFn(() => this._viewContainerRef.clear());

this._portal = portal;

// TODO(jelbourn): return locals from view
return new Map<string, any>();
return viewRef;
}
}

Expand Down
65 changes: 61 additions & 4 deletions src/cdk/portal/portal.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,11 @@ import {
Optional,
Injector,
ApplicationRef,
TemplateRef
} from '@angular/core';
import {CommonModule} from '@angular/common';
import {TemplatePortalDirective, PortalHostDirective, PortalModule} from './portal-directives';
import {Portal, ComponentPortal} from './portal';
import {Portal, ComponentPortal, TemplatePortal} from './portal';
import {DomPortalHost} from './dom-portal-host';


Expand Down Expand Up @@ -45,6 +46,57 @@ describe('Portals', () => {
expect(hostContainer.textContent).toContain('Pizza');
});

it('should load a template into the portal', () => {
let testAppComponent = fixture.debugElement.componentInstance;
let hostContainer = fixture.nativeElement.querySelector('.portal-container');

let templatePortal = new TemplatePortal(testAppComponent.templateRef, null!);
testAppComponent.selectedPortal = templatePortal;
fixture.detectChanges();
// Expect that the content of the attached portal is present and no context is projected
expect(hostContainer.textContent).toContain('Banana');
});

it('should project template context bindings in the portal', () => {
let testAppComponent = fixture.debugElement.componentInstance;
let hostContainer = fixture.nativeElement.querySelector('.portal-container');

// TemplatePortal without context:
let templatePortal = new TemplatePortal(testAppComponent.templateRef, null!);
testAppComponent.selectedPortal = templatePortal;
fixture.detectChanges();
// Expect that the content of the attached portal is present and NO context is projected
expect(hostContainer.textContent).toContain('Banana - !');

// using TemplatePortal.attach method to set context
testAppComponent.selectedPortal = undefined;
fixture.detectChanges();
templatePortal.attach(testAppComponent.portalHost, {$implicit: {status: 'rotten'}});
fixture.detectChanges();
// Expect that the content of the attached portal is present and context given via the
// attach method is projected
expect(hostContainer.textContent).toContain('Banana - rotten!');

// using TemplatePortal constructor to set the context
templatePortal =
new TemplatePortal(testAppComponent.templateRef, null!, {$implicit: {status: 'fresh'}});
testAppComponent.selectedPortal = templatePortal;
fixture.detectChanges();
// Expect that the content of the attached portal is present and context given via the
// constructor is projected
expect(hostContainer.textContent).toContain('Banana - fresh!');

// using TemplatePortal constructor to set the context but also calling attach method with
// context, the latter should take precedence:
testAppComponent.selectedPortal = undefined;
fixture.detectChanges();
templatePortal.attach(testAppComponent.portalHost, {$implicit: {status: 'rotten'}});
fixture.detectChanges();
// Expect that the content of the attached portal is present and and context given via the
// attach method is projected and get precedence over constructor context
expect(hostContainer.textContent).toContain('Banana - rotten!');
});

it('should dispose the host when destroyed', () => {
// Set the selectedHost to be a ComponentPortal.
let testAppComponent = fixture.debugElement.componentInstance;
Expand Down Expand Up @@ -299,15 +351,15 @@ describe('Portals', () => {
fixture.detectChanges();

// Attach the TemplatePortal.
testAppComponent.portalWithBinding.attach(host);
testAppComponent.portalWithBinding.attach(host, {$implicit: {status: 'fresh'}});
fixture.detectChanges();

// Now that the portal is attached, change detection has to happen again in order
// for the bindings to update.
fixture.detectChanges();

// Expect that the content of the attached portal is present.
expect(someDomElement.textContent).toContain('Banana');
expect(someDomElement.textContent).toContain('Banana - fresh');

// When updating the binding value.
testAppComponent.fruit = 'Mango';
Expand Down Expand Up @@ -416,18 +468,22 @@ class ArbitraryViewContainerRefComponent {
<ng-template cdk-portal>Cake</ng-template>

<div *cdk-portal>Pie</div>
<ng-template cdk-portal> {{fruit}} </ng-template>
<ng-template cdk-portal let-data> {{fruit}} - {{ data?.status }} </ng-template>

<ng-template cdk-portal>
<ul>
<li *ngFor="let fruitName of fruits"> {{fruitName}} </li>
</ul>
</ng-template>

<ng-template #templateRef let-data> {{fruit}} - {{ data?.status }}!</ng-template>
`,
})
class PortalTestApp {
@ViewChildren(TemplatePortalDirective) portals: QueryList<TemplatePortalDirective>;
@ViewChild(PortalHostDirective) portalHost: PortalHostDirective;
@ViewChild('templateRef', { read: TemplateRef }) templateRef: TemplateRef<any>;

selectedPortal: Portal<any>;
fruit: string = 'Banana';
fruits = ['Apple', 'Pineapple', 'Durian'];
Expand All @@ -449,6 +505,7 @@ class PortalTestApp {
get portalWithTemplate() {
return this.portals.toArray()[3];
}

}

// Create a real (non-test) NgModule as a workaround for
Expand Down
32 changes: 17 additions & 15 deletions src/cdk/portal/portal.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import {
ViewContainerRef,
ElementRef,
ComponentRef,
EmbeddedViewRef,
Injector
} from '@angular/core';
import {
Expand Down Expand Up @@ -103,42 +104,43 @@ export class ComponentPortal<T> extends Portal<ComponentRef<T>> {
}
}


/**
* A `TemplatePortal` is a portal that represents some embedded template (TemplateRef).
*/
export class TemplatePortal extends Portal<Map<string, any>> {
export class TemplatePortal<C> extends Portal<C> {
/** The embedded template that will be used to instantiate an embedded View in the host. */
templateRef: TemplateRef<any>;
templateRef: TemplateRef<C>;

/** Reference to the ViewContainer into which the template will be stamped out. */
viewContainerRef: ViewContainerRef;

/**
* Additional locals for the instantiated embedded view.
* These locals can be seen as "exports" for the template, such as how ngFor has
* index / event / odd.
* See https://angular.io/docs/ts/latest/api/core/EmbeddedViewRef-class.html
*/
locals: Map<string, any> = new Map<string, any>();
context: C | undefined;

constructor(template: TemplateRef<any>, viewContainerRef: ViewContainerRef) {
constructor(template: TemplateRef<any>, viewContainerRef: ViewContainerRef, context?: C) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's somewhat strange that the context can be set either in the constructor or in attach. Is this meant to be a default context when nothing is passed to attach?

Copy link
Contributor Author

@shlomiassaf shlomiassaf Aug 13, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is required to support the abstraction in dialog.ts

If you follow the trail, the attach method is never called and BasePortalHost (PortalHostDirective) handles it from the top via attachTemplatePortal().

Now, adding a context as a param in the method attachTemplatePortal is a mess since it involves changing a lot of classes that extend BasePortal, for example MdDialogContainer

Additionally, changing the signature of attachTemplatePortal to accept a context will make it different from the signature attachComponentPortal...

So yes, there are 2 ways to set the context, one can override the other.

We can remove it from attach, the only effect here is that an extra line of code is required to set the context when call attach, just before one will need to do templatePortal.context = context

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see what you mean. I think it's okay to include it in the signature, then, so long as the JsDoc makes it clear that this updates the context property of the TemplatePortal

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jelbourn I'v added a JsDoc comment to the attach method acoordignly.

I didn't find an example of how to comment a constructor signature, and if its like commenting a method, so I left it as is.

FYI: Both template and viewContainerRef constructor parameters do the same thing (set a property on the instance) and are not commented so I don't think we should go into it. It is self explaining.

super();
this.templateRef = template;
this.viewContainerRef = viewContainerRef;
if (context) {
this.context = context;
}
}

get origin(): ElementRef {
return this.templateRef.elementRef;
}

attach(host: PortalHost, locals?: Map<string, any>): Map<string, any> {
this.locals = locals == null ? new Map<string, any>() : locals;
/**
* Attach the the portal to the provided `PortalHost`.
* When a context is provided it will override the `context` property of the `TemplatePortal`
* instance.
*/
attach(host: PortalHost, context: C | undefined = this.context): C {
this.context = context;
return super.attach(host);
}

detach(): void {
this.locals = new Map<string, any>();
this.context = undefined;
return super.detach();
}
}
Expand Down Expand Up @@ -203,7 +205,7 @@ export abstract class BasePortalHost implements PortalHost {

abstract attachComponentPortal<T>(portal: ComponentPortal<T>): ComponentRef<T>;

abstract attachTemplatePortal(portal: TemplatePortal): Map<string, any>;
abstract attachTemplatePortal<C>(portal: TemplatePortal<C>): EmbeddedViewRef<C>;

detach(): void {
if (this._attachedPortal) {
Expand Down
12 changes: 11 additions & 1 deletion src/demo-app/dialog/dialog-demo.html
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,16 @@ <h2>Other options</h2>

<p>Last close result: {{lastCloseResult}}</p>

<ng-template>
<ng-template let-data let-dialogRef="dialogRef">
I'm a template dialog. I've been opened {{numTemplateOpens}} times!

<p>It's Jazz!</p>

<md-input-container>
<input mdInput placeholder="How much?" #howMuch>
</md-input-container>

<p> {{ data.message }} </p>
<button type="button" (click)="dialogRef.close(lastCloseResult = howMuch.value)">Close dialog</button>
<button (click)="dialogRef.updateSize('500px', '500px').updatePosition({ top: '25px', left: '25px' });">Change dimensions</button>`
</ng-template>
2 changes: 1 addition & 1 deletion src/lib/autocomplete/autocomplete-trigger.ts
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ export function getMdAutocompleteMissingPanelError(): Error {
})
export class MdAutocompleteTrigger implements ControlValueAccessor, OnDestroy {
private _overlayRef: OverlayRef | null;
private _portal: TemplatePortal;
private _portal: TemplatePortal<any>;
private _panelOpen: boolean = false;

/** Strategy that is used to position the panel. */
Expand Down
3 changes: 2 additions & 1 deletion src/lib/dialog/dialog-container.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import {
Component,
ComponentRef,
ElementRef,
EmbeddedViewRef,
EventEmitter,
Inject,
NgZone,
Expand Down Expand Up @@ -124,7 +125,7 @@ export class MdDialogContainer extends BasePortalHost {
* Attach a TemplatePortal as content to this dialog container.
* @param portal Portal to be attached as the dialog content.
*/
attachTemplatePortal(portal: TemplatePortal): Map<string, any> {
attachTemplatePortal<C>(portal: TemplatePortal<C>): EmbeddedViewRef<C> {
if (this._portalHost.hasAttached()) {
throwMdDialogContentAlreadyAttachedError();
}
Expand Down
42 changes: 42 additions & 0 deletions src/lib/dialog/dialog.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import {
Injector,
Inject,
ChangeDetectionStrategy,
TemplateRef
} from '@angular/core';
import {By} from '@angular/platform-browser';
import {NoopAnimationsModule} from '@angular/platform-browser/animations';
Expand Down Expand Up @@ -79,6 +80,28 @@ describe('MdDialog', () => {
expect(dialogContainerElement.getAttribute('role')).toBe('dialog');
});

it('should open a dialog with a template', () => {
const templateRefFixture = TestBed.createComponent(ComponentWithTemplateRef);
templateRefFixture.componentInstance.localValue = 'Bees';
templateRefFixture.detectChanges();

const data = {value: 'Knees'};

let dialogRef = dialog.open(templateRefFixture.componentInstance.templateRef, { data });

viewContainerFixture.detectChanges();

expect(overlayContainerElement.textContent).toContain('Cheese Bees Knees');
expect(templateRefFixture.componentInstance.dialogRef).toBe(dialogRef);

viewContainerFixture.detectChanges();

let dialogContainerElement = overlayContainerElement.querySelector('md-dialog-container')!;
expect(dialogContainerElement.getAttribute('role')).toBe('dialog');

dialogRef.close();
});

it('should use injector from viewContainerRef for DialogInjector', () => {
let dialogRef = dialog.open(PizzaMsg, {
viewContainerRef: testViewContainerRef
Expand Down Expand Up @@ -857,6 +880,23 @@ class ComponentWithChildViewContainer {
}
}

@Component({
selector: 'arbitrary-component-with-template-ref',
template: `<ng-template let-data let-dialogRef="dialogRef">
Cheese {{localValue}} {{data?.value}}{{setDialogRef(dialogRef)}}</ng-template>`,
})
class ComponentWithTemplateRef {
localValue: string;
dialogRef: MdDialogRef<any>;

@ViewChild(TemplateRef) templateRef: TemplateRef<any>;

setDialogRef(dialogRef: MdDialogRef<any>): string {
this.dialogRef = dialogRef;
return '';
}
}

/** Simple component for testing ComponentPortal. */
@Component({template: '<p>Pizza</p> <input> <button>Close</button>'})
class PizzaMsg {
Expand Down Expand Up @@ -897,6 +937,7 @@ class DialogWithInjectedData {
// https://github.com/angular/angular/issues/10760
const TEST_DIRECTIVES = [
ComponentWithChildViewContainer,
ComponentWithTemplateRef,
PizzaMsg,
DirectiveWithViewContainer,
ComponentWithOnPushViewContainer,
Expand All @@ -910,6 +951,7 @@ const TEST_DIRECTIVES = [
declarations: TEST_DIRECTIVES,
entryComponents: [
ComponentWithChildViewContainer,
ComponentWithTemplateRef,
PizzaMsg,
ContentElementDialog,
DialogWithInjectedData
Expand Down
Loading