Skip to content

Commit

Permalink
fix(modal): properly destroy content views
Browse files Browse the repository at this point in the history
Fixes #806

Closes #826
  • Loading branch information
pkozlowski-opensource committed Oct 2, 2016
1 parent 20a9074 commit 3cdb0ff
Show file tree
Hide file tree
Showing 3 changed files with 51 additions and 11 deletions.
21 changes: 14 additions & 7 deletions src/modal/modal-container.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import {
Injector,
Renderer,
TemplateRef,
ViewRef,
ViewContainerRef,
ComponentFactoryResolver,
ComponentFactory,
Expand All @@ -21,6 +22,10 @@ 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>;
Expand All @@ -37,15 +42,16 @@ export class NgbModalContainer {

open(content: string | TemplateRef<any>, options): NgbModalRef {
const modalContentContext = new ModalContentContext();
const nodes = this._getContentNodes(content, modalContentContext);
const windowCmptRef = this._viewContainerRef.createComponent(this._windowFactory, 0, this._injector, nodes);
const contentRef = this._getContentRef(content, modalContentContext);
const windowCmptRef =
this._viewContainerRef.createComponent(this._windowFactory, 0, this._injector, contentRef.nodes);
let backdropCmptRef: ComponentRef<NgbModalBackdrop>;
let ngbModalRef: NgbModalRef;

if (options.backdrop !== false) {
backdropCmptRef = this._viewContainerRef.createComponent(this._backdropFactory, 0, this._injector);
}
ngbModalRef = new NgbModalRef(this._viewContainerRef, windowCmptRef, backdropCmptRef);
ngbModalRef = new NgbModalRef(this._viewContainerRef, windowCmptRef, backdropCmptRef, contentRef.viewRef);

modalContentContext.close = (result: any) => { ngbModalRef.close(result); };
modalContentContext.dismiss = (reason: any) => { ngbModalRef.dismiss(reason); };
Expand All @@ -63,13 +69,14 @@ export class NgbModalContainer {
});
}

private _getContentNodes(content: string | TemplateRef<any>, context: ModalContentContext): any[] {
private _getContentRef(content: string | TemplateRef<any>, context: ModalContentContext): ContentRef {
if (!content) {
return [];
return new ContentRef([]);
} else if (content instanceof TemplateRef) {
return [this._viewContainerRef.createEmbeddedView(<TemplateRef<ModalContentContext>>content, context).rootNodes];
const viewRef = this._viewContainerRef.createEmbeddedView(<TemplateRef<ModalContentContext>>content, context);
return new ContentRef([viewRef.rootNodes], viewRef);
} else {
return [[this._renderer.createText(null, `${content}`)]];
return new ContentRef([[this._renderer.createText(null, `${content}`)]]);
}
}
}
8 changes: 6 additions & 2 deletions src/modal/modal-ref.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import {Injectable, ComponentRef, ViewContainerRef} from '@angular/core';
import {Injectable, ComponentRef, ViewRef, ViewContainerRef} from '@angular/core';

import {NgbModalBackdrop} from './modal-backdrop';
import {NgbModalWindow} from './modal-window';
Expand All @@ -18,7 +18,7 @@ export class NgbModalRef {

constructor(
private _viewContainerRef: ViewContainerRef, private _windowCmptRef: ComponentRef<NgbModalWindow>,
private _backdropCmptRef?: ComponentRef<NgbModalBackdrop>) {
private _backdropCmptRef?: ComponentRef<NgbModalBackdrop>, private _contentViewRef?: ViewRef) {
_windowCmptRef.instance.dismissEvent.subscribe((reason: any) => { this.dismiss(reason); });

this.result = new Promise((resolve, reject) => {
Expand Down Expand Up @@ -52,8 +52,12 @@ export class NgbModalRef {
if (this._backdropCmptRef) {
this._viewContainerRef.remove(this._viewContainerRef.indexOf(this._backdropCmptRef.hostView));
}
if (this._contentViewRef) {
this._viewContainerRef.remove(this._viewContainerRef.indexOf(this._contentViewRef));
}

this._windowCmptRef = null;
this._backdropCmptRef = null;
this._contentViewRef = null;
}
}
33 changes: 31 additions & 2 deletions src/modal/modal.spec.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,16 @@
import {Component, ViewChild} from '@angular/core';
import {Component, Injectable, ViewChild, OnDestroy} from '@angular/core';
import {TestBed, ComponentFixture} from '@angular/core/testing';
import {By} from '@angular/platform-browser';

import {NgbModalModule, NgbModal, NgbModalRef} from './modal.module';

const NOOP = () => {};

@Injectable()
class SpyService {
called = false;
}

describe('ngb-modal', () => {

let fixture: ComponentFixture<TestComponent>;
Expand Down Expand Up @@ -63,7 +68,8 @@ describe('ngb-modal', () => {
});

beforeEach(() => {
TestBed.configureTestingModule({declarations: [TestComponent], imports: [NgbModalModule.forRoot()]});
TestBed.configureTestingModule(
{declarations: [TestComponent, DestroyableCmpt], imports: [NgbModalModule.forRoot()], providers: [SpyService]});
fixture = TestBed.createComponent(TestComponent);
});

Expand All @@ -89,6 +95,19 @@ describe('ngb-modal', () => {
expect(fixture.nativeElement).not.toHaveModal();
});

it('should properly destroy TemplateRef content', () => {
const spyService = fixture.debugElement.injector.get(SpyService);
const modalInstance = fixture.componentInstance.openDestroyableTpl();
fixture.detectChanges();
expect(fixture.nativeElement).toHaveModal('Some content');
expect(spyService.called).toBeFalsy();

modalInstance.close('some result');
fixture.detectChanges();
expect(fixture.nativeElement).not.toHaveModal();
expect(spyService.called).toBeTruthy();
});

it('should open and close modal from inside', () => {
fixture.componentInstance.openTplClose();
fixture.detectChanges();
Expand Down Expand Up @@ -320,11 +339,19 @@ describe('ngb-modal', () => {
});
});

@Component({selector: 'destroyable-cmpt', template: 'Some content'})
export class DestroyableCmpt implements OnDestroy {
constructor(private _spyService: SpyService) {}

ngOnDestroy(): void { this._spyService.called = true; }
}

@Component({
selector: 'test-cmpt',
template: `
<template ngbModalContainer></template>
<template #content>Hello, {{name}}!</template>
<template #destroyableContent><destroyable-cmpt></destroyable-cmpt></template>
<template #contentWithClose let-close="close"><button id="close" (click)="close('myResult')">Close me</button></template>
<template #contentWithDismiss let-dismiss="dismiss"><button id="dismiss" (click)="dismiss('myReason')">Dismiss me</button></template>
<button id="open" (click)="open('from button')">Open</button>
Expand All @@ -333,6 +360,7 @@ describe('ngb-modal', () => {
class TestComponent {
name = 'World';
@ViewChild('content') tplContent;
@ViewChild('destroyableContent') tplDestroyableContent;
@ViewChild('contentWithClose') tplContentWithClose;
@ViewChild('contentWithDismiss') tplContentWithDismiss;
openedModal: NgbModalRef;
Expand All @@ -349,6 +377,7 @@ class TestComponent {
}
}
openTpl(options?: Object) { return this.modalService.open(this.tplContent, options); }
openDestroyableTpl(options?: Object) { return this.modalService.open(this.tplDestroyableContent, options); }
openTplClose(options?: Object) { return this.modalService.open(this.tplContentWithClose, options); }
openTplDismiss(options?: Object) { return this.modalService.open(this.tplContentWithDismiss, options); }
}

0 comments on commit 3cdb0ff

Please sign in to comment.