From 37f060345ade2fe8db8f2de5aa5a9631032c2697 Mon Sep 17 00:00:00 2001 From: gabrieljablonski Date: Tue, 2 Jul 2024 19:44:41 -0300 Subject: [PATCH] fix: correctly clear timeout refs --- src/components/Tooltip/Tooltip.tsx | 51 ++++++++++-------------------- src/test/utils.spec.js | 18 ++++++++++- src/utils/clear-timeout-ref.ts | 9 ++++++ src/utils/index.ts | 2 ++ 4 files changed, 45 insertions(+), 35 deletions(-) create mode 100644 src/utils/clear-timeout-ref.ts diff --git a/src/components/Tooltip/Tooltip.tsx b/src/components/Tooltip/Tooltip.tsx index d99345ad..37487927 100644 --- a/src/components/Tooltip/Tooltip.tsx +++ b/src/components/Tooltip/Tooltip.tsx @@ -8,6 +8,7 @@ import { getScrollParent, computeTooltipPosition, cssTimeToMs, + clearTimeoutRef, } from 'utils' import type { IComputedPosition } from 'utils' import { useTooltip } from 'components/TooltipProvider' @@ -223,9 +224,7 @@ const Tooltip = ({ if (show === wasShowing.current) { return } - if (missedTransitionTimerRef.current) { - clearTimeout(missedTransitionTimerRef.current) - } + clearTimeoutRef(missedTransitionTimerRef) wasShowing.current = show if (show) { afterShow?.() @@ -257,9 +256,7 @@ const Tooltip = ({ } const handleShowTooltipDelayed = (delay = delayShow) => { - if (tooltipShowDelayTimerRef.current) { - clearTimeout(tooltipShowDelayTimerRef.current) - } + clearTimeoutRef(tooltipShowDelayTimerRef) if (rendered) { // if the tooltip is already rendered, ignore delay @@ -273,9 +270,7 @@ const Tooltip = ({ } const handleHideTooltipDelayed = (delay = delayHide) => { - if (tooltipHideDelayTimerRef.current) { - clearTimeout(tooltipHideDelayTimerRef.current) - } + clearTimeoutRef(tooltipHideDelayTimerRef) tooltipHideDelayTimerRef.current = setTimeout(() => { if (hoveringTooltip.current) { @@ -307,9 +302,7 @@ const Tooltip = ({ setActiveAnchor(target) setProviderActiveAnchor({ current: target }) - if (tooltipHideDelayTimerRef.current) { - clearTimeout(tooltipHideDelayTimerRef.current) - } + clearTimeoutRef(tooltipHideDelayTimerRef) } const handleHideTooltip = () => { @@ -322,9 +315,7 @@ const Tooltip = ({ handleShow(false) } - if (tooltipShowDelayTimerRef.current) { - clearTimeout(tooltipShowDelayTimerRef.current) - } + clearTimeoutRef(tooltipShowDelayTimerRef) } const handleTooltipPosition = ({ x, y }: IPosition) => { @@ -386,9 +377,7 @@ const Tooltip = ({ return } handleShow(false) - if (tooltipShowDelayTimerRef.current) { - clearTimeout(tooltipShowDelayTimerRef.current) - } + clearTimeoutRef(tooltipShowDelayTimerRef) } // debounce handler to prevent call twice when @@ -697,12 +686,8 @@ const Tooltip = ({ setRendered(false) handleShow(false) setActiveAnchor(null) - if (tooltipShowDelayTimerRef.current) { - clearTimeout(tooltipShowDelayTimerRef.current) - } - if (tooltipHideDelayTimerRef.current) { - clearTimeout(tooltipHideDelayTimerRef.current) - } + clearTimeoutRef(tooltipShowDelayTimerRef) + clearTimeoutRef(tooltipHideDelayTimerRef) return true } return false @@ -790,12 +775,8 @@ const Tooltip = ({ handleShow(true) } return () => { - if (tooltipShowDelayTimerRef.current) { - clearTimeout(tooltipShowDelayTimerRef.current) - } - if (tooltipHideDelayTimerRef.current) { - clearTimeout(tooltipHideDelayTimerRef.current) - } + clearTimeoutRef(tooltipShowDelayTimerRef) + clearTimeoutRef(tooltipHideDelayTimerRef) } }, []) @@ -818,7 +799,11 @@ const Tooltip = ({ useEffect(() => { if (tooltipShowDelayTimerRef.current) { - clearTimeout(tooltipShowDelayTimerRef.current) + /** + * if the delay changes while the tooltip is waiting to show, + * reset the timer with the new delay + */ + clearTimeoutRef(tooltipShowDelayTimerRef) handleShowTooltipDelayed(delayShow) } }, [delayShow]) @@ -875,9 +860,7 @@ const Tooltip = ({ clickable && coreStyles['clickable'], )} onTransitionEnd={(event: TransitionEvent) => { - if (missedTransitionTimerRef.current) { - clearTimeout(missedTransitionTimerRef.current) - } + clearTimeoutRef(missedTransitionTimerRef) if (show || event.propertyName !== 'opacity') { return } diff --git a/src/test/utils.spec.js b/src/test/utils.spec.js index aee537e8..42597289 100644 --- a/src/test/utils.spec.js +++ b/src/test/utils.spec.js @@ -1,4 +1,4 @@ -import { debounce, deepEqual, computeTooltipPosition, cssTimeToMs } from 'utils' +import { debounce, deepEqual, computeTooltipPosition, cssTimeToMs, clearTimeoutRef } from 'utils' describe('compute positions', () => { test('empty reference elements', async () => { @@ -256,3 +256,19 @@ describe('deepEqual', () => { expect(deepEqual(obj1, obj2)).toBe(false) }) }) + +describe('clearTimeoutRef', () => { + jest.useFakeTimers() + + const func = jest.fn() + + test('clears timeout ref', () => { + const timeoutRef = { current: setTimeout(func, 1000) } + clearTimeoutRef(timeoutRef) + + jest.runAllTimers() + + expect(func).not.toHaveBeenCalled() + expect(timeoutRef.current).toBe(null) + }) +}) diff --git a/src/utils/clear-timeout-ref.ts b/src/utils/clear-timeout-ref.ts new file mode 100644 index 00000000..b3411c7f --- /dev/null +++ b/src/utils/clear-timeout-ref.ts @@ -0,0 +1,9 @@ +const clearTimeoutRef = (ref: React.MutableRefObject) => { + if (ref.current) { + clearTimeout(ref.current) + // eslint-disable-next-line no-param-reassign + ref.current = null + } +} + +export default clearTimeoutRef diff --git a/src/utils/index.ts b/src/utils/index.ts index c081ec9c..f5060b00 100644 --- a/src/utils/index.ts +++ b/src/utils/index.ts @@ -6,6 +6,7 @@ import debounce from './debounce' import deepEqual from './deep-equal' import getScrollParent from './get-scroll-parent' import useIsomorphicLayoutEffect from './use-isomorphic-layout-effect' +import clearTimeoutRef from './clear-timeout-ref' export type { IComputedPosition } export { @@ -16,4 +17,5 @@ export { deepEqual, getScrollParent, useIsomorphicLayoutEffect, + clearTimeoutRef, }