Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
fix(material/tooltip): animations running when timeouts haven't elaps…
…ed (#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.
  • Loading branch information
crisbeto committed Sep 26, 2022
1 parent 8c3d791 commit a9d3bc5
Show file tree
Hide file tree
Showing 4 changed files with 19 additions and 21 deletions.
2 changes: 0 additions & 2 deletions src/material-experimental/mdc-tooltip/tooltip.spec.ts
Expand Up @@ -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();
Expand All @@ -538,7 +537,6 @@ describe('MDC-based 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 @@ -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();
Expand All @@ -534,7 +533,6 @@ describe('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 @@ -377,13 +377,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): 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 @@ -405,8 +400,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 @@ -845,13 +846,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 @@ -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) {
Expand Down
4 changes: 1 addition & 3 deletions tools/public_api_guard/material/tooltip.md
Expand Up @@ -174,13 +174,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 @@ -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<HTMLElement>;
tooltipClass: string | string[] | Set<string> | {
[key: string]: any;
};
_triggerElement: HTMLElement;
_visibility: TooltipVisibility;
// (undocumented)
static ɵdir: i0.ɵɵDirectiveDeclaration<_TooltipComponentBase, never, never, {}, {}, never, never, false>;
// (undocumented)
Expand Down

0 comments on commit a9d3bc5

Please sign in to comment.