Skip to content

Commit

Permalink
fix(tooltip): not working properly with ChangeDetectionStrategy.OnPush (
Browse files Browse the repository at this point in the history
#2721)

Fixes #2713
  • Loading branch information
thomaspink authored and kara committed Jan 31, 2017
1 parent 08e9d70 commit 632b964
Show file tree
Hide file tree
Showing 3 changed files with 89 additions and 5 deletions.
3 changes: 2 additions & 1 deletion src/demo-app/tooltip/tooltip-demo.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import {Component} from '@angular/core';
import {Component, ChangeDetectionStrategy} from '@angular/core';
import {TooltipPosition} from '@angular/material';


Expand All @@ -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';
Expand Down
78 changes: 76 additions & 2 deletions src/lib/tooltip/tooltip.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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');
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -297,6 +311,53 @@ describe('MdTooltip', () => {
}).toThrowError('Tooltip position "everywhere" is invalid.');
});
});

describe('with OnPush', () => {
let fixture: ComponentFixture<OnPushTooltipDemo>;
let buttonDebugElement: DebugElement;
let buttonElement: HTMLButtonElement;
let tooltipDirective: MdTooltip;

beforeEach(() => {
fixture = TestBed.createComponent(OnPushTooltipDemo);
fixture.detectChanges();
buttonDebugElement = fixture.debugElement.query(By.css('button'));
buttonElement = <HTMLButtonElement> 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({
Expand All @@ -313,4 +374,17 @@ class BasicTooltipDemo {
message: string = initialTooltipMessage;
showButton: boolean = true;
}
@Component({
selector: 'app',
template: `
<button [mdTooltip]="message"
[mdTooltipPosition]="position">
Button
</button>`,
changeDetection: ChangeDetectionStrategy.OnPush
})
class OnPushTooltipDemo {
position: string = 'below';
message: string = initialTooltipMessage;
}

13 changes: 11 additions & 2 deletions src/lib/tooltip/tooltip.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,8 @@ import {
AnimationTransitionEvent,
NgZone,
Optional,
OnDestroy
OnDestroy,
ChangeDetectorRef
} from '@angular/core';
import {
Overlay,
Expand Down Expand Up @@ -306,7 +307,7 @@ export class TooltipComponent {
/** Subject for notifying that the tooltip has been hidden from the view */
private _onHide: Subject<any> = 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
Expand All @@ -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);
}
Expand All @@ -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);
}

Expand Down

0 comments on commit 632b964

Please sign in to comment.