From 6f247d20c12cc72835eb5e5020585046b4715f44 Mon Sep 17 00:00:00 2001 From: Adam Dierkens Date: Fri, 13 Nov 2026 16:23:19 -0500 Subject: [PATCH 1/2] Add public tooltip-only suppression Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .changeset/tooltip-suppression.md | 5 + .../react/src/TooltipV2/Tooltip.docs.json | 6 + .../TooltipV2/Tooltip.examples.stories.tsx | 65 +++++++- packages/react/src/TooltipV2/Tooltip.tsx | 36 +++-- .../src/TooltipV2/__tests__/Tooltip.test.tsx | 151 +++++++++++++++++- 5 files changed, 250 insertions(+), 13 deletions(-) create mode 100644 .changeset/tooltip-suppression.md diff --git a/.changeset/tooltip-suppression.md b/.changeset/tooltip-suppression.md new file mode 100644 index 00000000000..b8a3173e9fd --- /dev/null +++ b/.changeset/tooltip-suppression.md @@ -0,0 +1,5 @@ +--- +'@primer/react': minor +--- + +Tooltip: Add `disabled` to suppress the tooltip without disabling its trigger or removing its accessible label or description. diff --git a/packages/react/src/TooltipV2/Tooltip.docs.json b/packages/react/src/TooltipV2/Tooltip.docs.json index 019128ada7d..66523e84033 100644 --- a/packages/react/src/TooltipV2/Tooltip.docs.json +++ b/packages/react/src/TooltipV2/Tooltip.docs.json @@ -63,6 +63,12 @@ "type": "'short' | 'medium' | 'long'", "defaultValue": "short", "description": "Sets the delay before showing the tooltip. `short` is 50ms (default), `medium` is 400ms, and `long` is 1200ms." + }, + { + "name": "disabled", + "type": "boolean", + "defaultValue": "false", + "description": "Closes the visual tooltip, cancels pending opens, and prevents reopening without disabling its trigger or removing its accessible label or description. Re-enabling requires a fresh hover or focus. Either disabled or _privateDisableTooltip being true suppresses the tooltip." } ], "subcomponents": [] diff --git a/packages/react/src/TooltipV2/Tooltip.examples.stories.tsx b/packages/react/src/TooltipV2/Tooltip.examples.stories.tsx index d86ab511334..a95d0fdeb26 100644 --- a/packages/react/src/TooltipV2/Tooltip.examples.stories.tsx +++ b/packages/react/src/TooltipV2/Tooltip.examples.stories.tsx @@ -1,4 +1,5 @@ -import {useState, useCallback, useRef} from 'react' +import {useId, useState, useCallback, useRef} from 'react' +import {action} from 'storybook/actions' import {Button, IconButton} from '../Button' import Breadcrumbs from '../Breadcrumbs' import {ActionMenu} from '../ActionMenu' @@ -8,6 +9,7 @@ import {Tooltip} from './Tooltip' import {Dialog} from '../experimental' import {GitBranchIcon, KebabHorizontalIcon, TriangleDownIcon, CheckIcon, XIcon} from '@primer/octicons-react' import {default as VisuallyHidden} from '../_VisuallyHidden' +import Checkbox from '../Checkbox' export default { title: 'Components/TooltipV2/Examples', @@ -20,6 +22,67 @@ export const CustomId = () => ( ) +export function SuppressWhileMenuOpen() { + const [isMenuOpen, setIsMenuOpen] = useState(false) + return ( +
+

Hover or focus to show the hint. Opening the menu suppresses it without disabling the button.

+ + + + + + + + + View details + + + +
+ ) +} + +SuppressWhileMenuOpen.parameters = { + docs: { + description: { + story: + 'Use Tooltip.disabled to suppress the hint while its menu is open. The trigger remains enabled and its accessible description is retained. This does not control tooltip visibility directly.', + }, + }, +} + +export function TooltipOnlyDisabled() { + const checkboxId = useId() + const [disabled, setDisabled] = useState(false) + return ( +
+

+ Suppressing the tooltip does not disable the action or remove its accessible description. Trigger clicks still + appear in Actions. +

+ +
+ + + +
+
+ ) +} + +TooltipOnlyDisabled.parameters = { + docs: { + description: { + story: + 'disabled closes visible bubbles and cancels pending opens. Re-enable, then hover or focus again to reopen. The existing _privateDisableTooltip remains compatible: either prop being true wins. Disable the Button separately when the action itself is unavailable.', + }, + }, +} + export const FilesPage = () => ( diff --git a/packages/react/src/TooltipV2/Tooltip.tsx b/packages/react/src/TooltipV2/Tooltip.tsx index 82cd3c36ed0..e695232f191 100644 --- a/packages/react/src/TooltipV2/Tooltip.tsx +++ b/packages/react/src/TooltipV2/Tooltip.tsx @@ -15,6 +15,7 @@ import useSafeTimeout from '../hooks/useSafeTimeout' import type {SlotMarker} from '../utils/types' import {TooltipContext} from './TooltipContext' import {reactMajorVersion} from '../utils/environment' +import {useEffectCallback} from '../internal/hooks/useEffectCallback' export type TooltipDirection = 'nw' | 'n' | 'ne' | 'e' | 'se' | 's' | 'sw' | 'w' export type TooltipProps = React.PropsWithChildren<{ @@ -30,7 +31,13 @@ export type TooltipProps = React.PropsWithChildren<{ */ delay?: 'short' | 'medium' | 'long' /** - * Private API for use internally only. Prevents the tooltip from opening if `true`. + * Closes and suppresses the tooltip without disabling its trigger or removing + * its accessible label or description. Either suppression prop being true wins. + * @default false + */ + disabled?: boolean + /** + * Private API for use internally only. Closes and prevents the tooltip from opening if `true`. * * Accessibility note: This prop should be used with caution. Only use when needing to * programmatically close the tooltip in response to a specific user action, such as @@ -121,11 +128,13 @@ export const Tooltip: ForwardRefExoticComponent< className, keybindingHint = emptyKeybindingHints, delay = 'short', + disabled = false, _privateDisableTooltip = false, ...rest }: TooltipProps, forwardedRef, ) => { + const isTooltipDisabled = disabled || _privateDisableTooltip const tooltipId = useId(id) const child = Children.only(children) const isFragmentTrigger = React.isValidElement(child) && child.type === React.Fragment @@ -161,7 +170,7 @@ export const Tooltip: ForwardRefExoticComponent< readTriggerRef.current instanceof HTMLElement && tooltipElRef.current.hasAttribute('popover') && !tooltipElRef.current.matches(':popover-open') && - !_privateDisableTooltip + !isTooltipDisabled ) { const tooltip = tooltipElRef.current const trigger = readTriggerRef.current @@ -197,8 +206,8 @@ export const Tooltip: ForwardRefExoticComponent< } } } - const closeTooltip = () => { - if (openTimeoutRef.current) { + const closeTooltip = useEffectCallback(() => { + if (openTimeoutRef.current !== null) { safeClearTimeout(openTimeoutRef.current) openTimeoutRef.current = null } @@ -210,10 +219,8 @@ export const Tooltip: ForwardRefExoticComponent< tooltipElRef.current.matches(':popover-open') ) { tooltipElRef.current.hidePopover() - setIsPopoverOpen(false) - } else { - setIsPopoverOpen(false) } + setIsPopoverOpen(false) } catch (error) { // older browsers don't support the :popover-open selector and will throw, even though we use a polyfill // see https://github.com/github/issues/issues/12468 @@ -229,7 +236,13 @@ export const Tooltip: ForwardRefExoticComponent< throw error } } - } + }) + + useEffect(() => { + // Suppression must synchronize the external Popover API, not just future events. + // eslint-disable-next-line react-you-might-not-need-an-effect/no-event-handler + if (isTooltipDisabled) closeTooltip() + }, [isTooltipDisabled, closeTooltip]) // context value const value = useMemo(() => ({tooltipId}), [tooltipId]) @@ -289,14 +302,14 @@ export const Tooltip: ForwardRefExoticComponent< useOnEscapePress( (event: KeyboardEvent) => { if (isPopoverOpen) { - if (!_privateDisableTooltip) { + if (!isTooltipDisabled) { event.stopImmediatePropagation() event.preventDefault() } closeTooltip() } }, - [isPopoverOpen, _privateDisableTooltip], + [isPopoverOpen, isTooltipDisabled], ) const platform = usePlatform() @@ -356,6 +369,8 @@ export const Tooltip: ForwardRefExoticComponent< child.props.onFocus?.(event) }, onMouseOverCapture: (event: React.MouseEvent) => { + if (isTooltipDisabled) return + if (openTimeoutRef.current !== null) safeClearTimeout(openTimeoutRef.current) const delayTime = delayTimeMap[delay] || 50 // We use a `capture` event to ensure this is called first before // events that might cancel the opening timeout (like `onTouchEnd`) @@ -364,6 +379,7 @@ export const Tooltip: ForwardRefExoticComponent< openTimeoutRef.current = safeSetTimeout(() => { // if the mouse is already moved out, do not show the tooltip if (!openTimeoutRef.current) return + openTimeoutRef.current = null openTooltip() child.props.onMouseEnter?.(event) }, delayTime) diff --git a/packages/react/src/TooltipV2/__tests__/Tooltip.test.tsx b/packages/react/src/TooltipV2/__tests__/Tooltip.test.tsx index 46ca86f8d88..6642a041b7b 100644 --- a/packages/react/src/TooltipV2/__tests__/Tooltip.test.tsx +++ b/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 {afterEach, beforeEach, 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' @@ -330,3 +331,149 @@ describe('Tooltip forwarded ref (primer_react_merged_forwarded_refs)', () => { }) } }) + +describe('Tooltip suppression', () => { + it.each(['label', 'description'] as const)( + 'closes without removing the accessible %s or disabling its trigger', + async type => { + const onClick = vi.fn() + const example = (disabled: boolean) => ( + + + + ) + const {container, rerender} = HTMLRender(example(false)) + const button = page.getByRole('button', {name: type === 'label' ? 'Choose owner' : 'Owner', exact: true}) + const tooltip = container.querySelector('[data-component="Tooltip"]')! + const relationship = type === 'label' ? 'aria-labelledby' : 'aria-describedby' + const id = button.element().getAttribute(relationship) + expect(document.getElementById(id ?? '')).toHaveTextContent('Choose owner') + await userEvent.hover(button) + await expect.poll(() => tooltip.matches(':popover-open')).toBe(true) + + rerender(example(true)) + expect(tooltip.matches(':popover-open')).toBe(false) + expect(button.element()).toHaveAttribute(relationship, id) + expect(button.element()).toHaveAccessibleName(type === 'label' ? 'Choose owner' : 'Owner') + if (type === 'description') expect(button.element()).toHaveAccessibleDescription('Choose owner') + await expect.element(button).toBeEnabled() + expect(tooltip).not.toHaveAttribute('disabled') + await userEvent.tab() + await expect.element(button).toHaveFocus() + expect(tooltip.matches(':popover-open')).toBe(false) + await userEvent.click(button) + expect(onClick).toHaveBeenCalledOnce() + expect(tooltip.matches(':popover-open')).toBe(false) + }, + ) + + it('preserves configured menu anchor behavior while suppressing the tooltip', async () => { + const onClick = vi.fn() + HTMLRender( + + + + + + + + + Assign me + + + , + ) + const button = page.getByRole('button', {name: 'Owner', exact: true}) + expect(button.element()).toHaveAccessibleDescription('Choose owner') + await expect.element(button).toBeEnabled() + await userEvent.click(button) + expect(onClick).toHaveBeenCalledOnce() + await expect.element(button).toHaveAttribute('aria-expanded', 'true') + await expect.element(page.getByRole('menuitem', {name: 'Assign me', exact: true})).toHaveFocus() + }) + + it('does not re-enable a separately disabled trigger', () => { + const example = (disabled: boolean) => ( + + + + ) + const {rerender} = HTMLRender(example(true)) + expect(page.getByRole('button', {name: 'Owner', exact: true}).element()).toBeDisabled() + rerender(example(false)) + expect(page.getByRole('button', {name: 'Owner', exact: true}).element()).toBeDisabled() + }) + + describe('pending opens', () => { + beforeEach(() => vi.useFakeTimers()) + afterEach(() => vi.useRealTimers()) + + it('preserves private suppression and closes an already visible tooltip', () => { + const example = (disabled: boolean) => ( + + + + ) + const {container, rerender} = HTMLRender(example(false)) + fireEvent.mouseOver(page.getByRole('button', {name: 'Owner', exact: true}).element()) + act(() => vi.advanceTimersByTime(50)) + const tooltip = container.querySelector('[data-component="Tooltip"]')! + expect(tooltip.matches(':popover-open')).toBe(true) + rerender(example(true)) + expect(tooltip.matches(':popover-open')).toBe(false) + }) + + it('cancels pending work and requires fresh interaction after re-enabling', () => { + const example = (disabled: boolean) => ( + + + + ) + const {container, rerender} = HTMLRender(example(false)) + const button = page.getByRole('button', {name: 'Owner', exact: true}).element() + const tooltip = container.querySelector('[data-component="Tooltip"]')! + fireEvent.mouseOver(button) + fireEvent.mouseOver(button) + act(() => vi.advanceTimersByTime(100)) + expect(tooltip.matches(':popover-open')).toBe(false) + rerender(example(true)) + rerender(example(false)) + expect(tooltip.matches(':popover-open')).toBe(false) + fireEvent.mouseOver(button) + act(() => vi.advanceTimersByTime(399)) + expect(tooltip.matches(':popover-open')).toBe(false) + act(() => vi.advanceTimersByTime(1)) + expect(tooltip.matches(':popover-open')).toBe(true) + + rerender(example(true)) + expect(tooltip.matches(':popover-open')).toBe(false) + fireEvent.mouseOver(button) + rerender(example(false)) + act(() => vi.advanceTimersByTime(400)) + expect(tooltip.matches(':popover-open')).toBe(false) + fireEvent.mouseOver(button) + act(() => vi.advanceTimersByTime(400)) + expect(tooltip.matches(':popover-open')).toBe(true) + }) + + it.each([ + {disabled: true, _privateDisableTooltip: false}, + {disabled: false, _privateDisableTooltip: true}, + {disabled: true, _privateDisableTooltip: true}, + {disabled: false, _privateDisableTooltip: false}, + ])('suppresses when either public or private prop is true (%j)', props => { + const {container} = HTMLRender( + + + , + ) + fireEvent.mouseOver(page.getByRole('button', {name: 'Owner', exact: true}).element()) + act(() => vi.advanceTimersByTime(400)) + expect(container.querySelector('[data-component="Tooltip"]')?.matches(':popover-open')).toBe( + !props.disabled && !props._privateDisableTooltip, + ) + }) + }) +}) From c23e8e55b649efdc9e842b2fe7ba758d74b0f95b Mon Sep 17 00:00:00 2001 From: Adam Dierkens Date: Fri, 13 Nov 2026 16:31:40 -0500 Subject: [PATCH 2/2] Use public tooltip suppression in IconButton Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .changeset/tooltip-suppression.md | 2 +- packages/react/src/Button/IconButton.tsx | 2 +- .../src/Button/__tests__/Button.test.tsx | 22 +++++++++++++++++++ .../react/src/TooltipV2/Tooltip.docs.json | 2 +- .../TooltipV2/Tooltip.examples.stories.tsx | 2 +- packages/react/src/TooltipV2/Tooltip.tsx | 2 ++ 6 files changed, 28 insertions(+), 4 deletions(-) diff --git a/.changeset/tooltip-suppression.md b/.changeset/tooltip-suppression.md index b8a3173e9fd..6035c308293 100644 --- a/.changeset/tooltip-suppression.md +++ b/.changeset/tooltip-suppression.md @@ -2,4 +2,4 @@ '@primer/react': minor --- -Tooltip: Add `disabled` to suppress the tooltip without disabling its trigger or removing its accessible label or description. +Tooltip: Add `disabled` to suppress the tooltip without disabling its trigger or removing its accessible label or description. Deprecate `_privateDisableTooltip` in favor of `disabled`. diff --git a/packages/react/src/Button/IconButton.tsx b/packages/react/src/Button/IconButton.tsx index b48d54475a5..966bf5b9686 100644 --- a/packages/react/src/Button/IconButton.tsx +++ b/packages/react/src/Button/IconButton.tsx @@ -63,7 +63,7 @@ const IconButton = forwardRef( type={description ? undefined : 'label'} direction={tooltipDirection} keybindingHint={keybindingHint ?? keyshortcuts} - _privateDisableTooltip={hasActivePopup} + disabled={hasActivePopup} > { await expect.element(wrapper).toHaveClass(classes.ConditionalWrapperLink) }) + it('keeps an active popup trigger enabled while suppressing its tooltip', async () => { + const onClick = vi.fn() + const example = (expanded: boolean) => ( + + ) + const {container, rerender} = render(example(false)) + const button = page.getByRole('button', {name: 'Heart', exact: true}) + const tooltip = container.querySelector('[data-component="Tooltip"]')! + const labelId = button.element().getAttribute('aria-labelledby') + await userEvent.hover(button) + await expect.poll(() => tooltip.matches(':popover-open')).toBe(true) + + rerender(example(true)) + expect(tooltip.matches(':popover-open')).toBe(false) + expect(button.element()).toHaveAttribute('aria-labelledby', labelId) + expect(button.element()).toHaveAccessibleName('Heart') + await expect.element(button).toBeEnabled() + await userEvent.click(button) + expect(onClick).toHaveBeenCalledOnce() + expect(tooltip.matches(':popover-open')).toBe(false) + }) + it('should render tooltip on an icon button when unsafeDisableTooltip prop is passed as false', async () => { render() const triggerEL = page.getByRole('button') diff --git a/packages/react/src/TooltipV2/Tooltip.docs.json b/packages/react/src/TooltipV2/Tooltip.docs.json index 66523e84033..6e58c146f11 100644 --- a/packages/react/src/TooltipV2/Tooltip.docs.json +++ b/packages/react/src/TooltipV2/Tooltip.docs.json @@ -68,7 +68,7 @@ "name": "disabled", "type": "boolean", "defaultValue": "false", - "description": "Closes the visual tooltip, cancels pending opens, and prevents reopening without disabling its trigger or removing its accessible label or description. Re-enabling requires a fresh hover or focus. Either disabled or _privateDisableTooltip being true suppresses the tooltip." + "description": "Closes the visual tooltip, cancels pending opens, and prevents reopening without disabling its trigger or removing its accessible label or description. Re-enabling requires a fresh hover or focus. The deprecated _privateDisableTooltip alias remains compatible; either prop being true suppresses the tooltip." } ], "subcomponents": [] diff --git a/packages/react/src/TooltipV2/Tooltip.examples.stories.tsx b/packages/react/src/TooltipV2/Tooltip.examples.stories.tsx index a95d0fdeb26..d7feefc4396 100644 --- a/packages/react/src/TooltipV2/Tooltip.examples.stories.tsx +++ b/packages/react/src/TooltipV2/Tooltip.examples.stories.tsx @@ -78,7 +78,7 @@ TooltipOnlyDisabled.parameters = { docs: { description: { story: - 'disabled closes visible bubbles and cancels pending opens. Re-enable, then hover or focus again to reopen. The existing _privateDisableTooltip remains compatible: either prop being true wins. Disable the Button separately when the action itself is unavailable.', + 'disabled closes visible bubbles and cancels pending opens. Re-enable, then hover or focus again to reopen. The deprecated _privateDisableTooltip alias remains compatible: either prop being true wins. Disable the Button separately when the action itself is unavailable.', }, }, } diff --git a/packages/react/src/TooltipV2/Tooltip.tsx b/packages/react/src/TooltipV2/Tooltip.tsx index e695232f191..80f538fdbf5 100644 --- a/packages/react/src/TooltipV2/Tooltip.tsx +++ b/packages/react/src/TooltipV2/Tooltip.tsx @@ -37,6 +37,8 @@ export type TooltipProps = React.PropsWithChildren<{ */ disabled?: boolean /** + * @deprecated Use `disabled` instead. + * * Private API for use internally only. Closes and prevents the tooltip from opening if `true`. * * Accessibility note: This prop should be used with caution. Only use when needing to