fix(Tooltip): make tooltips with empty text inert

.changeset/tooltip-empty-text.md

@@ -0,0 +1,5 @@
+---
+'@primer/react': patch
+---
+
+Tooltip: An empty `text` no longer opens an empty tooltip or labels/describes the trigger, so a tooltip can be turned off conditionally without remounting its trigger.

packages/react/src/TooltipV2/Tooltip.docs.json

@@ -32,6 +32,9 @@
     },
     {
       "id": "components-tooltipv2-features--on-action-menu-anchor"
+    },
+    {
+      "id": "components-tooltipv2-features--conditional-text"
     }
   ],
   "props": [
@@ -45,7 +48,7 @@
       "name": "text",
       "required": true,
       "type": "string",
-      "description": "The text to be displayed in the tooltip"
+      "description": "The text to be displayed in the tooltip. When this is an empty string and there is no `keybindingHint`, the tooltip does not open and does not label or describe the trigger. Use this to turn a tooltip off conditionally while keeping the trigger mounted, so that it keeps focus."
     },
     {
       "name": "type",

packages/react/src/TooltipV2/Tooltip.features.stories.tsx

@@ -1,3 +1,4 @@
+import {useState} from 'react'
 import {IconButton, Button} from '../Button'
 import Link from '../Link'
 import {ActionMenu} from '../ActionMenu'
@@ -222,3 +223,15 @@ export const WithLongDelay = () => (
     </Tooltip>
   </div>
 )
+
+// An empty `text` turns the tooltip off without unmounting the trigger, so the trigger keeps focus
+export const ConditionalText = () => {
+  const [showTooltip, setShowTooltip] = useState(true)
+  return (
+    <div className={classes.PaddedContainer}>
+      <Tooltip text={showTooltip ? 'Press to turn this tooltip off' : ''}>
+        <Button onClick={() => setShowTooltip(!showTooltip)}>Toggle tooltip</Button>
+      </Tooltip>
+    </div>
+  )
+}

packages/react/src/TooltipV2/Tooltip.tsx

@@ -12,13 +12,19 @@ import {getAccessibleKeybindingHintString, KeybindingHint, type KeybindingHintPr
 import {usePlatform} from '../KeybindingHint/platform'
 import VisuallyHidden from '../_VisuallyHidden'
 import useSafeTimeout from '../hooks/useSafeTimeout'
+import {useEffectCallback} from '../internal/hooks/useEffectCallback'
 import type {SlotMarker} from '../utils/types'
 import {TooltipContext} from './TooltipContext'
 import {reactMajorVersion} from '../utils/environment'
 
 export type TooltipDirection = 'nw' | 'n' | 'ne' | 'e' | 'se' | 's' | 'sw' | 'w'
 export type TooltipProps = React.PropsWithChildren<{
   direction?: TooltipDirection
+  /**
+   * The text to display in the tooltip. When this is an empty string and there is no `keybindingHint`, the tooltip
+   * does not open and does not label or describe the trigger. Use this to turn a tooltip off conditionally while
+   * keeping the trigger mounted, so that it keeps focus.
+   */
   text: string
   type?: 'label' | 'description'
   keybindingHint?: KeybindingHintProps['keys'] | Array<KeybindingHintProps['keys']>
@@ -146,6 +152,10 @@ export const Tooltip: ForwardRefExoticComponent<
     const appliedTriggerRef = mergedRefEnabled ? mergedTriggerRef : providedOrCreatedRef
     const tooltipElRef = useRef<HTMLDivElement>(null)
 
+    // Normalize keybindingHint to an array for uniform rendering
+    const keybindingHints = Array.isArray(keybindingHint) ? keybindingHint : [keybindingHint]
+    const hasContent = Boolean(text) || keybindingHints.length > 0
+
     const [calculatedDirection, setCalculatedDirection] = useState<TooltipDirection>(direction)
 
     const [isPopoverOpen, setIsPopoverOpen] = useState(false)
@@ -161,7 +171,8 @@ export const Tooltip: ForwardRefExoticComponent<
           readTriggerRef.current instanceof HTMLElement &&
           tooltipElRef.current.hasAttribute('popover') &&
           !tooltipElRef.current.matches(':popover-open') &&
-          !_privateDisableTooltip
+          !_privateDisableTooltip &&
+          hasContent
         ) {
           const tooltip = tooltipElRef.current
           const trigger = readTriggerRef.current
@@ -197,11 +208,7 @@ export const Tooltip: ForwardRefExoticComponent<
         }
       }
     }
