From 96463dc3b43f96962786938bd83801ad78c63f8f Mon Sep 17 00:00:00 2001 From: crisbeto Date: Sat, 18 Mar 2017 17:22:41 +0100 Subject: [PATCH 1/3] 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. --- src/lib/tooltip/tooltip.spec.ts | 10 +++++++++- src/lib/tooltip/tooltip.ts | 21 ++++++++++++++++----- 2 files changed, 25 insertions(+), 6 deletions(-) diff --git a/src/lib/tooltip/tooltip.spec.ts b/src/lib/tooltip/tooltip.spec.ts index a5e3cc932c7d..c4288fa644b6 100644 --- a/src/lib/tooltip/tooltip.spec.ts +++ b/src/lib/tooltip/tooltip.spec.ts @@ -410,7 +410,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 @@ -432,6 +432,14 @@ describe('MdTooltip', () => { flushMicrotasks(); expect(tooltipDirective._tooltipInstance).toBeNull(); })); + + it('should have rendered the tooltip text on init', fakeAsync(() => { + dispatchFakeEvent(buttonElement, 'mouseenter'); + tick(0); + + const tooltipElement = overlayContainerElement.querySelector('.mat-tooltip') as HTMLElement; + expect(tooltipElement.textContent).toContain('initial tooltip message'); + })); }); describe('destroy', () => { diff --git a/src/lib/tooltip/tooltip.ts b/src/lib/tooltip/tooltip.ts index cc13061eb92c..4f3d897a2e76 100644 --- a/src/lib/tooltip/tooltip.ts +++ b/src/lib/tooltip/tooltip.ts @@ -15,7 +15,7 @@ import { OnDestroy, Renderer, OnInit, - ChangeDetectorRef + ChangeDetectorRef, } from '@angular/core'; import { Overlay, @@ -155,7 +155,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()); @@ -311,6 +311,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._updateView(); + this._ngZone.onMicrotaskEmpty.first().subscribe(() => { if (this._tooltipInstance) { this._overlayRef.updatePosition(); @@ -393,7 +395,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(); - setTimeout(() => { this._closeOnInteraction = true; }, 0); + setTimeout(() => this._closeOnInteraction = true, 0); }, delay); } @@ -439,8 +441,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); } } @@ -461,4 +463,13 @@ export class TooltipComponent { this.hide(0); } } + + /** + * Ensures that the tooltip text has rendered. Mainly used for rendering the initial text + * before positioning a tooltip, which can be problematic in components with OnPush change + * detection. + */ + _updateView(): void { + this._changeDetectorRef.detectChanges(); + } } From 777355e905e874e07980b412dc48acba6fed3f10 Mon Sep 17 00:00:00 2001 From: crisbeto Date: Sat, 18 Mar 2017 18:07:44 +0100 Subject: [PATCH 2/3] fix: switch to markForCheck --- src/lib/tooltip/tooltip.ts | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/lib/tooltip/tooltip.ts b/src/lib/tooltip/tooltip.ts index 4f3d897a2e76..d4116e786b69 100644 --- a/src/lib/tooltip/tooltip.ts +++ b/src/lib/tooltip/tooltip.ts @@ -311,7 +311,7 @@ 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._updateView(); + this._tooltipInstance._markForCheck(); this._ngZone.onMicrotaskEmpty.first().subscribe(() => { if (this._tooltipInstance) { @@ -394,7 +394,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(); setTimeout(() => this._closeOnInteraction = true, 0); }, delay); } @@ -415,7 +415,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); } @@ -465,11 +465,11 @@ export class TooltipComponent { } /** - * Ensures that the tooltip text has rendered. Mainly used for rendering the initial text - * before positioning a tooltip, which can be problematic in components with OnPush change - * detection. + * 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. */ - _updateView(): void { - this._changeDetectorRef.detectChanges(); + _markForCheck(): void { + this._changeDetectorRef.markForCheck(); } } From bb9c97057c50b370d75aaf5966a1e9ea73eb6378 Mon Sep 17 00:00:00 2001 From: crisbeto Date: Sat, 18 Mar 2017 18:07:55 +0100 Subject: [PATCH 3/3] fix: test failures on iOS --- src/lib/tooltip/tooltip.spec.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/lib/tooltip/tooltip.spec.ts b/src/lib/tooltip/tooltip.spec.ts index c4288fa644b6..9381839b6fe9 100644 --- a/src/lib/tooltip/tooltip.spec.ts +++ b/src/lib/tooltip/tooltip.spec.ts @@ -34,7 +34,7 @@ describe('MdTooltip', () => { imports: [MdTooltipModule.forRoot(), OverlayModule], declarations: [BasicTooltipDemo, ScrollableTooltipDemo, OnPushTooltipDemo], providers: [ - Platform, + {provide: Platform, useValue: {IOS: false}}, {provide: OverlayContainer, useFactory: () => { overlayContainerElement = document.createElement('div'); document.body.appendChild(overlayContainerElement); @@ -435,6 +435,7 @@ describe('MdTooltip', () => { 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;