Skip to content

Commit

Permalink
fix(tooltips): fixes for TooltipDelayGroup, onClose and nested (#1186)
Browse files Browse the repository at this point in the history
* fix(tooltips): trigger onClose only when needed in tooltipDelayGroups

* fix(tooltip): disable check for nested layout groups
  • Loading branch information
pedrobonamin authored and mariuslundgard committed Dec 7, 2023
1 parent 3376f63 commit 345e887
Show file tree
Hide file tree
Showing 3 changed files with 7 additions and 35 deletions.
20 changes: 0 additions & 20 deletions src/primitives/tooltip/tooltip.test.tsx
Expand Up @@ -265,26 +265,6 @@ describe('Tooltip', () => {
act(() => jest.advanceTimersByTime(openDelay / 2))
screen.getByText('Tooltip 2')
})
it("should throw an error if it's nested inside another <TooltipDelayGroupProvider />", () => {
// eslint-disable-next-line no-console
console.error = jest.fn() // Silence console.error as we are actually expecting an error in this test.
expect(() => {
render(
<TooltipDelayGroupProvider delay={100}>
<TooltipDelayGroupProvider delay={100}>
<Tooltip content={<Text size={1}>{'Tooltip 1'}</Text>} placement={'top'} delay={400}>
<Button mode="bleed" text="Button 1" />
</Tooltip>
</TooltipDelayGroupProvider>
</TooltipDelayGroupProvider>,
)
}).toThrow(
'TooltipDelayGroupProvider cannot be nested inside another TooltipDelayGroupProvider',
)

// Clean up the console.error mock
jest.clearAllMocks()
})
})

describe('Closing the <Tooltip /> with the Escape key', () => {
Expand Down
12 changes: 6 additions & 6 deletions src/primitives/tooltip/tooltip.tsx
Expand Up @@ -264,7 +264,7 @@ export const Tooltip = forwardRef(function Tooltip(
// necessary, because the tooltip might not always close as it should (e.g. when clicking
// the reference element triggers a CPU-heavy operation.)
useEffect(() => {
if (!isOpen) return
if (!showTooltip) return

function handleWindowMouseMove(event: MouseEvent) {
if (!referenceElement) return
Expand All @@ -284,17 +284,17 @@ export const Tooltip = forwardRef(function Tooltip(
return () => {
window.removeEventListener('mousemove', handleWindowMouseMove)
}
}, [isOpen, referenceElement, handleIsOpenChange])
}, [showTooltip, referenceElement, handleIsOpenChange])

// Close when `disabled` changes to `true`
useEffect(() => {
if (disabled) handleIsOpenChange(false)
}, [disabled, handleIsOpenChange])
if (disabled && showTooltip) handleIsOpenChange(false)
}, [disabled, handleIsOpenChange, showTooltip])

// Close when `content` changes to falsy
useEffect(() => {
if (!content) handleIsOpenChange(false)
}, [content, handleIsOpenChange])
if (!content && showTooltip) handleIsOpenChange(false)
}, [content, handleIsOpenChange, showTooltip])

// Update reference
useEffect(() => refs.setReference(referenceElement), [referenceElement, refs])
Expand Down
Expand Up @@ -3,7 +3,7 @@ import {useDelayedState} from '../../../hooks/useDelayedState'
import {Delay} from '../../types'
import {TooltipDelayGroupContext} from './tooltipDelayGroupContext'
import {TooltipDelayGroupContextValue} from './types'
import {useTooltipDelayGroup} from './useTooltipDelayGroup'

/**
* @public
* */
Expand Down Expand Up @@ -31,14 +31,6 @@ export function TooltipDelayGroupProvider(
const [isGroupActive, setIsGroupActive] = useDelayedState(false)
const [openTooltipId, setOpenTooltipId] = useDelayedState<string | null>(null)

const isInsideContext = useTooltipDelayGroup()

if (isInsideContext) {
throw new Error(
'TooltipDelayGroupProvider cannot be nested inside another TooltipDelayGroupProvider',
)
}

const openDelay = typeof delay === 'number' ? delay : delay?.open || 0
const closeDelay = typeof delay === 'number' ? delay : delay?.close || 0

Expand Down

0 comments on commit 345e887

Please sign in to comment.