Skip to content

Commit bcd026f

Browse files
crisbetommalerba
authored andcommitted
fix(tooltip): closing immediately when triggered on click (angular#6590)
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 angular#6552.
1 parent 0257f48 commit bcd026f

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
@@ -422,10 +422,8 @@ export type TooltipVisibility = 'initial' | 'visible' | 'hidden';
422422
changeDetection: ChangeDetectionStrategy.OnPush,
423423
animations: [
424424
trigger('state', [
425-
state('void', style({transform: 'scale(0)'})),
426-
state('initial', style({transform: 'scale(0)'})),
425+
state('initial, void, hidden', style({transform: 'scale(0)'})),
427426
state('visible', style({transform: 'scale(1)'})),
428-
state('hidden', style({transform: 'scale(0)'})),
429427
transition('* => visible', animate('150ms cubic-bezier(0.0, 0.0, 0.2, 1)')),
430428
transition('* => hidden', animate('150ms cubic-bezier(0.4, 0.0, 1, 1)')),
431429
])
@@ -455,7 +453,7 @@ export class TooltipComponent {
455453
_visibility: TooltipVisibility = 'initial';
456454

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

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

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

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

@@ -507,7 +497,6 @@ export class TooltipComponent {
507497

508498
this._hideTimeoutId = setTimeout(() => {
509499
this._visibility = 'hidden';
510-
this._closeOnInteraction = false;
511500

512501
// Mark for check so if any parent component has set the
513502
// ChangeDetectionStrategy to OnPush it will be checked anyways
@@ -543,10 +532,23 @@ export class TooltipComponent {
543532
}
544533
}
545534

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

552554
/**

0 commit comments

Comments
 (0)