Skip to content

Commit 632b964

Browse files
thomaspinkkara
authored andcommitted
fix(tooltip): not working properly with ChangeDetectionStrategy.OnPush (#2721)
Fixes #2713
1 parent 08e9d70 commit 632b964

File tree

3 files changed

+89
-5
lines changed

3 files changed

+89
-5
lines changed

src/demo-app/tooltip/tooltip-demo.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import {Component} from '@angular/core';
1+
import {Component, ChangeDetectionStrategy} from '@angular/core';
22
import {TooltipPosition} from '@angular/material';
33

44

@@ -7,6 +7,7 @@ import {TooltipPosition} from '@angular/material';
77
selector: 'tooltip-demo',
88
templateUrl: 'tooltip-demo.html',
99
styleUrls: ['tooltip-demo.css'],
10+
changeDetection: ChangeDetectionStrategy.OnPush // make sure tooltip also works OnPush
1011
})
1112
export class TooltipDemo {
1213
position: TooltipPosition = 'below';

src/lib/tooltip/tooltip.spec.ts

Lines changed: 76 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,12 @@ import {
66
fakeAsync,
77
flushMicrotasks
88
} from '@angular/core/testing';
9-
import {Component, DebugElement, AnimationTransitionEvent} from '@angular/core';
9+
import {
10+
Component,
11+
DebugElement,
12+
AnimationTransitionEvent,
13+
ChangeDetectionStrategy
14+
} from '@angular/core';
1015
import {By} from '@angular/platform-browser';
1116
import {TooltipPosition, MdTooltip, MdTooltipModule} from './tooltip';
1217
import {OverlayContainer} from '../core';
@@ -22,7 +27,7 @@ describe('MdTooltip', () => {
2227
beforeEach(async(() => {
2328
TestBed.configureTestingModule({
2429
imports: [MdTooltipModule.forRoot(), OverlayModule],
25-
declarations: [BasicTooltipDemo],
30+
declarations: [BasicTooltipDemo, OnPushTooltipDemo],
2631
providers: [
2732
{provide: OverlayContainer, useFactory: () => {
2833
overlayContainerElement = document.createElement('div');
@@ -59,6 +64,15 @@ describe('MdTooltip', () => {
5964
expect(tooltipDirective._isTooltipVisible()).toBe(true);
6065

6166
fixture.detectChanges();
67+
68+
// wait till animation has finished
69+
tick(500);
70+
71+
// Make sure tooltip is shown to the user and animation has finished
72+
const tooltipElement = overlayContainerElement.querySelector('.md-tooltip') as HTMLElement;
73+
expect(tooltipElement instanceof HTMLElement).toBe(true);
74+
expect(tooltipElement.style.transform).toBe('scale(1)');
75+
6276
expect(overlayContainerElement.textContent).toContain(initialTooltipMessage);
6377

6478
// After hide called, a timeout delay is created that will to hide the tooltip.
@@ -297,6 +311,53 @@ describe('MdTooltip', () => {
297311
}).toThrowError('Tooltip position "everywhere" is invalid.');
298312
});
299313
});
314+
315+
describe('with OnPush', () => {
316+
let fixture: ComponentFixture<OnPushTooltipDemo>;
317+
let buttonDebugElement: DebugElement;
318+
let buttonElement: HTMLButtonElement;
319+
let tooltipDirective: MdTooltip;
320+
321+
beforeEach(() => {
322+
fixture = TestBed.createComponent(OnPushTooltipDemo);
323+
fixture.detectChanges();
324+
buttonDebugElement = fixture.debugElement.query(By.css('button'));
325+
buttonElement = <HTMLButtonElement> buttonDebugElement.nativeElement;
326+
tooltipDirective = buttonDebugElement.injector.get(MdTooltip);
327+
});
328+
329+
it('should show and hide the tooltip', fakeAsync(() => {
330+
expect(tooltipDirective._tooltipInstance).toBeUndefined();
331+
332+
tooltipDirective.show();
333+
tick(0); // Tick for the show delay (default is 0)
334+
expect(tooltipDirective._isTooltipVisible()).toBe(true);
335+
336+
fixture.detectChanges();
337+
338+
// wait till animation has finished
339+
tick(500);
340+
341+
// Make sure tooltip is shown to the user and animation has finished
342+
const tooltipElement = overlayContainerElement.querySelector('.md-tooltip') as HTMLElement;
343+
expect(tooltipElement instanceof HTMLElement).toBe(true);
344+
expect(tooltipElement.style.transform).toBe('scale(1)');
345+
346+
// After hide called, a timeout delay is created that will to hide the tooltip.
347+
const tooltipDelay = 1000;
348+
tooltipDirective.hide(tooltipDelay);
349+
expect(tooltipDirective._isTooltipVisible()).toBe(true);
350+
351+
// After the tooltip delay elapses, expect that the tooltip is not visible.
352+
tick(tooltipDelay);
353+
fixture.detectChanges();
354+
expect(tooltipDirective._isTooltipVisible()).toBe(false);
355+
356+
// On animation complete, should expect that the tooltip has been detached.
357+
flushMicrotasks();
358+
expect(tooltipDirective._tooltipInstance).toBeNull();
359+
}));
360+
});
300361
});
301362

302363
@Component({
@@ -313,4 +374,17 @@ class BasicTooltipDemo {
313374
message: string = initialTooltipMessage;
314375
showButton: boolean = true;
315376
}
377+
@Component({
378+
selector: 'app',
379+
template: `
380+
<button [mdTooltip]="message"
381+
[mdTooltipPosition]="position">
382+
Button
383+
</button>`,
384+
changeDetection: ChangeDetectionStrategy.OnPush
385+
})
386+
class OnPushTooltipDemo {
387+
position: string = 'below';
388+
message: string = initialTooltipMessage;
389+
}
316390

src/lib/tooltip/tooltip.ts

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,8 @@ import {
1414
AnimationTransitionEvent,
1515
NgZone,
1616
Optional,
17-
OnDestroy
17+
OnDestroy,
18+
ChangeDetectorRef
1819
} from '@angular/core';
1920
import {
2021
Overlay,
@@ -306,7 +307,7 @@ export class TooltipComponent {
306307
/** Subject for notifying that the tooltip has been hidden from the view */
307308
private _onHide: Subject<any> = new Subject();
308309

309-
constructor(@Optional() private _dir: Dir) {}
310+
constructor(@Optional() private _dir: Dir, private _changeDetectorRef: ChangeDetectorRef) {}
310311

311312
/**
312313
* Shows the tooltip with an animation originating from the provided origin
@@ -329,6 +330,10 @@ export class TooltipComponent {
329330
// If this was set to true immediately, then a body click that triggers show() would
330331
// trigger interaction and close the tooltip right after it was displayed.
331332
this._closeOnInteraction = false;
333+
334+
// Mark for check so if any parent component has set the
335+
// ChangeDetectionStrategy to OnPush it will be checked anyways
336+
this._changeDetectorRef.markForCheck();
332337
setTimeout(() => { this._closeOnInteraction = true; }, 0);
333338
}, delay);
334339
}
@@ -346,6 +351,10 @@ export class TooltipComponent {
346351
this._hideTimeoutId = setTimeout(() => {
347352
this._visibility = 'hidden';
348353
this._closeOnInteraction = false;
354+
355+
// Mark for check so if any parent component has set the
356+
// ChangeDetectionStrategy to OnPush it will be checked anyways
357+
this._changeDetectorRef.markForCheck();
349358
}, delay);
350359
}
351360

0 commit comments

Comments
 (0)