Skip to content

Commit b92168e

Browse files
committed
fix(material/tooltip): clear pending timers on destroy
Currently we fire off some timers when a tooltip is shown or hidden, however if it gets destroyed before the timer has elapsed, we still end up executing some logic (change detection in this case). These changes clear the timers so that we don't end up triggering errors by accident.
1 parent 0930dc3 commit b92168e

File tree

4 files changed

+67
-17
lines changed

4 files changed

+67
-17
lines changed

src/material-experimental/mdc-tooltip/tooltip.spec.ts

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -750,6 +750,33 @@ describe('MDC-based MatTooltip', () => {
750750
expect(overlayRef.detach).not.toHaveBeenCalled();
751751
}));
752752

753+
it('should clear the show timeout on destroy', fakeAsync(() => {
754+
assertTooltipInstance(tooltipDirective, false);
755+
756+
tooltipDirective.show(1000);
757+
fixture.detectChanges();
758+
759+
// Note that we aren't asserting anything, but `fakeAsync` will
760+
// throw if we have any timers by the end of the test.
761+
fixture.destroy();
762+
}));
763+
764+
it('should clear the hide timeout on destroy', fakeAsync(() => {
765+
assertTooltipInstance(tooltipDirective, false);
766+
767+
tooltipDirective.show();
768+
tick(0);
769+
fixture.detectChanges();
770+
tick(500);
771+
772+
tooltipDirective.hide(1000);
773+
fixture.detectChanges();
774+
775+
// Note that we aren't asserting anything, but `fakeAsync` will
776+
// throw if we have any timers by the end of the test.
777+
fixture.destroy();
778+
}));
779+
753780
});
754781

755782
describe('fallback positions', () => {

src/material/tooltip/tooltip.spec.ts

Lines changed: 30 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -112,17 +112,17 @@ describe('MatTooltip', () => {
112112

113113
fixture.detectChanges();
114114

115-
// wait till animation has finished
115+
// Wait until the animation has finished.
116116
tick(500);
117117

118-
// Make sure tooltip is shown to the user and animation has finished
118+
// Make sure tooltip is shown to the user and animation has finished.
119119
const tooltipElement = overlayContainerElement.querySelector('.mat-tooltip') as HTMLElement;
120120
expect(tooltipElement instanceof HTMLElement).toBe(true);
121121
expect(tooltipElement.style.transform).toBe('scale(1)');
122122

123123
expect(overlayContainerElement.textContent).toContain(initialTooltipMessage);
124124

125-
// After hide called, a timeout delay is created that will to hide the tooltip.
125+
// After hide is called, a timeout delay is created that will to hide the tooltip.
126126
const tooltipDelay = 1000;
127127
tooltipDirective.hide(tooltipDelay);
128128
expect(tooltipDirective._isTooltipVisible()).toBe(true);
@@ -749,6 +749,33 @@ describe('MatTooltip', () => {
749749
expect(overlayRef.detach).not.toHaveBeenCalled();
750750
}));
751751

752+
it('should clear the show timeout on destroy', fakeAsync(() => {
753+
assertTooltipInstance(tooltipDirective, false);
754+
755+
tooltipDirective.show(1000);
756+
fixture.detectChanges();
757+
758+
// Note that we aren't asserting anything, but `fakeAsync` will
759+
// throw if we have any timers by the end of the test.
760+
fixture.destroy();
761+
}));
762+
763+
it('should clear the hide timeout on destroy', fakeAsync(() => {
764+
assertTooltipInstance(tooltipDirective, false);
765+
766+
tooltipDirective.show();
767+
tick(0);
768+
fixture.detectChanges();
769+
tick(500);
770+
771+
tooltipDirective.hide(1000);
772+
fixture.detectChanges();
773+
774+
// Note that we aren't asserting anything, but `fakeAsync` will
775+
// throw if we have any timers by the end of the test.
776+
fixture.destroy();
777+
}));
778+
752779
});
753780

754781
describe('fallback positions', () => {

src/material/tooltip/tooltip.ts

Lines changed: 8 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -727,10 +727,10 @@ export abstract class _TooltipComponentBase implements OnDestroy {
727727
tooltipClass: string|string[]|Set<string>|{[key: string]: any};
728728

729729
/** The timeout ID of any current timer set to show the tooltip */
730-
_showTimeoutId: number | null;
730+
_showTimeoutId: number | undefined;
731731

732732
/** The timeout ID of any current timer set to hide the tooltip */
733-
_hideTimeoutId: number | null;
733+
_hideTimeoutId: number | undefined;
734734

735735
/** Property watched by the animation framework to show or hide the tooltip */
736736
_visibility: TooltipVisibility = 'initial';
@@ -749,16 +749,13 @@ export abstract class _TooltipComponentBase implements OnDestroy {
749749
*/
750750
show(delay: number): void {
751751
// Cancel the delayed hide if it is scheduled
752-
if (this._hideTimeoutId) {
753-
clearTimeout(this._hideTimeoutId);
754-
this._hideTimeoutId = null;
755-
}
752+
clearTimeout(this._hideTimeoutId);
756753

757754
// Body interactions should cancel the tooltip if there is a delay in showing.
758755
this._closeOnInteraction = true;
759756
this._showTimeoutId = setTimeout(() => {
760757
this._visibility = 'visible';
761-
this._showTimeoutId = null;
758+
this._showTimeoutId = undefined;
762759

763760
// Mark for check so if any parent component has set the
764761
// ChangeDetectionStrategy to OnPush it will be checked anyways
@@ -772,14 +769,11 @@ export abstract class _TooltipComponentBase implements OnDestroy {
772769
*/
773770
hide(delay: number): void {
774771
// Cancel the delayed show if it is scheduled
775-
if (this._showTimeoutId) {
776-
clearTimeout(this._showTimeoutId);
777-
this._showTimeoutId = null;
778-
}
772+
clearTimeout(this._showTimeoutId);
779773

780774
this._hideTimeoutId = setTimeout(() => {
781775
this._visibility = 'hidden';
782-
this._hideTimeoutId = null;
776+
this._hideTimeoutId = undefined;
783777

784778
// Mark for check so if any parent component has set the
785779
// ChangeDetectionStrategy to OnPush it will be checked anyways
@@ -798,6 +792,8 @@ export abstract class _TooltipComponentBase implements OnDestroy {
798792
}
799793

800794
ngOnDestroy() {
795+
clearTimeout(this._showTimeoutId);
796+
clearTimeout(this._hideTimeoutId);
801797
this._onHide.complete();
802798
}
803799

tools/public_api_guard/material/tooltip.d.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -45,8 +45,8 @@ export declare abstract class _MatTooltipBase<T extends _TooltipComponentBase> i
4545
}
4646

4747
export declare abstract class _TooltipComponentBase implements OnDestroy {
48-
_hideTimeoutId: number | null;
49-
_showTimeoutId: number | null;
48+
_hideTimeoutId: number | undefined;
49+
_showTimeoutId: number | undefined;
5050
_visibility: TooltipVisibility;
5151
message: string;
5252
tooltipClass: string | string[] | Set<string> | {

0 commit comments

Comments
 (0)