Skip to content

Commit

Permalink
fix(tooltip): closing immediately when triggered on click (#6590)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
crisbeto authored and mmalerba committed Sep 12, 2017
1 parent 0257f48 commit bcd026f
Show file tree
Hide file tree
Showing 3 changed files with 55 additions and 19 deletions.
3 changes: 2 additions & 1 deletion src/lib/tooltip/tooltip.html
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,5 @@
[ngClass]="tooltipClass"
[style.transform-origin]="_transformOrigin"
[@state]="_visibility"
(@state.done)="_afterVisibilityAnimation($event)">{{message}}</div>
(@state.start)="_animationStart()"
(@state.done)="_animationDone($event)">{{message}}</div>
39 changes: 36 additions & 3 deletions src/lib/tooltip/tooltip.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}));

Expand Down Expand Up @@ -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', () => {
Expand Down
32 changes: 17 additions & 15 deletions src/lib/tooltip/tooltip.ts
Original file line number Diff line number Diff line change
Expand Up @@ -422,10 +422,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)')),
])
Expand Down Expand Up @@ -455,7 +453,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';
Expand All @@ -477,21 +475,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);
}

Expand All @@ -507,7 +497,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
Expand Down Expand Up @@ -543,10 +532,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);
}
}

/**
Expand Down

0 comments on commit bcd026f

Please sign in to comment.