From 24015f8c38953918144f63cefb120a57705847d6 Mon Sep 17 00:00:00 2001 From: Peter Blazejewicz Date: Tue, 24 Sep 2019 20:41:10 +0200 Subject: [PATCH] fix(tooltip): implement change detection This fixes issues with custom tooltip class changes not being applied after tooltip creation. Fixes: #3335 --- src/tooltip/tooltip.spec.ts | 81 +++++++++++++++++++++++++++++++++++-- src/tooltip/tooltip.ts | 24 +++++++++-- 2 files changed, 98 insertions(+), 7 deletions(-) diff --git a/src/tooltip/tooltip.spec.ts b/src/tooltip/tooltip.spec.ts index b0288608e3..c728d317b0 100644 --- a/src/tooltip/tooltip.spec.ts +++ b/src/tooltip/tooltip.spec.ts @@ -3,12 +3,13 @@ import {createGenericTestComponent, createKeyEvent, triggerEvent} from '../test/ import {By} from '@angular/platform-browser'; import { - Component, - ViewChild, + AfterViewInit, ChangeDetectionStrategy, + ChangeDetectorRef, + Component, TemplateRef, - ViewContainerRef, - AfterViewInit + ViewChild, + ViewContainerRef } from '@angular/core'; import {Key} from '../util/key'; @@ -60,6 +61,78 @@ describe('ngb-tooltip-window', () => { expect(fixture.nativeElement).toHaveCssClass('my-custom-class'); }); + it('should allow to change custom class to new one', () => { + const fixture = TestBed.createComponent(NgbTooltipWindow); + fixture.componentInstance.tooltipClass = 'first-custom-class'; + fixture.detectChanges(); + fixture.componentInstance.tooltipClass = 'second-custom-class'; + fixture.detectChanges(); + fixture.componentInstance.tooltipClass = 'third-custom-class'; + fixture.detectChanges(); + + expect(fixture.nativeElement).toHaveCssClass('third-custom-class'); + }); + + describe('ngb-tooltip-window tooltipClass change detection', () => { + + const mockChangeDetectorRef: ChangeDetectorRef = { + markForCheck: () => {}, + detach: () => {}, + detectChanges: () => {}, + checkNoChanges: () => {}, + reattach: () => {}, + }; + + it('should not call change detection on firt run', () => { + const componentInstance = new NgbTooltipWindow(mockChangeDetectorRef); + spyOn(mockChangeDetectorRef, 'markForCheck'); + + componentInstance.ngOnChanges({ + 'tooltipClass': { + firstChange: true, + isFirstChange: () => true, + previousValue: undefined, + currentValue: undefined, + } + }); + + expect(mockChangeDetectorRef.markForCheck).not.toHaveBeenCalled(); + }); + + it('should call change detection on subsequent run', () => { + const componentInstance = new NgbTooltipWindow(mockChangeDetectorRef); + spyOn(mockChangeDetectorRef, 'markForCheck'); + + componentInstance.ngOnChanges({ + 'tooltipClass': { + firstChange: false, + isFirstChange: () => false, + previousValue: undefined, + currentValue: undefined, + } + }); + + expect(mockChangeDetectorRef.markForCheck).toHaveBeenCalled(); + }); + + it('should not call change detection on other property change', () => { + const componentInstance = new NgbTooltipWindow(mockChangeDetectorRef); + spyOn(mockChangeDetectorRef, 'markForCheck'); + + componentInstance.ngOnChanges({ + 'someOtherProperty': { + firstChange: false, + isFirstChange: () => false, + previousValue: undefined, + currentValue: undefined, + } + }); + + expect(mockChangeDetectorRef.markForCheck).not.toHaveBeenCalled(); + }); + + }); + }); describe('ngb-tooltip', () => { diff --git a/src/tooltip/tooltip.ts b/src/tooltip/tooltip.ts index 093d6af663..09e3375a18 100644 --- a/src/tooltip/tooltip.ts +++ b/src/tooltip/tooltip.ts @@ -18,7 +18,9 @@ import { NgZone, ViewEncapsulation, ChangeDetectorRef, - ApplicationRef + ApplicationRef, + OnChanges, + SimpleChanges } from '@angular/core'; import {DOCUMENT} from '@angular/common'; @@ -39,16 +41,25 @@ let nextId = 0; template: `
`, styleUrls: ['./tooltip.scss'] }) -export class NgbTooltipWindow { +export class NgbTooltipWindow implements OnChanges { @Input() id: string; @Input() tooltipClass: string; + + constructor(private _changeDetector: ChangeDetectorRef) {} + + ngOnChanges(changes: SimpleChanges): void { + const tooltipClassChange = changes['tooltipClass']; + if (tooltipClassChange && !tooltipClassChange.firstChange) { + this._changeDetector.markForCheck(); + } + } } /** * A lightweight and extensible directive for fancy tooltip creation. */ @Directive({selector: '[ngbTooltip]', exportAs: 'ngbTooltip'}) -export class NgbTooltip implements OnInit, OnDestroy { +export class NgbTooltip implements OnInit, OnDestroy, OnChanges { /** * Indicates whether the tooltip should be closed on `Escape` key and inside/outside clicks: * @@ -253,6 +264,13 @@ export class NgbTooltip implements OnInit, OnDestroy { this.close.bind(this), +this.openDelay, +this.closeDelay); } + ngOnChanges(changes: SimpleChanges) { + const tooltipClassChange = changes['tooltipClass']; + if (tooltipClassChange && !tooltipClassChange.firstChange && this._windowRef) { + this._windowRef.instance.tooltipClass = tooltipClassChange.currentValue; + } + } + ngOnDestroy() { this.close(); // This check is needed as it might happen that ngOnDestroy is called before ngOnInit