Skip to content

Commit

Permalink
fix(tooltip): wrong position when using OnPush change detection (#3671)
Browse files Browse the repository at this point in the history
* fix(tooltip): wrong position when using OnPush change detection

Fixes the tooltip having a wrong position if it is inside a component with `OnPush` change detection. The issue was due to the fact that when `OnPush` is used, the tooltip text isn't rendered at the moment when the element is positioned.

Fixes #3497.
  • Loading branch information
crisbeto authored and jelbourn committed Apr 11, 2017
1 parent 3796f69 commit edf01c0
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 9 deletions.
13 changes: 11 additions & 2 deletions src/lib/tooltip/tooltip.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ describe('MdTooltip', () => {
imports: [MdTooltipModule.forRoot(), OverlayModule, NoopAnimationsModule],
declarations: [BasicTooltipDemo, ScrollableTooltipDemo, OnPushTooltipDemo],
providers: [
Platform,
{provide: Platform, useValue: {IOS: false}},
{provide: OverlayContainer, useFactory: () => {
overlayContainerElement = document.createElement('div');
document.body.appendChild(overlayContainerElement);
Expand Down Expand Up @@ -411,7 +411,7 @@ describe('MdTooltip', () => {

fixture.detectChanges();

// wait till animation has finished
// wait until animation has finished
tick(500);

// Make sure tooltip is shown to the user and animation has finished
Expand All @@ -433,6 +433,15 @@ describe('MdTooltip', () => {
flushMicrotasks();
expect(tooltipDirective._tooltipInstance).toBeNull();
}));

it('should have rendered the tooltip text on init', fakeAsync(() => {
dispatchFakeEvent(buttonElement, 'mouseenter');
fixture.detectChanges();
tick(0);

const tooltipElement = overlayContainerElement.querySelector('.mat-tooltip') as HTMLElement;
expect(tooltipElement.textContent).toContain('initial tooltip message');
}));
});

describe('destroy', () => {
Expand Down
25 changes: 18 additions & 7 deletions src/lib/tooltip/tooltip.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import {
OnDestroy,
Renderer,
OnInit,
ChangeDetectorRef
ChangeDetectorRef,
} from '@angular/core';
import {
style,
Expand Down Expand Up @@ -157,7 +157,7 @@ export class MdTooltip implements OnInit, OnDestroy {
@Optional() private _dir: Dir) {

// The mouse events shouldn't be bound on iOS devices, because
// they can prevent the first tap from firing it's click event.
// they can prevent the first tap from firing its click event.
if (!_platform.IOS) {
_renderer.listen(_elementRef.nativeElement, 'mouseenter', () => this.show());
_renderer.listen(_elementRef.nativeElement, 'mouseleave', () => this.hide());
Expand Down Expand Up @@ -313,6 +313,8 @@ export class MdTooltip implements OnInit, OnDestroy {
// Must wait for the message to be painted to the tooltip so that the overlay can properly
// calculate the correct positioning based on the size of the text.
this._tooltipInstance.message = message;
this._tooltipInstance._markForCheck();

this._ngZone.onMicrotaskEmpty.first().subscribe(() => {
if (this._tooltipInstance) {
this._overlayRef.updatePosition();
Expand Down Expand Up @@ -394,8 +396,8 @@ export class TooltipComponent {

// 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);
this._markForCheck();
setTimeout(() => this._closeOnInteraction = true, 0);
}, delay);
}

Expand All @@ -415,7 +417,7 @@ export class TooltipComponent {

// Mark for check so if any parent component has set the
// ChangeDetectionStrategy to OnPush it will be checked anyways
this._changeDetectorRef.markForCheck();
this._markForCheck();
}, delay);
}

Expand All @@ -441,8 +443,8 @@ export class TooltipComponent {
case 'after': this._transformOrigin = isLtr ? 'left' : 'right'; break;
case 'left': this._transformOrigin = 'right'; break;
case 'right': this._transformOrigin = 'left'; break;
case 'above': this._transformOrigin = 'bottom'; break;
case 'below': this._transformOrigin = 'top'; break;
case 'above': this._transformOrigin = 'bottom'; break;
case 'below': this._transformOrigin = 'top'; break;
default: throw new MdTooltipInvalidPositionError(value);
}
}
Expand All @@ -463,4 +465,13 @@ export class TooltipComponent {
this.hide(0);
}
}

/**
* Marks that the tooltip needs to be checked in the next change detection run.
* Mainly used for rendering the initial text before positioning a tooltip, which
* can be problematic in components with OnPush change detection.
*/
_markForCheck(): void {
this._changeDetectorRef.markForCheck();
}
}

0 comments on commit edf01c0

Please sign in to comment.