Skip to content

Commit e6c4d04

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 df28c3d commit e6c4d04

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
@@ -302,14 +302,14 @@ describe('MdTooltip', () => {
302302
fixture.detectChanges();
303303

304304
// At this point the animation should be able to complete itself and trigger the
305-
// _afterVisibilityAnimation function, but for unknown reasons in the test infrastructure,
305+
// _animationDone function, but for unknown reasons in the test infrastructure,
306306
// this does not occur. Manually call this and verify that doing so does not
307307
// throw an error.
308-
tooltipInstance._afterVisibilityAnimation({
308+
tooltipInstance._animationDone({
309309
fromState: 'visible',
310310
toState: 'hidden',
311311
totalTime: 150,
312-
phaseName: '',
312+
phaseName: 'done',
313313
} as AnimationEvent);
314314
}));
315315

@@ -403,6 +403,39 @@ describe('MdTooltip', () => {
403403
expect(tooltipWrapper).toBeTruthy('Expected tooltip to be shown.');
404404
expect(tooltipWrapper.getAttribute('dir')).toBe('rtl', 'Expected tooltip to be in RTL mode.');
405405
}));
406+
407+
it('should hide when clicking away', fakeAsync(() => {
408+
tooltipDirective.show();
409+
tick(0);
410+
fixture.detectChanges();
411+
tick(500);
412+
413+
expect(tooltipDirective._isTooltipVisible()).toBe(true);
414+
expect(overlayContainerElement.textContent).toContain(initialTooltipMessage);
415+
416+
document.body.click();
417+
tick(0);
418+
fixture.detectChanges();
419+
tick(500);
420+
421+
expect(tooltipDirective._isTooltipVisible()).toBe(false);
422+
expect(overlayContainerElement.textContent).toBe('');
423+
}));
424+
425+
it('should not hide immediately if a click fires while animating', fakeAsync(() => {
426+
tooltipDirective.show();
427+
tick(0);
428+
fixture.detectChanges();
429+
430+
document.body.click();
431+
fixture.detectChanges();
432+
433+
tick(500);
434+
435+
expect(tooltipDirective._isTooltipVisible()).toBe(true);
436+
expect(overlayContainerElement.textContent).toContain(initialTooltipMessage);
437+
}));
438+
406439
});
407440

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

src/lib/tooltip/tooltip.ts

Lines changed: 17 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -404,10 +404,8 @@ export type TooltipVisibility = 'initial' | 'visible' | 'hidden';
404404
changeDetection: ChangeDetectionStrategy.OnPush,
405405
animations: [
406406
trigger('state', [
407-
state('void', style({transform: 'scale(0)'})),
408-
state('initial', style({transform: 'scale(0)'})),
407+
state('initial, void, hidden', style({transform: 'scale(0)'})),
409408
state('visible', style({transform: 'scale(1)'})),
410-
state('hidden', style({transform: 'scale(0)'})),
411409
transition('* => visible', animate('150ms cubic-bezier(0.0, 0.0, 0.2, 1)')),
412410
transition('* => hidden', animate('150ms cubic-bezier(0.4, 0.0, 1, 1)')),
413411
])
@@ -436,7 +434,7 @@ export class TooltipComponent {
436434
_visibility: TooltipVisibility = 'initial';
437435

438436
/** Whether interactions on the page should close the tooltip */
439-
_closeOnInteraction: boolean = false;
437+
private _closeOnInteraction: boolean = false;
440438

441439
/** The transform origin used in the animation for showing and hiding the tooltip */
442440
_transformOrigin: string = 'bottom';
@@ -458,21 +456,13 @@ export class TooltipComponent {
458456
clearTimeout(this._hideTimeoutId);
459457
}
460458

461-
// Body interactions should cancel the tooltip if there is a delay in showing.
462-
this._closeOnInteraction = true;
463-
464459
this._setTransformOrigin(position);
465460
this._showTimeoutId = setTimeout(() => {
466461
this._visibility = 'visible';
467462

468-
// If this was set to true immediately, then a body click that triggers show() would
469-
// trigger interaction and close the tooltip right after it was displayed.
470-
this._closeOnInteraction = false;
471-
472463
// Mark for check so if any parent component has set the
473464
// ChangeDetectionStrategy to OnPush it will be checked anyways
474465
this._markForCheck();
475-
setTimeout(() => this._closeOnInteraction = true, 0);
476466
}, delay);
477467
}
478468

@@ -488,7 +478,6 @@ export class TooltipComponent {
488478

489479
this._hideTimeoutId = setTimeout(() => {
490480
this._visibility = 'hidden';
491-
this._closeOnInteraction = false;
492481

493482
// Mark for check so if any parent component has set the
494483
// ChangeDetectionStrategy to OnPush it will be checked anyways
@@ -524,10 +513,23 @@ export class TooltipComponent {
524513
}
525514
}
526515

527-
_afterVisibilityAnimation(e: AnimationEvent): void {
528-
if (e.toState === 'hidden' && !this.isVisible()) {
516+
_animationStart() {
517+
this._closeOnInteraction = false;
518+
}
519+
520+
_animationDone(event: AnimationEvent): void {
521+
const toState = event.toState as TooltipVisibility;
522+
523+
if (toState === 'hidden' && !this.isVisible()) {
529524
this._onHide.next();
530525
}
526+
527+
if (toState === 'visible' || toState === 'hidden') {
528+
// Note: as of Angular 4.3, the animations module seems to fire the `start` callback before
529+
// the end if animations are disabled. Make this call async to ensure that it still fires
530+
// at the appropriate time.
531+
Promise.resolve().then(() => this._closeOnInteraction = true);
532+
}
531533
}
532534

533535
/**

0 commit comments

Comments
 (0)