Skip to content

Commit

Permalink
fix(portal-host): unable to clear and portal reference not being set
Browse files Browse the repository at this point in the history
* Fixes not being able to clear an attached `PortalHost` by setting its contents to `null`.
* Fixes the `portal` reference not being set, if the portal host gets populated programmatically.
* Clears references to the attached portal on destroy.
* Switches to calling the inherited methods via `super`, instead of `this`, making it easier to spot which ones are inherited.
  • Loading branch information
crisbeto committed Feb 25, 2017
1 parent c203589 commit 541d89c
Show file tree
Hide file tree
Showing 2 changed files with 65 additions and 19 deletions.
34 changes: 16 additions & 18 deletions src/lib/core/portal/portal-directives.ts
Original file line number Diff line number Diff line change
Expand Up @@ -63,14 +63,21 @@ export class PortalHostDirective extends BasePortalHost implements OnDestroy {
return this._portal;
}

set portal(p: Portal<any>) {
if (p) {
this._replaceAttachedPortal(p);
set portal(portal: Portal<any>) {
if (this.hasAttached()) {
super.detach();
}

if (portal) {
super.attach(portal);
}

this._portal = portal;
}

ngOnDestroy() {
this.dispose();
super.dispose();
this._portal = null;
}

/**
Expand All @@ -93,7 +100,9 @@ export class PortalHostDirective extends BasePortalHost implements OnDestroy {
componentFactory, viewContainerRef.length,
portal.injector || viewContainerRef.parentInjector);

this.setDisposeFn(() => ref.destroy());
super.setDisposeFn(() => ref.destroy());
this._portal = portal;

return ref;
}

Expand All @@ -105,23 +114,12 @@ export class PortalHostDirective extends BasePortalHost implements OnDestroy {
portal.setAttachedHost(this);

this._viewContainerRef.createEmbeddedView(portal.templateRef);
this.setDisposeFn(() => this._viewContainerRef.clear());
super.setDisposeFn(() => this._viewContainerRef.clear());
this._portal = portal;

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

/** Detaches the currently attached Portal (if there is one) and attaches the given Portal. */
private _replaceAttachedPortal(p: Portal<any>): void {
if (this.hasAttached()) {
this.detach();
}

if (p) {
this.attach(p);
this._portal = p;
}
}
}


Expand Down
50 changes: 49 additions & 1 deletion src/lib/core/portal/portal.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import {inject, ComponentFixture, TestBed, async} from '@angular/core/testing';
import {
NgModule,
Component,
ViewChild,
ViewChildren,
QueryList,
ViewContainerRef,
Expand All @@ -10,7 +11,7 @@ import {
Injector,
ApplicationRef,
} from '@angular/core';
import {TemplatePortalDirective, PortalModule} from './portal-directives';
import {TemplatePortalDirective, PortalHostDirective, PortalModule} from './portal-directives';
import {Portal, ComponentPortal} from './portal';
import {DomPortalHost} from './dom-portal-host';

Expand Down Expand Up @@ -141,6 +142,52 @@ describe('Portals', () => {

expect(hostContainer.textContent).toContain('Pizza');
});

it('should detach the portal when it is set to null', () => {
let testAppComponent = fixture.debugElement.componentInstance;
testAppComponent.selectedPortal = new ComponentPortal(PizzaMsg);

fixture.detectChanges();
expect(testAppComponent.portalHost.hasAttached()).toBe(true);
expect(testAppComponent.portalHost.portal).toBe(testAppComponent.selectedPortal);

testAppComponent.selectedPortal = null;
fixture.detectChanges();

expect(testAppComponent.portalHost.hasAttached()).toBe(false);
expect(testAppComponent.portalHost.portal).toBeNull();
});

it('should set the `portal` when attaching a component portal programmatically', () => {
let testAppComponent = fixture.debugElement.componentInstance;
let portal = new ComponentPortal(PizzaMsg);

testAppComponent.portalHost.attachComponentPortal(portal);

expect(testAppComponent.portalHost.portal).toBe(portal);
});

it('should set the `portal` when attaching a template portal programmatically', () => {
let testAppComponent = fixture.debugElement.componentInstance;
fixture.detectChanges();

testAppComponent.portalHost.attachTemplatePortal(testAppComponent.cakePortal);

expect(testAppComponent.portalHost.portal).toBe(testAppComponent.cakePortal);
});

it('should clear the portal reference on destroy', () => {
let testAppComponent = fixture.debugElement.componentInstance;

testAppComponent.selectedPortal = new ComponentPortal(PizzaMsg);
fixture.detectChanges();

expect(testAppComponent.portalHost.portal).toBeTruthy();

fixture.destroy();

expect(testAppComponent.portalHost.portal).toBeNull();
});
});

describe('DomPortalHost', () => {
Expand Down Expand Up @@ -331,6 +378,7 @@ class ArbitraryViewContainerRefComponent {
})
class PortalTestApp {
@ViewChildren(TemplatePortalDirective) portals: QueryList<TemplatePortalDirective>;
@ViewChild(PortalHostDirective) portalHost: PortalHostDirective;
selectedPortal: Portal<any>;
fruit: string = 'Banana';

Expand Down

0 comments on commit 541d89c

Please sign in to comment.