From a9d3bc5801342583443029cf8d4f71120719e850 Mon Sep 17 00:00:00 2001 From: Kristiyan Kostadinov Date: Mon, 26 Sep 2022 14:43:03 +0200 Subject: [PATCH] fix(material/tooltip): animations running when timeouts haven't elapsed (#25699) (#25701) 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. --- .../mdc-tooltip/tooltip.spec.ts | 2 -- src/material/tooltip/tooltip.spec.ts | 2 -- src/material/tooltip/tooltip.ts | 32 +++++++++++-------- tools/public_api_guard/material/tooltip.md | 4 +-- 4 files changed, 19 insertions(+), 21 deletions(-) diff --git a/src/material-experimental/mdc-tooltip/tooltip.spec.ts b/src/material-experimental/mdc-tooltip/tooltip.spec.ts index 369779bad0e3..acb530b7affc 100644 --- a/src/material-experimental/mdc-tooltip/tooltip.spec.ts +++ b/src/material-experimental/mdc-tooltip/tooltip.spec.ts @@ -519,7 +519,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(); @@ -538,7 +537,6 @@ describe('MDC-based MatTooltip', () => { tooltipDirective.hide(0); tick(0); fixture.detectChanges(); - finishCurrentTooltipAnimation(overlayContainerElement, false); expect(spy).toHaveBeenCalled(); subscription.unsubscribe(); diff --git a/src/material/tooltip/tooltip.spec.ts b/src/material/tooltip/tooltip.spec.ts index 4bc23b88c980..5a4dd8755c43 100644 --- a/src/material/tooltip/tooltip.spec.ts +++ b/src/material/tooltip/tooltip.spec.ts @@ -515,7 +515,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(); @@ -534,7 +533,6 @@ describe('MatTooltip', () => { tooltipDirective.hide(0); tick(0); fixture.detectChanges(); - finishCurrentTooltipAnimation(overlayContainerElement, false); expect(spy).toHaveBeenCalled(); subscription.unsubscribe(); diff --git a/src/material/tooltip/tooltip.ts b/src/material/tooltip/tooltip.ts index 77c9a665acb4..11f98309b006 100644 --- a/src/material/tooltip/tooltip.ts +++ b/src/material/tooltip/tooltip.ts @@ -377,13 +377,8 @@ export abstract class _MatTooltipBase /** Shows the tooltip after the delay in ms, defaults to tooltip-delay-show or 0ms if no input */ show(delay: number = this.showDelay): 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; } @@ -405,8 +400,14 @@ export abstract class _MatTooltipBase /** 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(); + } } } @@ -845,13 +846,10 @@ export abstract class _TooltipComponentBase implements OnDestroy { tooltipClass: string | string[] | Set | {[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; @@ -972,6 +970,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) { diff --git a/tools/public_api_guard/material/tooltip.md b/tools/public_api_guard/material/tooltip.md index efcd8322ff4d..c94963e98766 100644 --- a/tools/public_api_guard/material/tooltip.md +++ b/tools/public_api_guard/material/tooltip.md @@ -174,13 +174,13 @@ export class TooltipComponent extends _TooltipComponentBase { export abstract class _TooltipComponentBase implements OnDestroy { constructor(_changeDetectorRef: ChangeDetectorRef, animationMode?: string); afterHidden(): Observable; + _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; @@ -190,13 +190,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; tooltipClass: string | string[] | Set | { [key: string]: any; }; _triggerElement: HTMLElement; - _visibility: TooltipVisibility; // (undocumented) static ɵdir: i0.ɵɵDirectiveDeclaration<_TooltipComponentBase, never, never, {}, {}, never, never, false>; // (undocumented)