Skip to content

Commit 816f938

Browse files
committed
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.
1 parent dfe01f2 commit 816f938

File tree

3 files changed

+55
-19
lines changed

3 files changed

+55
-19
lines changed

src/lib/tooltip/tooltip.html

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,4 +2,5 @@
22
[ngClass]="tooltipClass"
33
[style.transform-origin]="_transformOrigin"
44
[@state]="_visibility"
5-
(@state.done)="_afterVisibilityAnimation($event)">{{message}}</div>
5+
(@state.start)="_animationStart()"
6+
(@state.done)="_animationDone($event)">{{message}}</div>

src/lib/tooltip/tooltip.spec.ts

Lines changed: 36 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -328,14 +328,14 @@ describe('MdTooltip', () => {
328328
fixture.detectChanges();
329329

330330
// At this point the animation should be able to complete itself and trigger the
331-
// _afterVisibilityAnimation function, but for unknown reasons in the test infrastructure,
331+
// _animationDone function, but for unknown reasons in the test infrastructure,
332332
// this does not occur. Manually call this and verify that doing so does not
333333
// throw an error.
334-
tooltipInstance._afterVisibilityAnimation({
334+
tooltipInstance._animationDone({
335335
fromState: 'visible',
336336
toState: 'hidden',
337337
totalTime: 150,
338-
phaseName: '',
338+
phaseName: 'done',
339339
} as AnimationEvent);
340340
}));
341341

@@ -436,6 +436,39 @@ describe('MdTooltip', () => {
436436

437437
expect(tooltipDirective.message).toBe('100');
438438
}));
439+
440+
it('should hide when clicking away', fakeAsync(() => {
441+
tooltipDirective.show();
442+
tick(0);
443+
fixture.detectChanges();
444+
tick(500);
445+
446+
expect(tooltipDirective._isTooltipVisible()).toBe(true);
447+
expect(overlayContainerElement.textContent).toContain(initialTooltipMessage);
448+
449+
document.body.click();
450+
tick(0);
451+
fixture.detectChanges();
452+
tick(500);
453+
454+
expect(tooltipDirective._isTooltipVisible()).toBe(false);
455+
expect(overlayContainerElement.textContent).toBe('');
456+
}));
457+
458+
it('should not hide immediately if a click fires while animating', fakeAsync(() => {
459+
tooltipDirective.show();
460+
tick(0);
461+
fixture.detectChanges();
462+
463+
document.body.click();
464+
fixture.detectChanges();
465+
466+
tick(500);
467+
468+
expect(tooltipDirective._isTooltipVisible()).toBe(true);
469+
expect(overlayContainerElement.textContent).toContain(initialTooltipMessage);
470+
}));
471+
439472
});
440473

441474
describe('scrollable usage', () => {

src/lib/tooltip/tooltip.ts

Lines changed: 17 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -424,10 +424,8 @@ export type TooltipVisibility = 'initial' | 'visible' | 'hidden';
424424
changeDetection: ChangeDetectionStrategy.OnPush,
425425
animations: [
426426
trigger('state', [
427-
state('void', style({transform: 'scale(0)'})),
428-
state('initial', style({transform: 'scale(0)'})),
427+
state('initial, void, hidden', style({transform: 'scale(0)'})),
429428
state('visible', style({transform: 'scale(1)'})),
430-
state('hidden', style({transform: 'scale(0)'})),
431429
transition('* => visible', animate('150ms cubic-bezier(0.0, 0.0, 0.2, 1)')),
432430
transition('* => hidden', animate('150ms cubic-bezier(0.4, 0.0, 1, 1)')),
433431
])
@@ -457,7 +455,7 @@ export class TooltipComponent {
457455
_visibility: TooltipVisibility = 'initial';
458456

459457
/** Whether interactions on the page should close the tooltip */
460-
_closeOnInteraction: boolean = false;
458+
private _closeOnInteraction: boolean = false;
461459

462460
/** The transform origin used in the animation for showing and hiding the tooltip */
463461
_transformOrigin: string = 'bottom';
@@ -479,21 +477,13 @@ export class TooltipComponent {
479477
clearTimeout(this._hideTimeoutId);
480478
}
481479

482-
// Body interactions should cancel the tooltip if there is a delay in showing.
483-
this._closeOnInteraction = true;
484-
485480
this._setTransformOrigin(position);
486481
this._showTimeoutId = setTimeout(() => {
487482
this._visibility = 'visible';
488483

489-
// If this was set to true immediately, then a body click that triggers show() would
490-
// trigger interaction and close the tooltip right after it was displayed.
491-
this._closeOnInteraction = false;
492-
493484
// Mark for check so if any parent component has set the
494485
// ChangeDetectionStrategy to OnPush it will be checked anyways
495486
this._markForCheck();
496-
setTimeout(() => this._closeOnInteraction = true, 0);
497487
}, delay);
498488
}
499489

@@ -509,7 +499,6 @@ export class TooltipComponent {
509499

510500
this._hideTimeoutId = setTimeout(() => {
511501
this._visibility = 'hidden';
512-
this._closeOnInteraction = false;
513502

514503
// Mark for check so if any parent component has set the
515504
// ChangeDetectionStrategy to OnPush it will be checked anyways
@@ -545,10 +534,23 @@ export class TooltipComponent {
545534
}
546535
}
547536

548-
_afterVisibilityAnimation(e: AnimationEvent): void {
549-
if (e.toState === 'hidden' && !this.isVisible()) {
537+
_animationStart() {
538+
this._closeOnInteraction = false;
539+
}
540+
541+
_animationDone(event: AnimationEvent): void {
542+
const toState = event.toState as TooltipVisibility;
543+
544+
if (toState === 'hidden' && !this.isVisible()) {
550545
this._onHide.next();
551546
}
547+
548+
if (toState === 'visible' || toState === 'hidden') {
549+
// Note: as of Angular 4.3, the animations module seems to fire the `start` callback before
550+
// the end if animations are disabled. Make this call async to ensure that it still fires
551+
// at the appropriate time.
552+
Promise.resolve().then(() => this._closeOnInteraction = true);
553+
}
552554
}
553555

554556
/**

0 commit comments

Comments
 (0)