Skip to content

Commit

Permalink
fix(material/tooltip): animations running when timeouts haven't elaps…
Browse files Browse the repository at this point in the history
…ed (#25699)

After the tooltips were switched to CSS animations, a regression was introduced where the opposite animation is shown even if the tooltip didn't actually reach its target state.

These changes are an alternate take on the fix from #24652 which had to be reverted due to internal failures.

Fixes #24614.
  • Loading branch information
crisbeto committed Sep 26, 2022
1 parent 90e6446 commit 218297a
Show file tree
Hide file tree
Showing 4 changed files with 19 additions and 21 deletions.
2 changes: 0 additions & 2 deletions src/material/legacy-tooltip/tooltip.spec.ts
Expand Up @@ -586,7 +586,6 @@ describe('MatTooltip', () => {
const tooltipDelay = 1000;
tooltipDirective.hide();
tick(tooltipDelay); // Change the tooltip state to hidden and trigger animation start
finishCurrentTooltipAnimation(overlayContainerElement, false);

fixture.componentInstance.showButton = false;
fixture.detectChanges();
Expand All @@ -605,7 +604,6 @@ describe('MatTooltip', () => {
tooltipDirective.hide(0);
tick(0);
fixture.detectChanges();
finishCurrentTooltipAnimation(overlayContainerElement, false);

expect(spy).toHaveBeenCalled();
subscription.unsubscribe();
Expand Down
2 changes: 0 additions & 2 deletions src/material/tooltip/tooltip.spec.ts
Expand Up @@ -591,7 +591,6 @@ describe('MDC-based MatTooltip', () => {
const tooltipDelay = 1000;
tooltipDirective.hide();
tick(tooltipDelay); // Change the tooltip state to hidden and trigger animation start
finishCurrentTooltipAnimation(overlayContainerElement, false);

fixture.componentInstance.showButton = false;
fixture.detectChanges();
Expand All @@ -610,7 +609,6 @@ describe('MDC-based MatTooltip', () => {
tooltipDirective.hide(0);
tick(0);
fixture.detectChanges();
finishCurrentTooltipAnimation(overlayContainerElement, false);

expect(spy).toHaveBeenCalled();
subscription.unsubscribe();
Expand Down
32 changes: 18 additions & 14 deletions src/material/tooltip/tooltip.ts
Expand Up @@ -408,13 +408,8 @@ export abstract class _MatTooltipBase<T extends _TooltipComponentBase>

/** Shows the tooltip after the delay in ms, defaults to tooltip-delay-show or 0ms if no input */
show(delay: number = this.showDelay, origin?: {x: number; y: number}): void {
if (
this.disabled ||
!this.message ||
(this._isTooltipVisible() &&
!this._tooltipInstance!._showTimeoutId &&
!this._tooltipInstance!._hideTimeoutId)
) {
if (this.disabled || !this.message || this._isTooltipVisible()) {
this._tooltipInstance?._cancelPendingHide();
return;
}

Expand All @@ -436,8 +431,14 @@ export abstract class _MatTooltipBase<T extends _TooltipComponentBase>

/** Hides the tooltip after the delay in ms, defaults to tooltip-delay-hide or 0ms if no input */
hide(delay: number = this.hideDelay): void {
if (this._tooltipInstance) {
this._tooltipInstance.hide(delay);
const instance = this._tooltipInstance;

if (instance) {
if (instance.isVisible()) {
instance.hide(delay);
} else {
this._detach();
}
}
}

Expand Down Expand Up @@ -908,13 +909,10 @@ export abstract class _TooltipComponentBase implements OnDestroy {
tooltipClass: string | string[] | Set<string> | {[key: string]: any};

/** The timeout ID of any current timer set to show the tooltip */
_showTimeoutId: number | undefined;
private _showTimeoutId: number | undefined;

/** The timeout ID of any current timer set to hide the tooltip */
_hideTimeoutId: number | undefined;

/** Property watched by the animation framework to show or hide the tooltip */
_visibility: TooltipVisibility = 'initial';
private _hideTimeoutId: number | undefined;

/** Element that caused the tooltip to open. */
_triggerElement: HTMLElement;
Expand Down Expand Up @@ -1035,6 +1033,12 @@ export abstract class _TooltipComponentBase implements OnDestroy {
}
}

/** Cancels any pending hiding sequences. */
_cancelPendingHide() {
clearTimeout(this._hideTimeoutId);
this._hideTimeoutId = undefined;
}

/** Handles the cleanup after an animation has finished. */
private _finalizeAnimation(toVisible: boolean) {
if (toVisible) {
Expand Down
4 changes: 1 addition & 3 deletions tools/public_api_guard/material/tooltip.md
Expand Up @@ -188,13 +188,13 @@ export class TooltipComponent extends _TooltipComponentBase {
export abstract class _TooltipComponentBase implements OnDestroy {
constructor(_changeDetectorRef: ChangeDetectorRef, animationMode?: string);
afterHidden(): Observable<void>;
_cancelPendingHide(): void;
_handleAnimationEnd({ animationName }: AnimationEvent): void;
_handleBodyInteraction(): void;
// (undocumented)
_handleMouseLeave({ relatedTarget }: MouseEvent): void;
hide(delay: number): void;
protected abstract readonly _hideAnimation: string;
_hideTimeoutId: number | undefined;
isVisible(): boolean;
_markForCheck(): void;
message: string;
Expand All @@ -204,13 +204,11 @@ export abstract class _TooltipComponentBase implements OnDestroy {
protected _onShow(): void;
show(delay: number): void;
protected abstract readonly _showAnimation: string;
_showTimeoutId: number | undefined;
abstract _tooltip: ElementRef<HTMLElement>;
tooltipClass: string | string[] | Set<string> | {
[key: string]: any;
};
_triggerElement: HTMLElement;
_visibility: TooltipVisibility;
// (undocumented)
static ɵdir: i0.ɵɵDirectiveDeclaration<_TooltipComponentBase, never, never, {}, {}, never, never, false, never>;
// (undocumented)
Expand Down

0 comments on commit 218297a

Please sign in to comment.