fix(popover): properly cleanup content passed as TemplateRef

Fixes #827

Closes #830

src/modal/modal-container.ts

@@ -3,14 +3,14 @@ import {
   Injector,
   Renderer,
   TemplateRef,
-  ViewRef,
   ViewContainerRef,
   ComponentFactoryResolver,
   ComponentFactory,
   ComponentRef
 } from '@angular/core';
 
 import {isDefined} from '../util/util';
+import {ContentRef} from '../util/popup';
 
 import {NgbModalBackdrop} from './modal-backdrop';
 import {NgbModalWindow} from './modal-window';
@@ -22,10 +22,6 @@ class ModalContentContext {
   dismiss(reason?: any) {}
 }
 
-class ContentRef {
-  constructor(public nodes: any[], public viewRef?: ViewRef) {}
-}
-
 @Directive({selector: 'template[ngbModalContainer]'})
 export class NgbModalContainer {
   private _backdropFactory: ComponentFactory<NgbModalBackdrop>;

src/popover/popover.spec.ts

@@ -2,12 +2,17 @@ import {TestBed, ComponentFixture, inject} from '@angular/core/testing';
 import {createGenericTestComponent} from '../test/common';
 
 import {By} from '@angular/platform-browser';
-import {Component, ViewChild, ChangeDetectionStrategy} from '@angular/core';
+import {Component, ViewChild, ChangeDetectionStrategy, Injectable, OnDestroy} from '@angular/core';
 
 import {NgbPopoverModule} from './popover.module';
 import {NgbPopoverWindow, NgbPopover} from './popover';
 import {NgbPopoverConfig} from './popover-config';
 
+@Injectable()
+class SpyService {
+  called = false;
+}
+
 const createTestComponent = (html: string) =>
     createGenericTestComponent(html, TestComponent) as ComponentFixture<TestComponent>;
 
@@ -41,8 +46,11 @@ describe('ngb-popover-window', () => {
 describe('ngb-popover', () => {
 
   beforeEach(() => {
-    TestBed.configureTestingModule(
-        {declarations: [TestComponent, TestOnPushComponent], imports: [NgbPopoverModule.forRoot()]});
+    TestBed.configureTestingModule({
+      declarations: [TestComponent, TestOnPushComponent, DestroyableCmpt],
+      imports: [NgbPopoverModule.forRoot()],
+      providers: [SpyService]
+    });
   });
 
   function getWindow(fixture) { return fixture.nativeElement.querySelector('ngb-popover-window'); }
@@ -88,6 +96,24 @@ describe('ngb-popover', () => {
       expect(getWindow(fixture)).toBeNull();
     });
 
+    it('should properly destroy TemplateRef content', () => {
+      const fixture = createTestComponent(`
+          <template #t><destroyable-cmpt></destroyable-cmpt></template>
+          <div [ngbPopover]="t" popoverTitle="Title"></div>`);
+      const directive = fixture.debugElement.query(By.directive(NgbPopover));
+      const spyService = fixture.debugElement.injector.get(SpyService);
+
+      directive.triggerEventHandler('click', {});
+      fixture.detectChanges();
+      expect(getWindow(fixture)).not.toBeNull();
+      expect(spyService.called).toBeFalsy();
+
+      directive.triggerEventHandler('click', {});
+      fixture.detectChanges();
+      expect(getWindow(fixture)).toBeNull();
+      expect(spyService.called).toBeTruthy();
+    });
+
     it('should allow re-opening previously closed popovers', () => {
       const fixture = createTestComponent(`<div ngbPopover="Great tip!" popoverTitle="Title"></div>`);
       const directive = fixture.debugElement.query(By.directive(NgbPopover));
@@ -333,3 +359,10 @@ export class TestComponent {
 @Component({selector: 'test-onpush-cmpt', changeDetection: ChangeDetectionStrategy.OnPush, template: ``})
 export class TestOnPushComponent {
 }
+
+@Component({selector: 'destroyable-cmpt', template: 'Some content'})
+export class DestroyableCmpt implements OnDestroy {
+  constructor(private _spyService: SpyService) {}
+
+  ngOnDestroy(): void { this._spyService.called = true; }
+}

src/util/popup.ts

@@ -1,16 +1,22 @@
 import {
   Injector,
   TemplateRef,
+  ViewRef,
   ViewContainerRef,
   Renderer,
   ComponentRef,
   ComponentFactory,
   ComponentFactoryResolver
 } from '@angular/core';
 
+export class ContentRef {
+  constructor(public nodes: any[], public viewRef?: ViewRef) {}
+}
+
 export class PopupService<T> {
   private _windowFactory: ComponentFactory<T>;
   private _windowRef: ComponentRef<T>;
+  private _contentRef: ContentRef;
 
   constructor(
       type: any, private _injector: Injector, private _viewContainerRef: ViewContainerRef, private _renderer: Renderer,
@@ -20,8 +26,9 @@ export class PopupService<T> {
 
   open(content?: string | TemplateRef<any>): ComponentRef<T> {
     if (!this._windowRef) {
-      const nodes = this._getContentNodes(content);
-      this._windowRef = this._viewContainerRef.createComponent(this._windowFactory, 0, this._injector, nodes);
+      this._contentRef = this._getContentRef(content);
+      this._windowRef =
+          this._viewContainerRef.createComponent(this._windowFactory, 0, this._injector, this._contentRef.nodes);
     }
 
     return this._windowRef;
@@ -31,16 +38,22 @@ export class PopupService<T> {
     if (this._windowRef) {
       this._viewContainerRef.remove(this._viewContainerRef.indexOf(this._windowRef.hostView));
       this._windowRef = null;
+
+      if (this._contentRef.viewRef) {
+        this._viewContainerRef.remove(this._viewContainerRef.indexOf(this._contentRef.viewRef));
+        this._contentRef = null;
+      }
     }
   }
 
-  private _getContentNodes(content: string | TemplateRef<any>) {
+  private _getContentRef(content: string | TemplateRef<any>): ContentRef {
     if (!content) {
-      return [];
+      return new ContentRef([]);
     } else if (content instanceof TemplateRef) {
-      return [this._viewContainerRef.createEmbeddedView(<TemplateRef<T>>content).rootNodes];
+      const viewRef = this._viewContainerRef.createEmbeddedView(<TemplateRef<T>>content);
+      return new ContentRef([viewRef.rootNodes], viewRef);
     } else {
-      return [[this._renderer.createText(null, `${content}`)]];
+      return new ContentRef([[this._renderer.createText(null, `${content}`)]]);
     }
   }
 }