Skip to content

Commit edf01c0

Browse files
crisbetojelbourn
authored andcommitted
fix(tooltip): wrong position when using OnPush change detection (#3671)
* 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.
1 parent 3796f69 commit edf01c0

File tree

2 files changed

+29
-9
lines changed

2 files changed

+29
-9
lines changed

src/lib/tooltip/tooltip.spec.ts

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ describe('MdTooltip', () => {
3535
imports: [MdTooltipModule.forRoot(), OverlayModule, NoopAnimationsModule],
3636
declarations: [BasicTooltipDemo, ScrollableTooltipDemo, OnPushTooltipDemo],
3737
providers: [
38-
Platform,
38+
{provide: Platform, useValue: {IOS: false}},
3939
{provide: OverlayContainer, useFactory: () => {
4040
overlayContainerElement = document.createElement('div');
4141
document.body.appendChild(overlayContainerElement);
@@ -411,7 +411,7 @@ describe('MdTooltip', () => {
411411

412412
fixture.detectChanges();
413413

414-
// wait till animation has finished
414+
// wait until animation has finished
415415
tick(500);
416416

417417
// Make sure tooltip is shown to the user and animation has finished
@@ -433,6 +433,15 @@ describe('MdTooltip', () => {
433433
flushMicrotasks();
434434
expect(tooltipDirective._tooltipInstance).toBeNull();
435435
}));
436+
437+
it('should have rendered the tooltip text on init', fakeAsync(() => {
438+
dispatchFakeEvent(buttonElement, 'mouseenter');
439+
fixture.detectChanges();
440+
tick(0);
441+
442+
const tooltipElement = overlayContainerElement.querySelector('.mat-tooltip') as HTMLElement;
443+
expect(tooltipElement.textContent).toContain('initial tooltip message');
444+
}));
436445
});
437446

438447
describe('destroy', () => {

src/lib/tooltip/tooltip.ts

Lines changed: 18 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ import {
99
OnDestroy,
1010
Renderer,
1111
OnInit,
12-
ChangeDetectorRef
12+
ChangeDetectorRef,
1313
} from '@angular/core';
1414
import {
1515
style,
@@ -157,7 +157,7 @@ export class MdTooltip implements OnInit, OnDestroy {
157157
@Optional() private _dir: Dir) {
158158

159159
// The mouse events shouldn't be bound on iOS devices, because
160-
// they can prevent the first tap from firing it's click event.
160+
// they can prevent the first tap from firing its click event.
161161
if (!_platform.IOS) {
162162
_renderer.listen(_elementRef.nativeElement, 'mouseenter', () => this.show());
163163
_renderer.listen(_elementRef.nativeElement, 'mouseleave', () => this.hide());
@@ -313,6 +313,8 @@ export class MdTooltip implements OnInit, OnDestroy {
313313
// Must wait for the message to be painted to the tooltip so that the overlay can properly
314314
// calculate the correct positioning based on the size of the text.
315315
this._tooltipInstance.message = message;
316+
this._tooltipInstance._markForCheck();
317+
316318
this._ngZone.onMicrotaskEmpty.first().subscribe(() => {
317319
if (this._tooltipInstance) {
318320
this._overlayRef.updatePosition();
@@ -394,8 +396,8 @@ export class TooltipComponent {
394396

395397
// Mark for check so if any parent component has set the
396398
// ChangeDetectionStrategy to OnPush it will be checked anyways
397-
this._changeDetectorRef.markForCheck();
398-
setTimeout(() => { this._closeOnInteraction = true; }, 0);
399+
this._markForCheck();
400+
setTimeout(() => this._closeOnInteraction = true, 0);
399401
}, delay);
400402
}
401403

@@ -415,7 +417,7 @@ export class TooltipComponent {
415417

416418
// Mark for check so if any parent component has set the
417419
// ChangeDetectionStrategy to OnPush it will be checked anyways
418-
this._changeDetectorRef.markForCheck();
420+
this._markForCheck();
419421
}, delay);
420422
}
421423

@@ -441,8 +443,8 @@ export class TooltipComponent {
441443
case 'after': this._transformOrigin = isLtr ? 'left' : 'right'; break;
442444
case 'left': this._transformOrigin = 'right'; break;
443445
case 'right': this._transformOrigin = 'left'; break;
444-
case 'above': this._transformOrigin = 'bottom'; break;
445-
case 'below': this._transformOrigin = 'top'; break;
446+
case 'above': this._transformOrigin = 'bottom'; break;
447+
case 'below': this._transformOrigin = 'top'; break;
446448
default: throw new MdTooltipInvalidPositionError(value);
447449
}
448450
}
@@ -463,4 +465,13 @@ export class TooltipComponent {
463465
this.hide(0);
464466
}
465467
}
468+
469+
/**
470+
* Marks that the tooltip needs to be checked in the next change detection run.
471+
* Mainly used for rendering the initial text before positioning a tooltip, which
472+
* can be problematic in components with OnPush change detection.
473+
*/
474+
_markForCheck(): void {
475+
this._changeDetectorRef.markForCheck();
476+
}
466477
}

0 commit comments

Comments
 (0)