From 632b964b0dd1cf15349265ec118a6844e6cec9d0 Mon Sep 17 00:00:00 2001 From: Thomas Pink Date: Tue, 31 Jan 2017 23:28:26 +0100 Subject: [PATCH] fix(tooltip): not working properly with ChangeDetectionStrategy.OnPush (#2721) Fixes #2713 --- src/demo-app/tooltip/tooltip-demo.ts | 3 +- src/lib/tooltip/tooltip.spec.ts | 78 +++++++++++++++++++++++++++- src/lib/tooltip/tooltip.ts | 13 ++++- 3 files changed, 89 insertions(+), 5 deletions(-) diff --git a/src/demo-app/tooltip/tooltip-demo.ts b/src/demo-app/tooltip/tooltip-demo.ts index a18c5605a24..96a59b7e53c 100644 --- a/src/demo-app/tooltip/tooltip-demo.ts +++ b/src/demo-app/tooltip/tooltip-demo.ts @@ -1,4 +1,4 @@ -import {Component} from '@angular/core'; +import {Component, ChangeDetectionStrategy} from '@angular/core'; import {TooltipPosition} from '@angular/material'; @@ -7,6 +7,7 @@ import {TooltipPosition} from '@angular/material'; selector: 'tooltip-demo', templateUrl: 'tooltip-demo.html', styleUrls: ['tooltip-demo.css'], + changeDetection: ChangeDetectionStrategy.OnPush // make sure tooltip also works OnPush }) export class TooltipDemo { position: TooltipPosition = 'below'; diff --git a/src/lib/tooltip/tooltip.spec.ts b/src/lib/tooltip/tooltip.spec.ts index 70abb527b47..6c69af2167a 100644 --- a/src/lib/tooltip/tooltip.spec.ts +++ b/src/lib/tooltip/tooltip.spec.ts @@ -6,7 +6,12 @@ import { fakeAsync, flushMicrotasks } from '@angular/core/testing'; -import {Component, DebugElement, AnimationTransitionEvent} from '@angular/core'; +import { + Component, + DebugElement, + AnimationTransitionEvent, + ChangeDetectionStrategy +} from '@angular/core'; import {By} from '@angular/platform-browser'; import {TooltipPosition, MdTooltip, MdTooltipModule} from './tooltip'; import {OverlayContainer} from '../core'; @@ -22,7 +27,7 @@ describe('MdTooltip', () => { beforeEach(async(() => { TestBed.configureTestingModule({ imports: [MdTooltipModule.forRoot(), OverlayModule], - declarations: [BasicTooltipDemo], + declarations: [BasicTooltipDemo, OnPushTooltipDemo], providers: [ {provide: OverlayContainer, useFactory: () => { overlayContainerElement = document.createElement('div'); @@ -59,6 +64,15 @@ describe('MdTooltip', () => { expect(tooltipDirective._isTooltipVisible()).toBe(true); fixture.detectChanges(); + + // wait till animation has finished + tick(500); + + // Make sure tooltip is shown to the user and animation has finished + const tooltipElement = overlayContainerElement.querySelector('.md-tooltip') as HTMLElement; + expect(tooltipElement instanceof HTMLElement).toBe(true); + expect(tooltipElement.style.transform).toBe('scale(1)'); + expect(overlayContainerElement.textContent).toContain(initialTooltipMessage); // After hide called, a timeout delay is created that will to hide the tooltip. @@ -297,6 +311,53 @@ describe('MdTooltip', () => { }).toThrowError('Tooltip position "everywhere" is invalid.'); }); }); + + describe('with OnPush', () => { + let fixture: ComponentFixture; + let buttonDebugElement: DebugElement; + let buttonElement: HTMLButtonElement; + let tooltipDirective: MdTooltip; + + beforeEach(() => { + fixture = TestBed.createComponent(OnPushTooltipDemo); + fixture.detectChanges(); + buttonDebugElement = fixture.debugElement.query(By.css('button')); + buttonElement = buttonDebugElement.nativeElement; + tooltipDirective = buttonDebugElement.injector.get(MdTooltip); + }); + + it('should show and hide the tooltip', fakeAsync(() => { + expect(tooltipDirective._tooltipInstance).toBeUndefined(); + + tooltipDirective.show(); + tick(0); // Tick for the show delay (default is 0) + expect(tooltipDirective._isTooltipVisible()).toBe(true); + + fixture.detectChanges(); + + // wait till animation has finished + tick(500); + + // Make sure tooltip is shown to the user and animation has finished + const tooltipElement = overlayContainerElement.querySelector('.md-tooltip') as HTMLElement; + expect(tooltipElement instanceof HTMLElement).toBe(true); + expect(tooltipElement.style.transform).toBe('scale(1)'); + + // After hide called, a timeout delay is created that will to hide the tooltip. + const tooltipDelay = 1000; + tooltipDirective.hide(tooltipDelay); + expect(tooltipDirective._isTooltipVisible()).toBe(true); + + // After the tooltip delay elapses, expect that the tooltip is not visible. + tick(tooltipDelay); + fixture.detectChanges(); + expect(tooltipDirective._isTooltipVisible()).toBe(false); + + // On animation complete, should expect that the tooltip has been detached. + flushMicrotasks(); + expect(tooltipDirective._tooltipInstance).toBeNull(); + })); + }); }); @Component({ @@ -313,4 +374,17 @@ class BasicTooltipDemo { message: string = initialTooltipMessage; showButton: boolean = true; } +@Component({ + selector: 'app', + template: ` + `, + changeDetection: ChangeDetectionStrategy.OnPush +}) +class OnPushTooltipDemo { + position: string = 'below'; + message: string = initialTooltipMessage; +} diff --git a/src/lib/tooltip/tooltip.ts b/src/lib/tooltip/tooltip.ts index 47b88c7d172..94dc7780062 100644 --- a/src/lib/tooltip/tooltip.ts +++ b/src/lib/tooltip/tooltip.ts @@ -14,7 +14,8 @@ import { AnimationTransitionEvent, NgZone, Optional, - OnDestroy + OnDestroy, + ChangeDetectorRef } from '@angular/core'; import { Overlay, @@ -306,7 +307,7 @@ export class TooltipComponent { /** Subject for notifying that the tooltip has been hidden from the view */ private _onHide: Subject = new Subject(); - constructor(@Optional() private _dir: Dir) {} + constructor(@Optional() private _dir: Dir, private _changeDetectorRef: ChangeDetectorRef) {} /** * Shows the tooltip with an animation originating from the provided origin @@ -329,6 +330,10 @@ export class TooltipComponent { // If this was set to true immediately, then a body click that triggers show() would // trigger interaction and close the tooltip right after it was displayed. this._closeOnInteraction = false; + + // Mark for check so if any parent component has set the + // ChangeDetectionStrategy to OnPush it will be checked anyways + this._changeDetectorRef.markForCheck(); setTimeout(() => { this._closeOnInteraction = true; }, 0); }, delay); } @@ -346,6 +351,10 @@ export class TooltipComponent { this._hideTimeoutId = setTimeout(() => { this._visibility = 'hidden'; this._closeOnInteraction = false; + + // Mark for check so if any parent component has set the + // ChangeDetectionStrategy to OnPush it will be checked anyways + this._changeDetectorRef.markForCheck(); }, delay); }