-    const closeTooltip = () => {
-      if (openTimeoutRef.current) {
-        safeClearTimeout(openTimeoutRef.current)
-        openTimeoutRef.current = null
-      }
+    const hideTooltip = () => {
       try {
         if (
           tooltipElRef.current &&
@@ -231,6 +238,25 @@ export const Tooltip: ForwardRefExoticComponent<
       }
     }
 
+    const closeTooltip = () => {
+      if (openTimeoutRef.current) {
+        safeClearTimeout(openTimeoutRef.current)
+        openTimeoutRef.current = null
+      }
+      hideTooltip()
+    }
+
+    // A pending hover open outlives re-renders, so it must read the latest props when it fires.
+    const openTooltipFromTimeout = useEffectCallback(openTooltip)
+
+    // Hide the tooltip if its content is removed while it is open. This syncs the imperative popover state with props,
+    // so it can't move to an event handler. A pending hover open is kept, since it checks the content when it fires.
+    const hideTooltipFromEffect = useEffectCallback(hideTooltip)
+    useEffect(() => {
+      // eslint-disable-next-line react-you-might-not-need-an-effect/no-event-handler
+      if (!hasContent) hideTooltipFromEffect()
+    }, [hasContent, hideTooltipFromEffect])
+
     // context value
     const value = useMemo(() => ({tooltipId}), [tooltipId])
 
@@ -302,9 +328,6 @@ export const Tooltip: ForwardRefExoticComponent<
     const platform = usePlatform()
     const hasAriaLabel = 'aria-label' in rest
 
-    // Normalize keybindingHint to an array for uniform rendering
-    const keybindingHints = Array.isArray(keybindingHint) ? keybindingHint : [keybindingHint]
-
     if (isFragmentTrigger) return child
 
     return (
@@ -316,8 +339,8 @@ export const Tooltip: ForwardRefExoticComponent<
               ref: appliedTriggerRef,
               // If it is a type description, we use tooltip to describe the trigger
               'aria-describedby': (() => {
-                // If tooltip is not a description type, keep the original aria-describedby
-                if (type !== 'description') {
+                // If tooltip is not a description type or has no content, keep the original aria-describedby
+                if (type !== 'description' || !hasContent) {
                   return child.props['aria-describedby']
                 }
 
@@ -331,7 +354,7 @@ export const Tooltip: ForwardRefExoticComponent<
                 return tooltipId
               })(),
               // If it is a label type, we use tooltip to label the trigger
-              'aria-labelledby': type === 'label' ? tooltipId : child.props['aria-labelledby'],
+              'aria-labelledby': type === 'label' && hasContent ? tooltipId : child.props['aria-labelledby'],
               onBlur: (event: React.FocusEvent) => {
                 closeTooltip()
                 child.props.onBlur?.(event)
@@ -364,7 +387,7 @@ export const Tooltip: ForwardRefExoticComponent<
                 openTimeoutRef.current = safeSetTimeout(() => {
                   // if the mouse is already moved out, do not show the tooltip
                   if (!openTimeoutRef.current) return
-                  openTooltip()
+                  openTooltipFromTimeout()
                   child.props.onMouseEnter?.(event)
                 }, delayTime)
               },

packages/react/src/TooltipV2/__tests__/Tooltip.test.tsx

@@ -1,8 +1,9 @@
 import type React from 'react'
 import {describe, expect, it, vi} from 'vitest'
+import {page, userEvent} from 'vitest/browser'
 import type {TooltipProps} from '../Tooltip'
 import {Tooltip} from '../Tooltip'
-import {fireEvent, render as HTMLRender} from '@testing-library/react'
+import {act, fireEvent, render as HTMLRender} from '@testing-library/react'
 import BaseStyles from '../../BaseStyles'
 import {Button, IconButton} from '../../Button'
 import {ActionMenu} from '../../ActionMenu'
@@ -12,7 +13,7 @@ import {XIcon} from '@primer/octicons-react'
 import classes from '../Tooltip.module.css'
 
 import type {JSX} from 'react'
-import {createRef, forwardRef, useImperativeHandle} from 'react'
+import {createRef, forwardRef, useImperativeHandle, useState} from 'react'
 import {FeatureFlags} from '../../FeatureFlags'
 import {implementsClassName, withExpectedConsoleError, withExpectedConsoleWarning} from '../../utils/testing'
 
@@ -239,6 +240,123 @@ describe('Tooltip', () => {
   })
 })
 
+describe('Tooltip with empty text', () => {
+  const getTooltipElement = (container: HTMLElement) =>
+    container.querySelector<HTMLElement>('[data-component="Tooltip"]')!
+
+  it('does not describe the trigger', () => {
+    const {getByRole} = HTMLRender(<TooltipComponent text="" />)
+    expect(getByRole('button')).not.toHaveAttribute('aria-describedby')
+  })
+
+  it('does not label the trigger when the tooltip type is label', () => {
+    const {getByRole} = HTMLRender(<TooltipComponent text="" type="label" />)
+    const trigger = getByRole('button')
+    expect(trigger).not.toHaveAttribute('aria-labelledby')
+    expect(trigger).toHaveAccessibleName('Button Text')
+  })
+
+  it("keeps the trigger's own aria-describedby", () => {
+    const {getByRole} = HTMLRender(<TooltipComponentWithExistingDescription text="" />)
+    expect(getByRole('button')).toHaveAttribute('aria-describedby', 'external-description')
+  })
+
+  it('still labels the trigger when a keybinding hint is provided', () => {
+    const {getByRole, container} = HTMLRender(<TooltipComponent text="" type="label" keybindingHint="Control+K" />)
+    expect(getByRole('button')).toHaveAttribute(
+      'aria-labelledby',
+      getTooltipElement(container).querySelector('[id]')!.id,
+    )
+  })
+
+  it('does not open on keyboard focus', async () => {
+    const {container} = HTMLRender(<TooltipComponent text="" />)
+    await userEvent.tab()
+    expect(page.getByRole('button', {name: 'Button Text', exact: true}).element()).toHaveFocus()
+    expect(getTooltipElement(container).matches(':popover-open')).toBe(false)
+  })
+
+  it('closes an open tooltip when the text becomes empty', async () => {
+    const {container, rerender} = HTMLRender(<TooltipComponent />)
+    const tooltip = getTooltipElement(container)
+    await userEvent.tab()
+    expect(tooltip.matches(':popover-open')).toBe(true)
+
+    rerender(<TooltipComponent text="" />)
+    await expect.poll(() => tooltip.matches(':popover-open')).toBe(false)
+    expect(getTooltipElement(container)).toBe(tooltip)
+  })
+
+  it('opens from a pending hover when the text returns before the delay elapses', () => {
+    vi.useFakeTimers()
+    try {
+      const {container, rerender, getByRole} = HTMLRender(<TooltipComponent />)
+      const tooltip = getTooltipElement(container)
+
+      fireEvent.mouseOver(getByRole('button'))
+      rerender(<TooltipComponent text="" />)
+      rerender(<TooltipComponent text="Back again" />)
+      act(() => {
+        vi.advanceTimersByTime(100)
+      })
+
+      expect(tooltip.matches(':popover-open')).toBe(true)
+      expect(tooltip).toHaveTextContent('Back again')
+    } finally {
+      vi.useRealTimers()
+    }
+  })
+
+  it('does not open from a pending hover when the text is still empty after the delay elapses', () => {
+    vi.useFakeTimers()
+    try {
+      const {container, rerender, getByRole} = HTMLRender(<TooltipComponent />)
+      const tooltip = getTooltipElement(container)
+
+      fireEvent.mouseOver(getByRole('button'))
+      rerender(<TooltipComponent text="" />)
+      act(() => {
+        vi.advanceTimersByTime(100)
+      })
+
+      expect(tooltip.matches(':popover-open')).toBe(false)
+    } finally {
+      vi.useRealTimers()
+    }
+  })
+
+  it('keeps the trigger mounted and focused when the text is toggled', async () => {
+    function ConditionalTooltip() {
+      const [allowed, setAllowed] = useState(false)
+      return (
+        <Tooltip text={allowed ? '' : 'Not allowed'}>
+          <Button onClick={() => setAllowed(value => !value)}>Toggle</Button>
+        </Tooltip>
+      )
+    }
+    const {container} = HTMLRender(<ConditionalTooltip />)
+    const button = page.getByRole('button', {name: 'Toggle', exact: true})
+    const trigger = button.element()
+    const tooltip = getTooltipElement(container)
+
+    await userEvent.tab()
+    expect(trigger).toHaveFocus()
+    expect(tooltip.matches(':popover-open')).toBe(true)
+    expect(trigger).toHaveAccessibleDescription('Not allowed')
+
+    await userEvent.keyboard('{Enter}')
+    await expect.element(button).not.toHaveAttribute('aria-describedby')
+    await expect.poll(() => tooltip.matches(':popover-open')).toBe(false)
+    expect(button.element()).toBe(trigger)
+    expect(trigger).toHaveFocus()
+
+    await userEvent.keyboard('{Enter}')
+    await expect.element(button).toHaveAccessibleDescription('Not allowed')
+    expect(button.element()).toBe(trigger)
+    expect(trigger).toHaveFocus()
+  })
+})
+
 describe('Tooltip data-component attributes', () => {
   it('renders Tooltip with data-component attribute', () => {
     const {getByText} = HTMLRender(<TooltipComponent />)