From 816f938c3ec04a4264b2a8416bba1e0d6b2d8372 Mon Sep 17 00:00:00 2001 From: crisbeto Date: Tue, 22 Aug 2017 23:20:07 +0200 Subject: [PATCH] fix(tooltip): closing immediately when triggered on click Fixes the tooltip being closed immediately if it is opened as a result of a click. It seems like the logic that was supposed to handle this in master fires before the event has had the chance to bubble up to the body. These changes switch to relying on Angular's animation events to disable the body click. Relates to #6552. --- src/lib/tooltip/tooltip.html | 3 ++- src/lib/tooltip/tooltip.spec.ts | 39 ++++++++++++++++++++++++++++++--- src/lib/tooltip/tooltip.ts | 32 ++++++++++++++------------- 3 files changed, 55 insertions(+), 19 deletions(-) diff --git a/src/lib/tooltip/tooltip.html b/src/lib/tooltip/tooltip.html index 1da0d330d3ea..8f9981cf2e24 100644 --- a/src/lib/tooltip/tooltip.html +++ b/src/lib/tooltip/tooltip.html @@ -2,4 +2,5 @@ [ngClass]="tooltipClass" [style.transform-origin]="_transformOrigin" [@state]="_visibility" - (@state.done)="_afterVisibilityAnimation($event)">{{message}} + (@state.start)="_animationStart()" + (@state.done)="_animationDone($event)">{{message}} diff --git a/src/lib/tooltip/tooltip.spec.ts b/src/lib/tooltip/tooltip.spec.ts index a251597ddc52..d05fd3e983e4 100644 --- a/src/lib/tooltip/tooltip.spec.ts +++ b/src/lib/tooltip/tooltip.spec.ts @@ -328,14 +328,14 @@ describe('MdTooltip', () => { fixture.detectChanges(); // At this point the animation should be able to complete itself and trigger the - // _afterVisibilityAnimation function, but for unknown reasons in the test infrastructure, + // _animationDone function, but for unknown reasons in the test infrastructure, // this does not occur. Manually call this and verify that doing so does not // throw an error. - tooltipInstance._afterVisibilityAnimation({ + tooltipInstance._animationDone({ fromState: 'visible', toState: 'hidden', totalTime: 150, - phaseName: '', + phaseName: 'done', } as AnimationEvent); })); @@ -436,6 +436,39 @@ describe('MdTooltip', () => { expect(tooltipDirective.message).toBe('100'); })); + + it('should hide when clicking away', fakeAsync(() => { + tooltipDirective.show(); + tick(0); + fixture.detectChanges(); + tick(500); + + expect(tooltipDirective._isTooltipVisible()).toBe(true); + expect(overlayContainerElement.textContent).toContain(initialTooltipMessage); + + document.body.click(); + tick(0); + fixture.detectChanges(); + tick(500); + + expect(tooltipDirective._isTooltipVisible()).toBe(false); + expect(overlayContainerElement.textContent).toBe(''); + })); + + it('should not hide immediately if a click fires while animating', fakeAsync(() => { + tooltipDirective.show(); + tick(0); + fixture.detectChanges(); + + document.body.click(); + fixture.detectChanges(); + + tick(500); + + expect(tooltipDirective._isTooltipVisible()).toBe(true); + expect(overlayContainerElement.textContent).toContain(initialTooltipMessage); + })); + }); describe('scrollable usage', () => { diff --git a/src/lib/tooltip/tooltip.ts b/src/lib/tooltip/tooltip.ts index d5a7c14169a6..6b9045fea051 100644 --- a/src/lib/tooltip/tooltip.ts +++ b/src/lib/tooltip/tooltip.ts @@ -424,10 +424,8 @@ export type TooltipVisibility = 'initial' | 'visible' | 'hidden'; changeDetection: ChangeDetectionStrategy.OnPush, animations: [ trigger('state', [ - state('void', style({transform: 'scale(0)'})), - state('initial', style({transform: 'scale(0)'})), + state('initial, void, hidden', style({transform: 'scale(0)'})), state('visible', style({transform: 'scale(1)'})), - state('hidden', style({transform: 'scale(0)'})), transition('* => visible', animate('150ms cubic-bezier(0.0, 0.0, 0.2, 1)')), transition('* => hidden', animate('150ms cubic-bezier(0.4, 0.0, 1, 1)')), ]) @@ -457,7 +455,7 @@ export class TooltipComponent { _visibility: TooltipVisibility = 'initial'; /** Whether interactions on the page should close the tooltip */ - _closeOnInteraction: boolean = false; + private _closeOnInteraction: boolean = false; /** The transform origin used in the animation for showing and hiding the tooltip */ _transformOrigin: string = 'bottom'; @@ -479,21 +477,13 @@ export class TooltipComponent { clearTimeout(this._hideTimeoutId); } - // Body interactions should cancel the tooltip if there is a delay in showing. - this._closeOnInteraction = true; - this._setTransformOrigin(position); this._showTimeoutId = setTimeout(() => { this._visibility = 'visible'; - // 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._markForCheck(); - setTimeout(() => this._closeOnInteraction = true, 0); }, delay); } @@ -509,7 +499,6 @@ 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 @@ -545,10 +534,23 @@ export class TooltipComponent { } } - _afterVisibilityAnimation(e: AnimationEvent): void { - if (e.toState === 'hidden' && !this.isVisible()) { + _animationStart() { + this._closeOnInteraction = false; + } + + _animationDone(event: AnimationEvent): void { + const toState = event.toState as TooltipVisibility; + + if (toState === 'hidden' && !this.isVisible()) { this._onHide.next(); } + + if (toState === 'visible' || toState === 'hidden') { + // Note: as of Angular 4.3, the animations module seems to fire the `start` callback before + // the end if animations are disabled. Make this call async to ensure that it still fires + // at the appropriate time. + Promise.resolve().then(() => this._closeOnInteraction = true); + } } /**