Skip to content

Commit

Permalink
fix: correctly clear timeout refs
Browse files Browse the repository at this point in the history
  • Loading branch information
gabrieljablonski committed Jul 3, 2024
1 parent 329f432 commit 3e34982
Show file tree
Hide file tree
Showing 4 changed files with 45 additions and 35 deletions.
51 changes: 17 additions & 34 deletions src/components/Tooltip/Tooltip.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import {
getScrollParent,
computeTooltipPosition,
cssTimeToMs,
clearTimeoutRef,
} from 'utils'
import type { IComputedPosition } from 'utils'
import { useTooltip } from 'components/TooltipProvider'
Expand Down Expand Up @@ -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?.()
Expand Down Expand Up @@ -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
Expand All @@ -273,9 +270,7 @@ const Tooltip = ({
}

const handleHideTooltipDelayed = (delay = delayHide) => {
if (tooltipHideDelayTimerRef.current) {
clearTimeout(tooltipHideDelayTimerRef.current)
}
clearTimeoutRef(tooltipHideDelayTimerRef)

tooltipHideDelayTimerRef.current = setTimeout(() => {
if (hoveringTooltip.current) {
Expand Down Expand Up @@ -307,9 +302,7 @@ const Tooltip = ({
setActiveAnchor(target)
setProviderActiveAnchor({ current: target })

if (tooltipHideDelayTimerRef.current) {
clearTimeout(tooltipHideDelayTimerRef.current)
}
clearTimeoutRef(tooltipHideDelayTimerRef)
}

const handleHideTooltip = () => {
Expand All @@ -322,9 +315,7 @@ const Tooltip = ({
handleShow(false)
}

if (tooltipShowDelayTimerRef.current) {
clearTimeout(tooltipShowDelayTimerRef.current)
}
clearTimeoutRef(tooltipShowDelayTimerRef)
}

const handleTooltipPosition = ({ x, y }: IPosition) => {
Expand Down Expand Up @@ -386,9 +377,7 @@ const Tooltip = ({
return
}
handleShow(false)
if (tooltipShowDelayTimerRef.current) {
clearTimeout(tooltipShowDelayTimerRef.current)
}
clearTimeoutRef(tooltipShowDelayTimerRef)
}

// debounce handler to prevent call twice when
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)
}
}, [])

Expand All @@ -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])
Expand Down Expand Up @@ -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
}
Expand Down
18 changes: 17 additions & 1 deletion src/test/utils.spec.js
Original file line number Diff line number Diff line change
@@ -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 () => {
Expand Down Expand Up @@ -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)
})
})
9 changes: 9 additions & 0 deletions src/utils/clear-timeout-ref.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
const clearTimeoutRef = (ref: React.MutableRefObject<NodeJS.Timeout | null>) => {
if (ref.current) {
clearTimeout(ref.current)
// eslint-disable-next-line no-param-reassign
ref.current = null
}
}

export default clearTimeoutRef
2 changes: 2 additions & 0 deletions src/utils/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -16,4 +17,5 @@ export {
deepEqual,
getScrollParent,
useIsomorphicLayoutEffect,
clearTimeoutRef,
}

0 comments on commit 3e34982

Please sign in to comment.