Skip to content

Commit

Permalink
fix(tooltip): properly destroy tooltips using container option
Browse files Browse the repository at this point in the history 
Fixes #910

Closes #912
  • Loading branch information
@pkozlowski-opensource
pkozlowski-opensource committed Oct 20, 2016
1 parent 1259d37 commit 94a4d5c
Show file tree
Hide file tree
Showing 2 changed files with 57 additions and 46 deletions.
92 changes: 51 additions & 41 deletions src/tooltip/tooltip.spec.ts
View file
Expand Up @@ -41,7 +41,7 @@ describe('ngb-tooltip', () => {
{declarations: [TestComponent, TestOnPushComponent], imports: [NgbTooltipModule.forRoot()]});
});

function getWindow(fixture) { return fixture.nativeElement.querySelector('ngb-tooltip-window'); }
function getWindow(element) { return element.querySelector('ngb-tooltip-window'); }

describe('basic functionality', () => {

Expand All @@ -52,7 +52,7 @@ describe('ngb-tooltip', () => {

directive.triggerEventHandler('mouseenter', {});
fixture.detectChanges();
const windowEl = getWindow(fixture);
const windowEl = getWindow(fixture.nativeElement);

expect(windowEl).toHaveCssClass('tooltip');
expect(windowEl).toHaveCssClass(`tooltip-${defaultConfig.placement}`);
Expand All @@ -62,7 +62,7 @@ describe('ngb-tooltip', () => {

directive.triggerEventHandler('mouseleave', {});
fixture.detectChanges();
expect(getWindow(fixture)).toBeNull();
expect(getWindow(fixture.nativeElement)).toBeNull();
});

it('should open and close a tooltip - default settings and content from a template', () => {
Expand All @@ -71,7 +71,7 @@ describe('ngb-tooltip', () => {

directive.triggerEventHandler('mouseenter', {});
fixture.detectChanges();
const windowEl = getWindow(fixture);
const windowEl = getWindow(fixture.nativeElement);

expect(windowEl).toHaveCssClass('tooltip');
expect(windowEl).toHaveCssClass('tooltip-top');
Expand All @@ -81,7 +81,7 @@ describe('ngb-tooltip', () => {

directive.triggerEventHandler('mouseleave', {});
fixture.detectChanges();
expect(getWindow(fixture)).toBeNull();
expect(getWindow(fixture.nativeElement)).toBeNull();
});

it('should not open a tooltip if content is falsy', () => {
Expand All @@ -90,7 +90,7 @@ describe('ngb-tooltip', () => {

directive.triggerEventHandler('mouseenter', {});
fixture.detectChanges();
const windowEl = getWindow(fixture);
const windowEl = getWindow(fixture.nativeElement);

expect(windowEl).toBeNull();
});
Expand All @@ -101,11 +101,11 @@ describe('ngb-tooltip', () => {

directive.triggerEventHandler('mouseenter', {});
fixture.detectChanges();
expect(getWindow(fixture)).not.toBeNull();
expect(getWindow(fixture.nativeElement)).not.toBeNull();

fixture.componentInstance.name = null;
fixture.detectChanges();
expect(getWindow(fixture)).toBeNull();
expect(getWindow(fixture.nativeElement)).toBeNull();
});

it('should allow re-opening previously closed tooltips', () => {
Expand All @@ -114,15 +114,15 @@ describe('ngb-tooltip', () => {

directive.triggerEventHandler('mouseenter', {});
fixture.detectChanges();
expect(getWindow(fixture)).not.toBeNull();
expect(getWindow(fixture.nativeElement)).not.toBeNull();

directive.triggerEventHandler('mouseleave', {});
fixture.detectChanges();
expect(getWindow(fixture)).toBeNull();
expect(getWindow(fixture.nativeElement)).toBeNull();

directive.triggerEventHandler('mouseenter', {});
fixture.detectChanges();
expect(getWindow(fixture)).not.toBeNull();
expect(getWindow(fixture.nativeElement)).not.toBeNull();
});

it('should not leave dangling tooltips in the DOM', () => {
Expand All @@ -131,11 +131,11 @@ describe('ngb-tooltip', () => {

directive.triggerEventHandler('mouseenter', {});
fixture.detectChanges();
expect(getWindow(fixture)).not.toBeNull();
expect(getWindow(fixture.nativeElement)).not.toBeNull();

fixture.componentInstance.show = false;
fixture.detectChanges();
expect(getWindow(fixture)).toBeNull();
expect(getWindow(fixture.nativeElement)).toBeNull();
});

it('should properly cleanup tooltips with manual triggers', () => {
Expand All @@ -147,11 +147,11 @@ describe('ngb-tooltip', () => {

directive.triggerEventHandler('mouseenter', {});
fixture.detectChanges();
expect(getWindow(fixture)).not.toBeNull();
expect(getWindow(fixture.nativeElement)).not.toBeNull();

fixture.componentInstance.show = false;
fixture.detectChanges();
expect(getWindow(fixture)).toBeNull();
expect(getWindow(fixture.nativeElement)).toBeNull();
});

describe('positioning', () => {
Expand All @@ -162,7 +162,7 @@ describe('ngb-tooltip', () => {

directive.triggerEventHandler('mouseenter', {});
fixture.detectChanges();
const windowEl = getWindow(fixture);
const windowEl = getWindow(fixture.nativeElement);

expect(windowEl).toHaveCssClass('tooltip');
expect(windowEl).toHaveCssClass('tooltip-left');
Expand All @@ -175,7 +175,7 @@ describe('ngb-tooltip', () => {

directive.triggerEventHandler('mouseenter', {});
fixture.detectChanges();
const windowEl = getWindow(fixture);
const windowEl = getWindow(fixture.nativeElement);

expect(windowEl).toHaveCssClass('tooltip');
expect(windowEl).toHaveCssClass('tooltip-left');
Expand All @@ -191,11 +191,11 @@ describe('ngb-tooltip', () => {

directive.triggerEventHandler('click', {});
fixture.detectChanges();
expect(getWindow(fixture)).not.toBeNull();
expect(getWindow(fixture.nativeElement)).not.toBeNull();

directive.triggerEventHandler('click', {});
fixture.detectChanges();
expect(getWindow(fixture)).toBeNull();
expect(getWindow(fixture.nativeElement)).toBeNull();
});

it('should non-default toggle triggers', () => {
Expand All @@ -204,11 +204,11 @@ describe('ngb-tooltip', () => {

directive.triggerEventHandler('mouseenter', {});
fixture.detectChanges();
expect(getWindow(fixture)).not.toBeNull();
expect(getWindow(fixture.nativeElement)).not.toBeNull();

directive.triggerEventHandler('click', {});
fixture.detectChanges();
expect(getWindow(fixture)).toBeNull();
expect(getWindow(fixture.nativeElement)).toBeNull();
});

it('should support multiple triggers', () => {
Expand All @@ -218,11 +218,11 @@ describe('ngb-tooltip', () => {

directive.triggerEventHandler('mouseenter', {});
fixture.detectChanges();
expect(getWindow(fixture)).not.toBeNull();
expect(getWindow(fixture.nativeElement)).not.toBeNull();

directive.triggerEventHandler('click', {});
fixture.detectChanges();
expect(getWindow(fixture)).toBeNull();
expect(getWindow(fixture.nativeElement)).toBeNull();
});

it('should not use default for manual triggers', () => {
Expand All @@ -231,7 +231,7 @@ describe('ngb-tooltip', () => {

directive.triggerEventHandler('mouseenter', {});
fixture.detectChanges();
expect(getWindow(fixture)).toBeNull();
expect(getWindow(fixture.nativeElement)).toBeNull();
});

it('should allow toggling for manual triggers', () => {
Expand All @@ -242,11 +242,11 @@ describe('ngb-tooltip', () => {

button.click();
fixture.detectChanges();
expect(getWindow(fixture)).not.toBeNull();
expect(getWindow(fixture.nativeElement)).not.toBeNull();

button.click();
fixture.detectChanges();
expect(getWindow(fixture)).toBeNull();
expect(getWindow(fixture.nativeElement)).toBeNull();
});

it('should allow open / close for manual triggers', () => {
Expand All @@ -259,11 +259,11 @@ describe('ngb-tooltip', () => {

buttons[0].click(); // open
fixture.detectChanges();
expect(getWindow(fixture)).not.toBeNull();
expect(getWindow(fixture.nativeElement)).not.toBeNull();

buttons[1].click(); // close
fixture.detectChanges();
expect(getWindow(fixture)).toBeNull();
expect(getWindow(fixture.nativeElement)).toBeNull();
});

it('should not throw when open called for manual triggers and open tooltip', () => {
Expand All @@ -274,11 +274,11 @@ describe('ngb-tooltip', () => {

button.click(); // open
fixture.detectChanges();
expect(getWindow(fixture)).not.toBeNull();
expect(getWindow(fixture.nativeElement)).not.toBeNull();

button.click(); // open
fixture.detectChanges();
expect(getWindow(fixture)).not.toBeNull();
expect(getWindow(fixture.nativeElement)).not.toBeNull();
});

it('should not throw when closed called for manual triggers and closed tooltip', () => {
Expand All @@ -290,7 +290,7 @@ describe('ngb-tooltip', () => {

button.click(); // close
fixture.detectChanges();
expect(getWindow(fixture)).toBeNull();
expect(getWindow(fixture.nativeElement)).toBeNull();
});
});
});
Expand All @@ -304,14 +304,24 @@ describe('ngb-tooltip', () => {

directive.triggerEventHandler('mouseenter', {});
fixture.detectChanges();
let windowEl = getWindow(fixture);
expect(windowEl).toBeNull();

const container = window.document.querySelector(selector);
windowEl = container.querySelector('ngb-tooltip-window');
expect(windowEl.parentNode).toBe(container);
expect(getWindow(fixture.nativeElement)).toBeNull();
expect(getWindow(document.querySelector(selector))).not.toBeNull();
});

it('should properly destroy tooltips when the "container" option is used', () => {
const selector = 'body';
const fixture =
createTestComponent(`<div *ngIf="show" ngbTooltip="Great tip!" container="` + selector + `"></div>`);
const directive = fixture.debugElement.query(By.directive(NgbTooltip));

directive.triggerEventHandler('mouseenter', {});
fixture.detectChanges();

expect(getWindow(document.querySelector(selector))).not.toBeNull();
fixture.componentRef.instance.show = false;
fixture.detectChanges();
expect(getWindow(document.querySelector(selector))).toBeNull();
});
});

describe('visibility', () => {
Expand All @@ -325,12 +335,12 @@ describe('ngb-tooltip', () => {

directive.triggerEventHandler('click', {});
fixture.detectChanges();
expect(getWindow(fixture)).not.toBeNull();
expect(getWindow(fixture.nativeElement)).not.toBeNull();
expect(shownSpy).toHaveBeenCalled();

directive.triggerEventHandler('click', {});
fixture.detectChanges();
expect(getWindow(fixture)).toBeNull();
expect(getWindow(fixture.nativeElement)).toBeNull();
expect(hiddenSpy).toHaveBeenCalled();
});

Expand All @@ -347,7 +357,7 @@ describe('ngb-tooltip', () => {
fixture.componentInstance.tooltip.open();
fixture.detectChanges();

expect(getWindow(fixture)).not.toBeNull();
expect(getWindow(fixture.nativeElement)).not.toBeNull();
expect(shownSpy).toHaveBeenCalled();
expect(shownSpy.calls.count()).toEqual(1);
expect(hiddenSpy).not.toHaveBeenCalled();
Expand All @@ -362,7 +372,7 @@ describe('ngb-tooltip', () => {

fixture.componentInstance.tooltip.close();
fixture.detectChanges();
expect(getWindow(fixture)).toBeNull();
expect(getWindow(fixture.nativeElement)).toBeNull();
expect(shownSpy).not.toHaveBeenCalled();
expect(hiddenSpy).not.toHaveBeenCalled();
});
Expand Down
11 changes: 6 additions & 5 deletions src/tooltip/tooltip.ts
View file
Expand Up @@ -83,11 +83,6 @@ export class NgbTooltip implements OnInit, OnDestroy {
positionElements(
this._elementRef.nativeElement, this._windowRef.location.nativeElement, this.placement,
this.container === 'body');

if (this.container === 'body') {
let windowEl = this._windowRef.location.nativeElement;
window.document.querySelector(this.container).appendChild(windowEl);
}
}
});
}
Expand All @@ -112,6 +107,11 @@ export class NgbTooltip implements OnInit, OnDestroy {
if (!this._windowRef && this._ngbTooltip) {
this._windowRef = this._popupService.open(this._ngbTooltip);
this._windowRef.instance.placement = this.placement;

if (this.container === 'body') {
window.document.querySelector(this.container).appendChild(this._windowRef.location.nativeElement);
}

// we need to manually invoke change detection since events registered via
// Renderer::listen() - to be determined if this is a bug in the Angular 2
this._windowRef.changeDetectorRef.markForCheck();
Expand Down Expand Up @@ -153,6 +153,7 @@ export class NgbTooltip implements OnInit, OnDestroy {
}

ngOnDestroy() {
this.close();
this._unregisterListenersFn();
this._zoneSubscription.unsubscribe();
}
Expand Down

0 comments on commit 94a4d5c

Please sign in to comment.