From ba03cb6524b86b59b997896b31ca7bcd7b8465fb Mon Sep 17 00:00:00 2001 From: Adam Dierkens Date: Fri, 13 Nov 2026 11:21:45 -0500 Subject: [PATCH 1/9] Add composable trigger rendering and anchor ref context Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .changeset/composable-trigger-rendering.md | 5 + contributor-docs/extending-components.md | 114 +++++++++++++ .../react/src/ActionMenu/ActionMenu.docs.json | 5 + .../react/src/ActionMenu/ActionMenu.test.tsx | 119 ++++++++++++- packages/react/src/ActionMenu/ActionMenu.tsx | 23 ++- .../src/ActionMenu/ActionMenu.types.test.tsx | 22 +++ .../AnchoredOverlay/AnchoredOverlay.docs.json | 4 +- .../src/AnchoredOverlay/AnchoredOverlay.tsx | 32 ++-- .../AnchoredOverlay.types.test.tsx | 22 +++ packages/react/src/AnchoredOverlay/index.ts | 2 +- .../src/SelectPanel/SelectPanel.test.tsx | 34 +++- .../react/src/SelectPanel/SelectPanel.tsx | 25 +-- .../react/src/TooltipV2/Tooltip.docs.json | 5 + packages/react/src/TooltipV2/Tooltip.tsx | 159 +++++++++--------- .../src/TooltipV2/__tests__/Tooltip.test.tsx | 78 ++++++++- packages/react/src/TooltipV2/index.ts | 2 +- .../__snapshots__/exports.test.ts.snap | 3 + packages/react/src/deprecated/ActionMenu.tsx | 16 +- packages/react/src/index.ts | 11 +- 19 files changed, 562 insertions(+), 119 deletions(-) create mode 100644 .changeset/composable-trigger-rendering.md create mode 100644 contributor-docs/extending-components.md diff --git a/.changeset/composable-trigger-rendering.md b/.changeset/composable-trigger-rendering.md new file mode 100644 index 00000000000..396463dcb05 --- /dev/null +++ b/.changeset/composable-trigger-rendering.md @@ -0,0 +1,5 @@ +--- +'@primer/react': minor +--- + +Tooltip, ActionMenu: Add custom trigger rendering to compose multiple behaviors on a single interactive element. AnchoredOverlay and SelectPanel expose the anchor ref in the render callback's second argument. diff --git a/contributor-docs/extending-components.md b/contributor-docs/extending-components.md new file mode 100644 index 00000000000..e5fc68fe815 --- /dev/null +++ b/contributor-docs/extending-components.md @@ -0,0 +1,114 @@ +# Extending components + +## Composing trigger behaviors + +Prefer the configured `Tooltip` children and `ActionMenu.Button` APIs for ordinary +usage. Use `renderAnchor` when a custom trigger needs to route props or refs +explicitly. `Tooltip`, `ActionMenu`, and `AnchoredOverlay` support this convention; +the existing children-based APIs remain supported. + +For example, compose a menu and tooltip on one icon button without teaching the +menu to inspect the tooltip's children: + +```tsx +import {ActionList, ActionMenu, IconButton, Tooltip} from '@primer/react' +import {KebabHorizontalIcon} from '@primer/octicons-react' + +function Actions() { + return ( + ( + ( + + )} + /> + )} + > + + + Copy link + + + + ) +} +``` + +The menu's click/keyboard handlers and tooltip's focus/pointer handlers are +disjoint in this example. The tooltip combines the menu ref with its own ref and +any forwarded ref. The final callback ref reaches the same button used for +positioning, outside-click handling, and focus return. Callback and object refs +passed to `Tooltip` are supported in this path without a feature flag. + +`renderAnchor` is a rendering callback, not a component: do not call hooks inside +it. Render a separate component if you need hooks, such as `useMergedRefs` from +`@primer/react`, to attach additional refs or route the supplied ref through a +custom prop. All behavior refs must resolve to the actual interactive HTML +element, not a wrapper or imperative handle. + +### Prop ownership + +- Spread every supplied prop onto the single interactive element, including the + ref. Do not nest buttons. Use either `Tooltip` children or `renderAnchor`, and + either configured menu anchors or `ActionMenu.renderAnchor`, not both. +- Keep the generated `id`, `aria-haspopup`, `aria-expanded`, and tooltip + `aria-labelledby`/`aria-describedby` relationships on that element. If adding + another description, append its ID to `aria-describedby` rather than replacing + the tooltip ID. Use `type="description"` for an already named button, or + `type="label"` to name an otherwise unnamed trigger. + When spreading onto `IconButton` with an explicit `aria-label`, set + `aria-labelledby={undefined}` to satisfy its mutually exclusive naming props. +- Call supplied event handlers before additional handlers, and skip additional + handlers when `event.defaultPrevented` is true, following Primer's prop-merging + convention. If deliberately cancelling menu activation, call `preventDefault` + before forwarding the event; menu click and keyboard activation respect it. +- If props overlap, merge them explicitly rather than relying on spread order: + combine class names, apply custom styles after supplied styles, and compose + handlers as above. Render callbacks do not automatically merge props you add. +- Native `disabled` buttons remain non-interactive. As with the existing + `Tooltip` children API, do not use a disabled native button as a tooltip + trigger. Use the existing `inactive` button pattern if an unavailable action + needs an explanation. + +`Popover` is presentational and does not provide trigger behavior. For a custom +popover or picker interaction, `AnchoredOverlay` and `SelectPanel` provide the +object ref in a second callback argument: + +```tsx + setOpen(true)} + onClose={() => setOpen(false)} + renderAnchor={(anchorProps, {ref}) => ( + ( + + )} + /> + )} +> + {children} + +``` + +The context ref is the same object already supplied at runtime in the anchor +props. It is also the supplied `anchorRef`, if present. Existing one-argument +callbacks and object-ref behavior are unchanged. Keeping the additional typed ref +in a separate context avoids making existing prop spreads incompatible with +more narrowly typed button and link refs. + +This API does not introduce an independent focus-return target or replace either +component's overlay behavior. diff --git a/packages/react/src/ActionMenu/ActionMenu.docs.json b/packages/react/src/ActionMenu/ActionMenu.docs.json index 3df8c5d079b..86604378ca6 100644 --- a/packages/react/src/ActionMenu/ActionMenu.docs.json +++ b/packages/react/src/ActionMenu/ActionMenu.docs.json @@ -28,6 +28,11 @@ ], "importPath": "@primer/react", "props": [ + { + "name": "renderAnchor", + "type": "(props: ActionMenuAnchorRenderProps) => React.ReactElement", + "description": "Render a custom trigger instead of ActionMenu.Button or ActionMenu.Anchor. Forward every supplied prop, including ref, to a single interactive HTML element." + }, { "name": "children", "type": "React.ReactElement[]", diff --git a/packages/react/src/ActionMenu/ActionMenu.test.tsx b/packages/react/src/ActionMenu/ActionMenu.test.tsx index e0f641963bc..bef3954cdb7 100644 --- a/packages/react/src/ActionMenu/ActionMenu.test.tsx +++ b/packages/react/src/ActionMenu/ActionMenu.test.tsx @@ -1,9 +1,9 @@ import {page, userEvent} from 'vitest/browser' import {describe, expect, it, vi, beforeEach} from 'vitest' -import {render as HTMLRender, waitFor, act, within} from '@testing-library/react' +import {render as HTMLRender, waitFor, act, within, fireEvent} from '@testing-library/react' import rtlUserEvent from '@testing-library/user-event' import type React from 'react' -import {useRef, useState} from 'react' +import {createRef, useRef, useState} from 'react' import BaseStyles from '../BaseStyles' import {ActionMenu} from '.' import {ActionList} from '../ActionList' @@ -22,6 +22,121 @@ import {implementsClassName} from '../utils/testing' import {createRenderCounter} from '../utils/testing/profiler' import {FeatureFlags} from '../FeatureFlags' +describe('ActionMenu renderAnchor', () => { + it.each([false, true])('composes a tooltip and menu on one button with merged refs (flag: %s)', async mergedRefs => { + const anchorRef = createRef() + const tooltipRef = vi.fn() + const {unmount} = HTMLRender( + + ( + { + ref(node) + tooltipRef(node) + }} + text="Choose an action" + renderAnchor={tooltipProps => ( + + )} + /> + )} + > + + + New file + + + + , + ) + const button = page.getByRole('button', {name: 'Actions', exact: true}) + expect(document.querySelectorAll('button')).toHaveLength(1) + expect(anchorRef.current).toBe(button.element()) + expect(tooltipRef).toHaveBeenLastCalledWith(button.element()) + const description = button.element().getAttribute('aria-describedby') + expect(document.getElementById(description ?? '')).toHaveTextContent('Choose an action') + await expect.element(button).toHaveAttribute('aria-haspopup', 'true') + + await userEvent.click(button) + await expect.element(button).toHaveAttribute('aria-expanded', 'true') + await expect.element(page.getByRole('menuitem', {name: 'New file'})).toHaveFocus() + await userEvent.keyboard('{Escape}') + await expect.element(button).toHaveAttribute('aria-expanded', 'false') + await expect.element(button).toHaveFocus() + + await userEvent.keyboard('{ArrowDown}') + await expect.element(page.getByRole('menuitem', {name: 'New file'})).toHaveFocus() + unmount() + expect(anchorRef.current).toBeNull() + expect(tooltipRef).toHaveBeenLastCalledWith(null) + }) + + it('lets a custom trigger cancel opening before forwarding the event', async () => { + const onOpenChange = vi.fn() + const calls: string[] = [] + HTMLRender( + ( + + )} + > + Menu contents + , + ) + const button = page.getByRole('button', {name: 'Actions', exact: true}) + await userEvent.click(button) + expect(calls).toEqual(['consumer', 'forwarded']) + expect(onOpenChange).not.toHaveBeenCalled() + await userEvent.keyboard('{ArrowDown}') + expect(onOpenChange).not.toHaveBeenCalled() + await expect.element(button).toHaveAttribute('aria-expanded', 'false') + }) + + it('preserves native disabled behavior for a custom trigger', async () => { + const onOpenChange = vi.fn() + HTMLRender( + ( + + )} + > + Menu contents + , + ) + const button = page.getByRole('button', {name: 'Actions', exact: true}) + await expect.element(button).toBeDisabled() + fireEvent.click(button.element()) + expect(onOpenChange).not.toHaveBeenCalled() + await expect.element(button).toHaveAttribute('aria-expanded', 'false') + }) +}) + // Mock getAnchoredPosition for feature flag tests vi.mock('@primer/behaviors', async () => { const actual = await vi.importActual('@primer/behaviors') diff --git a/packages/react/src/ActionMenu/ActionMenu.tsx b/packages/react/src/ActionMenu/ActionMenu.tsx index c33997690f5..07642d5daf0 100644 --- a/packages/react/src/ActionMenu/ActionMenu.tsx +++ b/packages/react/src/ActionMenu/ActionMenu.tsx @@ -4,7 +4,7 @@ import {TriangleDownIcon, ChevronRightIcon} from '@primer/octicons-react' import type {AnchoredOverlayProps} from '../AnchoredOverlay' import {AnchoredOverlay} from '../AnchoredOverlay' import type {OverlayProps} from '../Overlay' -import {useProvidedRefOrCreate, useProvidedStateOrCreate, useMenuKeyboardNavigation} from '../hooks' +import {useMergedRefs, useProvidedRefOrCreate, useProvidedStateOrCreate, useMenuKeyboardNavigation} from '../hooks' import {Divider} from '../ActionList/Divider' import {ActionListContainerContext} from '../ActionList/ActionListContainerContext' import type {ButtonProps} from '../Button' @@ -35,6 +35,11 @@ export type MenuContextProps = Pick< const MenuContext = React.createContext({renderAnchor: null, open: false}) export type ActionMenuProps = { + /** + * Render a custom trigger instead of ActionMenu.Button or ActionMenu.Anchor. + * Forward all supplied props, including ref, to a single interactive element. + */ + renderAnchor?: (props: ActionMenuAnchorRenderProps) => React.ReactElement /** * Recommended: `ActionMenu.Button` or `ActionMenu.Anchor` with `ActionMenu.Overlay` */ @@ -52,6 +57,10 @@ export type ActionMenuProps = { onOpenChange?: (s: boolean) => void } & Pick +export type ActionMenuAnchorRenderProps = Omit, 'aria-label' | 'aria-labelledby'> & { + ref: React.RefCallback +} + // anchorProps adds onClick and onKeyDown, so we need to merge them with buttonProps const mergeAnchorHandlers = (anchorProps: React.HTMLAttributes, buttonProps: ButtonProps) => { const mergedAnchorProps = {...anchorProps} @@ -86,6 +95,7 @@ const Menu: FCWithSlotMarker> = ({ open, onOpenChange, children, + renderAnchor: customRenderAnchor, }: ActionMenuProps) => { const parentMenuContext = useContext(MenuContext) @@ -116,13 +126,22 @@ const Menu: FCWithSlotMarker> = ({ const menuButtonChildId = React.isValidElement(menuButtonChild) ? menuButtonChild.props.id : undefined const anchorRef = useProvidedRefOrCreate(externalAnchorRef) + const callbackAnchorRef = useMergedRefs(anchorRef, undefined) const anchorId = useId(menuButtonChildId) - let renderAnchor: AnchoredOverlayProps['renderAnchor'] = null + const renderCustomAnchor = useMemo( + () => + customRenderAnchor + ? (props: React.HTMLAttributes) => customRenderAnchor({...props, ref: callbackAnchorRef}) + : null, + [customRenderAnchor, callbackAnchorRef], + ) + let renderAnchor: AnchoredOverlayProps['renderAnchor'] = renderCustomAnchor // 🚨 Hack for good API! // we strip out Anchor from children and pass it to AnchoredOverlay to render // with additional props for accessibility // 🚨 Accounting for Tooltip wrapping ActionMenu.Button or being a direct child of ActionMenu.Anchor. const contents = React.Children.map(children, child => { + if (customRenderAnchor) return child // Is ActionMenu.Button wrapped with Tooltip? If this is the case, our anchor is the tooltip's trigger (ActionMenu.Button's grandchild) if (child.type === Tooltip || isSlot(child, Tooltip)) { // tooltip trigger diff --git a/packages/react/src/ActionMenu/ActionMenu.types.test.tsx b/packages/react/src/ActionMenu/ActionMenu.types.test.tsx index 2ac83271c66..ef029b079b4 100644 --- a/packages/react/src/ActionMenu/ActionMenu.types.test.tsx +++ b/packages/react/src/ActionMenu/ActionMenu.types.test.tsx @@ -1,4 +1,6 @@ import {ActionMenu} from '.' +import {Tooltip} from '../TooltipV2' +import {Button} from '../Button' export function actionButtonWithoutProps() { return @@ -25,3 +27,23 @@ export function actionOverlayWithInvalidPreventFocusOnClose() { // @ts-expect-error preventFocusOnClose must be a boolean return Menu contents } + +export function actionMenuWithComposedTrigger() { + return ( + ( + ( + + )} + /> + )} + > + Menu contents + + ) +} diff --git a/packages/react/src/AnchoredOverlay/AnchoredOverlay.docs.json b/packages/react/src/AnchoredOverlay/AnchoredOverlay.docs.json index 75fe3cf0487..424c63ca5be 100644 --- a/packages/react/src/AnchoredOverlay/AnchoredOverlay.docs.json +++ b/packages/react/src/AnchoredOverlay/AnchoredOverlay.docs.json @@ -64,9 +64,9 @@ }, { "name": "renderAnchor", - "type": ">(props: T) => JSX.Element", + "type": ">(props: T, context: AnchoredOverlayAnchorContext) => JSX.Element", "defaultValue": "", - "description": "A custom function component used to render the anchor element. When renderAnchor is null, an anchorRef is required." + "description": "Render the anchor element. The second argument exposes the same object ref already supplied in the props, for composing trigger behaviors. Existing one-argument callbacks remain supported. When renderAnchor is null, an anchorRef is required." }, { "name": "anchorRef", diff --git a/packages/react/src/AnchoredOverlay/AnchoredOverlay.tsx b/packages/react/src/AnchoredOverlay/AnchoredOverlay.tsx index bc373025e56..f02371ee3b9 100644 --- a/packages/react/src/AnchoredOverlay/AnchoredOverlay.tsx +++ b/packages/react/src/AnchoredOverlay/AnchoredOverlay.tsx @@ -18,13 +18,19 @@ import {useFeatureFlag} from '../FeatureFlags' import {widthMap} from '../Overlay/constants' import {reactMajorVersion} from '../utils/environment' +export type AnchoredOverlayAnchorContext = { + ref: {readonly current: HTMLElement | null} +} + interface AnchoredOverlayPropsWithAnchor { /** * A custom function component used to render the anchor element. * Will receive the selected text as `children` prop when an item is activated. + * The second argument exposes the anchor's object ref for composing behaviors. */ renderAnchor: , 'aria-label' | 'aria-labelledby'>>( props: T, + context: AnchoredOverlayAnchorContext, ) => JSX.Element /** @@ -406,17 +412,21 @@ export const AnchoredOverlay: React.FC {renderAnchor && - // eslint-disable-next-line react-hooks/refs - renderAnchor({ - ref: anchorRef, - id: anchorId, - 'aria-haspopup': 'true', - 'aria-expanded': open, - tabIndex: 0, - onClick: onAnchorClick, - onKeyDown: onAnchorKeyDown, - ...popoverTargetProps, - })} + renderAnchor( + // eslint-disable-next-line react-hooks/refs + { + ref: anchorRef, + id: anchorId, + 'aria-haspopup': 'true', + 'aria-expanded': open, + tabIndex: 0, + onClick: onAnchorClick, + onKeyDown: onAnchorKeyDown, + ...popoverTargetProps, + }, + // eslint-disable-next-line react-hooks/refs + {ref: anchorRef}, + )} {open ? ( ) } + +export function itemWithTooltip() { + return ( + ( + ( + + )} + /> + )} + > +

GitHub

+ + ) +} diff --git a/packages/react/src/AnchoredOverlay/index.ts b/packages/react/src/AnchoredOverlay/index.ts index 0a89ba09da0..2db56bee504 100644 --- a/packages/react/src/AnchoredOverlay/index.ts +++ b/packages/react/src/AnchoredOverlay/index.ts @@ -1,2 +1,2 @@ export {AnchoredOverlay} from './AnchoredOverlay' -export type {AnchoredOverlayProps} from './AnchoredOverlay' +export type {AnchoredOverlayProps, AnchoredOverlayAnchorContext} from './AnchoredOverlay' diff --git a/packages/react/src/SelectPanel/SelectPanel.test.tsx b/packages/react/src/SelectPanel/SelectPanel.test.tsx index 6e4054a89e2..daa08382561 100644 --- a/packages/react/src/SelectPanel/SelectPanel.test.tsx +++ b/packages/react/src/SelectPanel/SelectPanel.test.tsx @@ -6,7 +6,8 @@ import {SelectPanel, type SelectPanelProps, type ItemInput, type GroupedListProp import {FeatureFlags} from '../FeatureFlags' import type {InitialLoadingType} from './SelectPanel' import type {LiveRegionElement} from '@primer/live-region-element' -import {IconButton} from '../Button' +import {Button, IconButton} from '../Button' +import {Tooltip} from '../TooltipV2' import {ArrowLeftIcon} from '@primer/octicons-react' import classes from './SelectPanel.test.module.css' import {implementsClassName} from '../utils/testing' @@ -99,6 +100,37 @@ function BasicSelectPanel(passthroughProps: Record) { globalThis.Element.prototype.scrollTo = vi.fn() +describe('SelectPanel renderAnchor context', () => { + it('forwards the unchanged object ref so a tooltip can compose with its trigger', async () => { + const anchorRef = React.createRef() + const renderAnchor = vi.fn>((props, {ref}) => ( + ( + + )} + /> + )) + const {unmount} = render() + const button = page.getByRole('button', {name: 'Select items', exact: true}) + expect(renderAnchor.mock.calls[0][1].ref).toBe(anchorRef) + expect(anchorRef.current).toBe(button.element()) + const description = button.element().getAttribute('aria-describedby') + expect(document.getElementById(description ?? '')).toHaveTextContent('Choose items') + expect(document.querySelectorAll('button')).toHaveLength(1) + await userEvent.click(button) + await expect.element(button).toHaveAttribute('aria-expanded', 'true') + await userEvent.keyboard('{Escape}') + await expect.element(button).toHaveAttribute('aria-expanded', 'false') + await expect.element(button).toHaveFocus() + unmount() + expect(anchorRef.current).toBeNull() + }) +}) + describe('SelectPanel state synchronization', () => { const synchronizedItems: ItemInput[] = [ {id: 'one', text: 'item one'}, diff --git a/packages/react/src/SelectPanel/SelectPanel.tsx b/packages/react/src/SelectPanel/SelectPanel.tsx index f3b655065d1..5248b67dbd5 100644 --- a/packages/react/src/SelectPanel/SelectPanel.tsx +++ b/packages/react/src/SelectPanel/SelectPanel.tsx @@ -1,6 +1,6 @@ import {SearchIcon, TriangleDownIcon, XIcon, type IconProps} from '@primer/octicons-react' import React, {useCallback, useEffect, useMemo, useRef, useState, type JSX} from 'react' -import type {AnchoredOverlayProps} from '../AnchoredOverlay' +import type {AnchoredOverlayProps, AnchoredOverlayAnchorContext} from '../AnchoredOverlay' import {AnchoredOverlay} from '../AnchoredOverlay' import type {AnchoredOverlayWrapperAnchorProps} from '../AnchoredOverlay/AnchoredOverlay' import type {FilteredActionListProps} from '../FilteredActionList' @@ -632,16 +632,19 @@ function Panel({ const selectedValueText = selectedItems.length ? selectedItems.map(item => item.text).join(', ') : placeholder const shouldAutoWireLabel = isReferenced === false && Boolean(labelId) && Boolean(selectedValueId) - return >(props: T) => { - return renderAnchor({ - ...props, - ...(shouldAutoWireLabel - ? { - 'aria-labelledby': [labelId, selectedValueId].filter(Boolean).join(' '), - } - : {}), - children: shouldAutoWireLabel ? {selectedValueText} : selectedValueText, - }) + return >(props: T, context: AnchoredOverlayAnchorContext) => { + return renderAnchor( + { + ...props, + ...(shouldAutoWireLabel + ? { + 'aria-labelledby': [labelId, selectedValueId].filter(Boolean).join(' '), + } + : {}), + children: shouldAutoWireLabel ? {selectedValueText} : selectedValueText, + }, + context, + ) } }, [placeholder, renderAnchor, selected, isReferenced, labelId, selectedValueId]) diff --git a/packages/react/src/TooltipV2/Tooltip.docs.json b/packages/react/src/TooltipV2/Tooltip.docs.json index 019128ada7d..07f2f161bf0 100644 --- a/packages/react/src/TooltipV2/Tooltip.docs.json +++ b/packages/react/src/TooltipV2/Tooltip.docs.json @@ -35,6 +35,11 @@ } ], "props": [ + { + "name": "renderAnchor", + "type": "(props: TooltipAnchorProps) => React.ReactElement", + "description": "Render a single interactive trigger instead of children. Forward every supplied prop, including the callback ref, to that element. Tooltip combines its internal ref with any forwarded ref." + }, { "name": "direction", "type": "'n' | 'ne' | 'e' | 'se' | 's' | 'sw' | 'w' | 'nw'", diff --git a/packages/react/src/TooltipV2/Tooltip.tsx b/packages/react/src/TooltipV2/Tooltip.tsx index 82cd3c36ed0..99d30897a09 100644 --- a/packages/react/src/TooltipV2/Tooltip.tsx +++ b/packages/react/src/TooltipV2/Tooltip.tsx @@ -18,6 +18,11 @@ import {reactMajorVersion} from '../utils/environment' export type TooltipDirection = 'nw' | 'n' | 'ne' | 'e' | 'se' | 's' | 'sw' | 'w' export type TooltipProps = React.PropsWithChildren<{ + /** + * Render a single interactive trigger instead of passing children. + * Forward all supplied props, including ref, to that element. + */ + renderAnchor?: (props: TooltipAnchorProps) => React.ReactElement direction?: TooltipDirection text: string type?: 'label' | 'description' @@ -42,19 +47,8 @@ export type TooltipProps = React.PropsWithChildren<{ }> & React.HTMLAttributes -type TriggerPropsType = Pick< - React.HTMLAttributes, - | 'aria-describedby' - | 'aria-labelledby' - | 'onBlur' - | 'onTouchEnd' - | 'onFocus' - | 'onMouseOverCapture' - | 'onMouseLeave' - | 'onTouchCancel' - | 'onTouchEnd' -> & { - ref?: React.Ref +export type TooltipAnchorProps = React.HTMLAttributes & { + ref: React.RefCallback } // map tooltip direction to anchoredPosition props @@ -117,6 +111,7 @@ export const Tooltip: ForwardRefExoticComponent< text, type = 'description', children, + renderAnchor, id, className, keybindingHint = emptyKeybindingHints, @@ -127,7 +122,8 @@ export const Tooltip: ForwardRefExoticComponent< forwardedRef, ) => { const tooltipId = useId(id) - const child = Children.only(children) + invariant(!renderAnchor || children == null, 'Pass either `children` or `renderAnchor` to `Tooltip`, not both.') + const child = renderAnchor ? undefined : Children.only(children) const isFragmentTrigger = React.isValidElement(child) && child.type === React.Fragment warning( isFragmentTrigger, @@ -142,8 +138,8 @@ export const Tooltip: ForwardRefExoticComponent< // Feature-flag scaffolding for `primer_react_merged_forwarded_refs`. // At graduation: remove the three declarations below, and replace all instances of `readTriggerRef` with `triggerRef` and `appliedTriggerRef` with `mergedTriggerRef`. const providedOrCreatedRef = useProvidedRefOrCreate((forwardedRef ?? childRef) as React.RefObject) - const readTriggerRef = mergedRefEnabled ? triggerRef : providedOrCreatedRef - const appliedTriggerRef = mergedRefEnabled ? mergedTriggerRef : providedOrCreatedRef + const readTriggerRef = renderAnchor || mergedRefEnabled ? triggerRef : providedOrCreatedRef + const appliedTriggerRef = renderAnchor || mergedRefEnabled ? mergedTriggerRef : providedOrCreatedRef const tooltipElRef = useRef(null) const [calculatedDirection, setCalculatedDirection] = useState(direction) @@ -235,6 +231,8 @@ export const Tooltip: ForwardRefExoticComponent< const value = useMemo(() => ({tooltipId}), [tooltipId]) useEffect(() => { + // The renderAnchor path intentionally selects the internal ref used by this effect. + // eslint-disable-next-line react-you-might-not-need-an-effect/no-event-handler if (!tooltipElRef.current || !readTriggerRef.current) return const trigger = readTriggerRef.current const isInvalidTrigger = !(trigger instanceof HTMLElement) @@ -242,6 +240,7 @@ export const Tooltip: ForwardRefExoticComponent< isInvalidTrigger, 'The `Tooltip` component expects its trigger ref to resolve to an HTML element. Ensure the trigger forwards its ref to a single interactive element instead of a React Fragment.', ) + // eslint-disable-next-line react-you-might-not-need-an-effect/no-event-handler if (isInvalidTrigger) return /* * ACCESSIBILITY CHECKS @@ -307,72 +306,76 @@ export const Tooltip: ForwardRefExoticComponent< if (isFragmentTrigger) return child - return ( - - <> - {React.isValidElement(child) && - React.cloneElement(child as React.ReactElement, { - // @ts-expect-error the provided-or-created ref path needs a non nullable ref - 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') { - return child.props['aria-describedby'] - } + const triggerProps: React.HTMLAttributes & React.RefAttributes = { + // @ts-expect-error the provided-or-created ref path needs a non nullable ref + 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') { + return elementChild?.props['aria-describedby'] + } + + // If tooltip is a description type, append our tooltipId + const existingDescribedBy = elementChild?.props['aria-describedby'] + if (existingDescribedBy) { + return `${existingDescribedBy} ${tooltipId}` + } - // If tooltip is a description type, append our tooltipId - const existingDescribedBy = child.props['aria-describedby'] - if (existingDescribedBy) { - return `${existingDescribedBy} ${tooltipId}` - } + // If no existing aria-describedby, use our tooltipId + return tooltipId + })(), + // If it is a label type, we use tooltip to label the trigger + 'aria-labelledby': type === 'label' ? tooltipId : elementChild?.props['aria-labelledby'], + onBlur: (event: React.FocusEvent) => { + closeTooltip() + elementChild?.props.onBlur?.(event) + }, + onTouchEnd: (event: React.TouchEvent) => { + elementChild?.props.onTouchEnd?.(event) - // If no existing aria-describedby, use our tooltipId - return tooltipId - })(), - // If it is a label type, we use tooltip to label the trigger - 'aria-labelledby': type === 'label' ? tooltipId : child.props['aria-labelledby'], - onBlur: (event: React.FocusEvent) => { - closeTooltip() - child.props.onBlur?.(event) - }, - onTouchEnd: (event: React.TouchEvent) => { - child.props.onTouchEnd?.(event) + // Hide tooltips on tap to essentially disable them on touch devices; + // this still allows viewing the tooltip on tap-and-hold + safeSetTimeout(() => closeTooltip(), 10) + }, + onFocus: (event: React.FocusEvent) => { + // only show tooltip on :focus-visible, not on :focus + try { + if (!event.target.matches(':focus-visible')) return + } catch (_error) { + // jsdom (jest) does not support `:focus-visible` yet and would throw an error + // https://github.com/jsdom/jsdom/issues/3426 + } - // Hide tooltips on tap to essentially disable them on touch devices; - // this still allows viewing the tooltip on tap-and-hold - safeSetTimeout(() => closeTooltip(), 10) - }, - onFocus: (event: React.FocusEvent) => { - // only show tooltip on :focus-visible, not on :focus - try { - if (!event.target.matches(':focus-visible')) return - } catch (_error) { - // jsdom (jest) does not support `:focus-visible` yet and would throw an error - // https://github.com/jsdom/jsdom/issues/3426 - } + openTooltip() + elementChild?.props.onFocus?.(event) + }, + onMouseOverCapture: (event: React.MouseEvent) => { + 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`) + // show tooltip after mouse has been hovering for the specified delay time + // (prevent showing tooltip when mouse is just passing through) + openTimeoutRef.current = safeSetTimeout(() => { + // if the mouse is already moved out, do not show the tooltip + if (!openTimeoutRef.current) return + openTooltip() + elementChild?.props.onMouseEnter?.(event) + }, delayTime) + }, + onMouseLeave: (event: React.MouseEvent) => { + closeTooltip() + elementChild?.props.onMouseLeave?.(event) + }, + } - openTooltip() - child.props.onFocus?.(event) - }, - onMouseOverCapture: (event: React.MouseEvent) => { - 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`) - // show tooltip after mouse has been hovering for the specified delay time - // (prevent showing tooltip when mouse is just passing through) - openTimeoutRef.current = safeSetTimeout(() => { - // if the mouse is already moved out, do not show the tooltip - if (!openTimeoutRef.current) return - openTooltip() - child.props.onMouseEnter?.(event) - }, delayTime) - }, - onMouseLeave: (event: React.MouseEvent) => { - closeTooltip() - child.props.onMouseLeave?.(event) - }, - })} + return ( + + <> + {renderAnchor + ? renderAnchor({...triggerProps, ref: mergedTriggerRef}) + : React.isValidElement(child) && + React.cloneElement(child as React.ReactElement, triggerProps)} & {text?: string}) => ( @@ -59,6 +61,80 @@ function ExampleWithActionMenu({actionMenuTrigger}: {actionMenuTrigger: React.Re } describe('Tooltip', () => { + it('composes a tooltip with a popover anchor on a single element', async () => { + function Example() { + const [open, setOpen] = useState(false) + return ( + setOpen(true)} + onClose={() => setOpen(false)} + renderAnchor={(anchorProps, {ref}) => ( + ( + + )} + /> + )} + > + + + ) + } + + HTMLRender() + const button = page.getByRole('button', {name: 'Pages', exact: true}) + expect(document.querySelectorAll('button')).toHaveLength(1) + await userEvent.click(button) + await expect.element(button).toHaveAttribute('aria-expanded', 'true') + const close = page.getByRole('button', {name: 'Close details', exact: true}) + await expect.element(close).toHaveFocus() + await userEvent.click(close) + await expect.element(button).toHaveAttribute('aria-expanded', 'false') + await expect.element(button).toHaveFocus() + }) + + it('renders and labels a custom trigger with a callback ref without requiring a feature flag', async () => { + const ref = createRef() + const {unmount} = HTMLRender( + + )} + > + + , + ), + ).toThrow('Pass either `children` or `renderAnchor` to `Tooltip`, not both.') + }) + }) + implementsClassName(TooltipComponent, classes.Tooltip) it('renders `data-direction="s"` by default', () => { const {getByText} = HTMLRender() diff --git a/packages/react/src/TooltipV2/index.ts b/packages/react/src/TooltipV2/index.ts index 9a336ded38c..e39076959e1 100644 --- a/packages/react/src/TooltipV2/index.ts +++ b/packages/react/src/TooltipV2/index.ts @@ -1,2 +1,2 @@ export {Tooltip} from './Tooltip' -export type {TooltipProps, TooltipDirection} from './Tooltip' +export type {TooltipProps, TooltipDirection, TooltipAnchorProps} from './Tooltip' diff --git a/packages/react/src/__tests__/__snapshots__/exports.test.ts.snap b/packages/react/src/__tests__/__snapshots__/exports.test.ts.snap index ee096520739..e81052616e4 100644 --- a/packages/react/src/__tests__/__snapshots__/exports.test.ts.snap +++ b/packages/react/src/__tests__/__snapshots__/exports.test.ts.snap @@ -18,9 +18,11 @@ exports[`@primer/react > should not update exports without a semver change 1`] = "type ActionListTrailingVisualProps", "ActionMenu", "type ActionMenuAnchorProps", + "type ActionMenuAnchorRenderProps", "type ActionMenuButtonProps", "type ActionMenuProps", "AnchoredOverlay", + "type AnchoredOverlayAnchorContext", "type AnchoredOverlayProps", "AnchoredPositionHookSettings", "AriaAlert", @@ -218,6 +220,7 @@ exports[`@primer/react > should not update exports without a semver change 1`] = "Token", "type TokenProps", "Tooltip", + "type TooltipAnchorProps", "type TooltipProps", "type TouchOrMouseEvent", "TreeView", diff --git a/packages/react/src/deprecated/ActionMenu.tsx b/packages/react/src/deprecated/ActionMenu.tsx index fa0cf1de949..703cd82f40d 100644 --- a/packages/react/src/deprecated/ActionMenu.tsx +++ b/packages/react/src/deprecated/ActionMenu.tsx @@ -8,6 +8,7 @@ import {Button} from '../Button' import type React from 'react' import {useCallback, useMemo, type JSX} from 'react' import {AnchoredOverlay} from '../AnchoredOverlay' +import type {AnchoredOverlayAnchorContext} from '../AnchoredOverlay' import {useProvidedStateOrCreate} from '../hooks/useProvidedStateOrCreate' import type {OverlayProps} from '../Overlay' import {useProvidedRefOrCreate} from '../hooks' @@ -68,12 +69,15 @@ const ActionMenuBase = ({ if (renderAnchor === null) { return null } - return >(props: T) => { - return renderAnchor({ - 'aria-label': 'menu', - children: anchorContent, - ...props, - }) + return >(props: T, context: AnchoredOverlayAnchorContext) => { + return renderAnchor( + { + 'aria-label': 'menu', + children: anchorContent, + ...props, + }, + context, + ) } }, [anchorContent, renderAnchor]) diff --git a/packages/react/src/index.ts b/packages/react/src/index.ts index 48750d9e583..d1a4a4d4b09 100644 --- a/packages/react/src/index.ts +++ b/packages/react/src/index.ts @@ -85,9 +85,14 @@ export type { ActionListTrailingVisualProps, } from './ActionList' export {ActionMenu} from './ActionMenu' -export type {ActionMenuProps, ActionMenuAnchorProps, ActionMenuButtonProps} from './ActionMenu' +export type { + ActionMenuProps, + ActionMenuAnchorProps, + ActionMenuButtonProps, + ActionMenuAnchorRenderProps, +} from './ActionMenu' export {AnchoredOverlay} from './AnchoredOverlay' -export type {AnchoredOverlayProps} from './AnchoredOverlay' +export type {AnchoredOverlayProps, AnchoredOverlayAnchorContext} from './AnchoredOverlay' export {default as Autocomplete} from './Autocomplete' export type {AutocompleteMenuProps, AutocompleteInputProps, AutocompleteOverlayProps} from './Autocomplete' export {default as Avatar} from './Avatar' @@ -209,7 +214,7 @@ export type { export {default as Token, IssueLabelToken} from './Token' export type {TokenProps, IssueLabelTokenProps} from './Token' export {Tooltip} from './TooltipV2' -export type {TooltipProps} from './TooltipV2' +export type {TooltipProps, TooltipAnchorProps} from './TooltipV2' export {default as Truncate} from './Truncate' export type {TruncateProps} from './Truncate' From 3507ca3bb59794912e0f67c5615a518201fbc261 Mon Sep 17 00:00:00 2001 From: Adam Dierkens Date: Fri, 13 Nov 2026 11:27:17 -0500 Subject: [PATCH 2/9] Preserve callback invocation and composed ARIA relationships Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- contributor-docs/extending-components.md | 30 +++++++++-- .../AnchoredOverlay/AnchoredOverlay.docs.json | 2 +- .../src/AnchoredOverlay/AnchoredOverlay.tsx | 2 +- .../AnchoredOverlay.types.test.tsx | 10 ++-- .../src/SelectPanel/SelectPanel.test.tsx | 6 +-- .../react/src/SelectPanel/SelectPanel.tsx | 2 +- packages/react/src/TooltipV2/Tooltip.tsx | 22 ++------ .../src/TooltipV2/__tests__/Tooltip.test.tsx | 50 ++++++++++++++++++- packages/react/src/deprecated/ActionMenu.tsx | 2 +- 9 files changed, 93 insertions(+), 33 deletions(-) diff --git a/contributor-docs/extending-components.md b/contributor-docs/extending-components.md index e5fc68fe815..e6b59d5511a 100644 --- a/contributor-docs/extending-components.md +++ b/contributor-docs/extending-components.md @@ -79,6 +79,27 @@ element, not a wrapper or imperative handle. trigger. Use the existing `inactive` button pattern if an unavailable action needs an explanation. +Tooltip supplies only the ARIA relationship for its selected type. A description +tooltip does not supply `aria-labelledby`, so spreading its props preserves an +overlay's existing label. A label tooltip similarly leaves existing descriptions +alone. When both behaviors supply description IDs, explicitly combine them on the +final element; the render callback cannot inspect the props you pass separately: + +```tsx + ( + + )} +/> +``` + `Popover` is presentational and does not provide trigger behavior. For a custom popover or picker interaction, `AnchoredOverlay` and `SelectPanel` provide the object ref in a second callback argument: @@ -88,9 +109,9 @@ object ref in a second callback argument: open={open} onOpen={() => setOpen(true)} onClose={() => setOpen(false)} - renderAnchor={(anchorProps, {ref}) => ( + renderAnchor={(anchorProps, context) => ( ( } /> + } +> + + + Assign me + + + ``` -The menu's click/keyboard handlers and tooltip's focus/pointer handlers are -disjoint in this example. The tooltip combines the menu ref with its own ref and -any forwarded ref. The final callback ref reaches the same button used for -positioning, outside-click handling, and focus return. Callback and object refs -passed to `Tooltip` are supported in this path without a feature flag. - -`renderAnchor` is a rendering callback, not a component: do not call hooks inside -it. Render a separate component if you need hooks, such as `useMergedRefs` from -`@primer/react`, to attach additional refs or route the supplied ref through a -custom prop. All behavior refs must resolve to the actual interactive HTML -element, not a wrapper or imperative handle. - -### Prop ownership - -- Spread every supplied prop onto the single interactive element, including the - ref. Do not nest buttons. Use either `Tooltip` children or `renderAnchor`, and - either configured menu anchors or `ActionMenu.renderAnchor`, not both. -- Keep the generated `id`, `aria-haspopup`, `aria-expanded`, and tooltip - `aria-labelledby`/`aria-describedby` relationships on that element. If adding - another description, append its ID to `aria-describedby` rather than replacing - the tooltip ID. Use `type="description"` for an already named button, or - `type="label"` to name an otherwise unnamed trigger. - When spreading onto `IconButton` with an explicit `aria-label`, set - `aria-labelledby={undefined}` to satisfy its mutually exclusive naming props. -- Call supplied event handlers before additional handlers, and skip additional - handlers when `event.defaultPrevented` is true, following Primer's prop-merging - convention. If deliberately cancelling menu activation, call `preventDefault` - before forwarding the event; menu click and keyboard activation respect it. -- If props overlap, merge them explicitly rather than relying on spread order: - combine class names, apply custom styles after supplied styles, and compose - handlers as above. Render callbacks do not automatically merge props you add. -- Native `disabled` buttons remain non-interactive. As with the existing - `Tooltip` children API, do not use a disabled native button as a tooltip - trigger. Use the existing `inactive` button pattern if an unavailable action - needs an explanation. - -Tooltip supplies only the ARIA relationship for its selected type. A description -tooltip does not supply `aria-labelledby`, so spreading its props preserves an -overlay's existing label. A label tooltip similarly leaves existing descriptions -alone. When both behaviors supply description IDs, explicitly combine them on the -final element; the render callback cannot inspect the props you pass separately: +The button's handler and menu's toggle both run. The caller does not extract a +ref, forward an internal handler, or merge tooltip description IDs. An existing +button ref continues to receive that same button, alongside the behaviors' refs. -```tsx - ( - - )} -/> -``` +Configured menus already render one native button, too. The benefit is not fewer +DOM nodes: element rendering removes the need for the menu to recognize specific +tooltip/anchor nesting patterns, and gives composed behaviors a consistent +prop-merging contract. -`Popover` is presentational and does not provide trigger behavior. For a custom -popover or picker interaction, `AnchoredOverlay` and `SelectPanel` provide the -object ref in a second callback argument: +### Direct overlay and tooltip ```tsx - setOpen(true)} onClose={() => setOpen(false)} - renderAnchor={(anchorProps, context) => ( + renderAnchor={ ( - - )} + text="Message details" + type="description" + renderAnchor={} /> - )} + } > {children} ``` -The context ref is the same object already supplied at runtime in the anchor -props. It is also the supplied `anchorRef`, if present. Existing one-argument -callbacks, one-argument callback invocations, and object-ref behavior are -unchanged. Primer always supplies the context, but its type is optional for -compatibility with callers invoking a render callback themselves; use -`context?.ref` when forwarding it. Keeping the additional typed ref -in a separate context avoids making existing prop spreads incompatible with -more narrowly typed button and link refs. - -This API does not introduce an independent focus-return target or replace either -component's overlay behavior. +The overlay and tooltip use the same DOM anchor for positioning, outside-click +handling, and focus return. No broad object ref needs to be spread into a more +narrowly typed button ref. This does not introduce an independent focus-return +target or change overlay state management. + +### Element merging contract + +| Prop | Behavior | +| ------------------------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `ref` | All behavior, forwarded, and render-element refs receive the same DOM node; React 19 cleanup functions are preserved. | +| Event handlers | The render element's handler runs first, followed by supplied handlers unless `defaultPrevented`. `stopPropagation()` does not cancel the internal handler. | +| `className` | Both values are combined, render element first. | +| `style` | Shallow merge; the render element's values win. | +| `aria-labelledby`, `aria-describedby` | ID lists are combined, render element first, removing duplicates. An omitted or undefined relationship does not erase another behavior's IDs. | +| Other attributes | The render element's value wins, including `id`. Menu labelling follows the actual anchor's ID or label relationship. | + +Two deliberate differences from Base UI apply: Primer uses the existing +`preventDefault()` cancellation convention rather than adding +`preventBaseUIHandler()`, and preserves ARIA ID lists rather than overwriting +them. Element-first event ordering is an intentional exception to Primer's +usual component-first prop-merging convention for this experiment. + +In the experimental Tooltip's **render mode**, HTML attributes such as `id`, +`className`, `style`, ARIA attributes, and handlers configure its **trigger**, not +its tooltip bubble. `text`, `type`, `direction`, `delay`, and `keybindingHint` +still configure the tooltip. The existing children mode retains its original +attribute routing. + +### Responsibilities and escape hatches + +- Use `renderAnchor` or a configured menu anchor, not both. Similarly, provide + either Tooltip children or `renderAnchor`. Never nest native buttons. +- Custom render elements must forward their ref and received trigger props to + the interactive DOM element, not a wrapper or imperative handle. +- Use a description tooltip for a button that already has an accessible name; + use a label tooltip for an otherwise unnamed trigger. Existing description IDs + on the button are preserved automatically in element mode. +- Disabled behavior is not a generic boolean merge. Configure the actual + rendered control using its normal `disabled` or `inactive` API. Native disabled + controls cannot open the overlay or tooltip. `aria-disabled` controls cannot + activate the overlay but remain focusable for an explanatory tooltip; Tab is + not cancelled. No new root-level disabled state is introduced. +- Manage controlled open state normally. Composing triggers does not arbitrate + between independently controlled menus, dialogs, or overlays. + +Function-valued `renderAnchor` remains an escape hatch. It receives the supplied +props and ref, but the caller owns how those props are applied and merged with +the returned element's props. Spread order alone cannot preserve overlapping +handlers or ARIA IDs. Do not call hooks inside render callbacks; render a +component when hooks are needed. + +The original `AnchoredOverlay` callback still receives one argument with its +original runtime object ref. There is no new second-argument ref context. diff --git a/packages/react/src/ActionMenu/ActionMenu.docs.json b/packages/react/src/ActionMenu/ActionMenu.docs.json index 86604378ca6..3df8c5d079b 100644 --- a/packages/react/src/ActionMenu/ActionMenu.docs.json +++ b/packages/react/src/ActionMenu/ActionMenu.docs.json @@ -28,11 +28,6 @@ ], "importPath": "@primer/react", "props": [ - { - "name": "renderAnchor", - "type": "(props: ActionMenuAnchorRenderProps) => React.ReactElement", - "description": "Render a custom trigger instead of ActionMenu.Button or ActionMenu.Anchor. Forward every supplied prop, including ref, to a single interactive HTML element." - }, { "name": "children", "type": "React.ReactElement[]", diff --git a/packages/react/src/ActionMenu/ActionMenu.test.tsx b/packages/react/src/ActionMenu/ActionMenu.test.tsx index bef3954cdb7..abac30c74bc 100644 --- a/packages/react/src/ActionMenu/ActionMenu.test.tsx +++ b/packages/react/src/ActionMenu/ActionMenu.test.tsx @@ -21,8 +21,14 @@ import type {JSX} from 'react' import {implementsClassName} from '../utils/testing' import {createRenderCounter} from '../utils/testing/profiler' import {FeatureFlags} from '../FeatureFlags' +import { + ActionMenu as ExperimentalActionMenu, + Tooltip as ExperimentalTooltip, +} from '../experimental/TriggerComposition/TriggerComposition' describe('ActionMenu renderAnchor', () => { + const ActionMenu = ExperimentalActionMenu + const TooltipV2 = ExperimentalTooltip it.each([false, true])('composes a tooltip and menu on one button with merged refs (flag: %s)', async mergedRefs => { const anchorRef = createRef() const tooltipRef = vi.fn() diff --git a/packages/react/src/ActionMenu/ActionMenu.tsx b/packages/react/src/ActionMenu/ActionMenu.tsx index 07642d5daf0..455e3e0db1f 100644 --- a/packages/react/src/ActionMenu/ActionMenu.tsx +++ b/packages/react/src/ActionMenu/ActionMenu.tsx @@ -20,6 +20,8 @@ import {isSlot} from '../utils/is-slot' import type {FCWithSlotMarker, WithSlotMarker} from '../utils/types/Slots' import {DialogContext} from '../Dialog/DialogContext' import useLayoutEffect from '../utils/useIsomorphicLayoutEffect' +import type {AnchorRenderProps} from '../internal/components/RenderAnchor' +import {invariant} from '../utils/invariant' export type MenuCloseHandler = ( gesture: 'anchor-click' | 'click-outside' | 'escape' | 'tab' | 'item-select' | 'arrow-left' | 'close', @@ -31,15 +33,11 @@ export type MenuContextProps = Pick< > & { onClose?: MenuCloseHandler isSubmenu?: boolean + isCustomAnchor?: boolean } const MenuContext = React.createContext({renderAnchor: null, open: false}) export type ActionMenuProps = { - /** - * Render a custom trigger instead of ActionMenu.Button or ActionMenu.Anchor. - * Forward all supplied props, including ref, to a single interactive element. - */ - renderAnchor?: (props: ActionMenuAnchorRenderProps) => React.ReactElement /** * Recommended: `ActionMenu.Button` or `ActionMenu.Anchor` with `ActionMenu.Overlay` */ @@ -57,10 +55,6 @@ export type ActionMenuProps = { onOpenChange?: (s: boolean) => void } & Pick -export type ActionMenuAnchorRenderProps = Omit, 'aria-label' | 'aria-labelledby'> & { - ref: React.RefCallback -} - // anchorProps adds onClick and onKeyDown, so we need to merge them with buttonProps const mergeAnchorHandlers = (anchorProps: React.HTMLAttributes, buttonProps: ButtonProps) => { const mergedAnchorProps = {...anchorProps} @@ -90,13 +84,15 @@ const mergeAnchorHandlers = (anchorProps: React.HTMLAttributes, but return mergedAnchorProps } -const Menu: FCWithSlotMarker> = ({ +type ActionMenuImplProps = ActionMenuProps & {renderAnchor?: (props: AnchorRenderProps) => React.ReactElement} + +export const ActionMenuImpl: FCWithSlotMarker> = ({ anchorRef: externalAnchorRef, open, onOpenChange, children, renderAnchor: customRenderAnchor, -}: ActionMenuProps) => { +}: ActionMenuImplProps) => { const parentMenuContext = useContext(MenuContext) const [combinedOpenState, setCombinedOpenState] = useProvidedStateOrCreate(open, onOpenChange, false) @@ -124,6 +120,19 @@ const Menu: FCWithSlotMarker> = ({ (child.type === MenuButton || child.type === Anchor || isSlot(child, Anchor) || isSlot(child, MenuButton)), ) const menuButtonChildId = React.isValidElement(menuButtonChild) ? menuButtonChild.props.id : undefined + const hasTooltipAnchor = + customRenderAnchor && + React.Children.toArray(children).some( + child => + React.isValidElement(child) && + (child.type === Tooltip || isSlot(child, Tooltip)) && + React.isValidElement(child.props.children) && + (child.props.children.type === MenuButton || isSlot(child.props.children, MenuButton)), + ) + invariant( + !customRenderAnchor || (!menuButtonChild && !hasTooltipAnchor), + 'Pass either `renderAnchor` or `ActionMenu.Button`/`ActionMenu.Anchor`, not both.', + ) const anchorRef = useProvidedRefOrCreate(externalAnchorRef) const callbackAnchorRef = useMergedRefs(anchorRef, undefined) @@ -203,8 +212,9 @@ const Menu: FCWithSlotMarker> = ({ onOpen, onClose, isSubmenu, + isCustomAnchor: Boolean(customRenderAnchor), }), - [anchorRef, renderAnchor, anchorId, combinedOpenState, onOpen, onClose, isSubmenu], + [anchorRef, renderAnchor, anchorId, combinedOpenState, onOpen, onClose, isSubmenu, customRenderAnchor], ) return {contents} @@ -314,6 +324,7 @@ const Overlay: FCWithSlotMarker> = ({ onOpen, onClose, isSubmenu = false, + isCustomAnchor = false, } = React.useContext(MenuContext) as MandateProps const containerRef = React.useRef(null) @@ -345,10 +356,12 @@ const Overlay: FCWithSlotMarker> = ({ const [anchorAriaLabelledby, setAnchorAriaLabelledby] = useState(null) useLayoutEffect(() => { if (open) { - setAnchorAriaLabelledby(anchorRef.current?.getAttribute('aria-labelledby') ?? null) + setAnchorAriaLabelledby( + anchorRef.current?.getAttribute('aria-labelledby') || (isCustomAnchor ? anchorRef.current?.id : null) || null, + ) setAnchorLabelReady(true) } - }, [anchorRef, open]) + }, [anchorRef, open, isCustomAnchor]) const afterSelect = useCallback(() => onClose?.('item-select'), [onClose]) @@ -400,6 +413,7 @@ const Overlay: FCWithSlotMarker> = ({ ) } +const Menu: FCWithSlotMarker> = ActionMenuImpl Menu.displayName = 'ActionMenu' MenuButton.__SLOT__ = Symbol('ActionMenu.Button') diff --git a/packages/react/src/ActionMenu/ActionMenu.types.test.tsx b/packages/react/src/ActionMenu/ActionMenu.types.test.tsx index ef029b079b4..1accc0c836f 100644 --- a/packages/react/src/ActionMenu/ActionMenu.types.test.tsx +++ b/packages/react/src/ActionMenu/ActionMenu.types.test.tsx @@ -1,5 +1,5 @@ import {ActionMenu} from '.' -import {Tooltip} from '../TooltipV2' +import {ActionMenu as ExperimentalActionMenu, Tooltip} from '../experimental/TriggerComposition/TriggerComposition' import {Button} from '../Button' export function actionButtonWithoutProps() { @@ -29,6 +29,7 @@ export function actionOverlayWithInvalidPreventFocusOnClose() { } export function actionMenuWithComposedTrigger() { + const ActionMenu = ExperimentalActionMenu return ( ( diff --git a/packages/react/src/AnchoredOverlay/AnchoredOverlay.docs.json b/packages/react/src/AnchoredOverlay/AnchoredOverlay.docs.json index 362e64307d6..75fe3cf0487 100644 --- a/packages/react/src/AnchoredOverlay/AnchoredOverlay.docs.json +++ b/packages/react/src/AnchoredOverlay/AnchoredOverlay.docs.json @@ -64,9 +64,9 @@ }, { "name": "renderAnchor", - "type": ">(props: T, context?: AnchoredOverlayAnchorContext) => JSX.Element", + "type": ">(props: T) => JSX.Element", "defaultValue": "", - "description": "Render the anchor element. The second argument exposes the same object ref already supplied in the props, for composing trigger behaviors. Existing one-argument callbacks remain supported. When renderAnchor is null, an anchorRef is required." + "description": "A custom function component used to render the anchor element. When renderAnchor is null, an anchorRef is required." }, { "name": "anchorRef", diff --git a/packages/react/src/AnchoredOverlay/AnchoredOverlay.tsx b/packages/react/src/AnchoredOverlay/AnchoredOverlay.tsx index 6d0a6af05bb..cedbce6f149 100644 --- a/packages/react/src/AnchoredOverlay/AnchoredOverlay.tsx +++ b/packages/react/src/AnchoredOverlay/AnchoredOverlay.tsx @@ -18,19 +18,13 @@ import {useFeatureFlag} from '../FeatureFlags' import {widthMap} from '../Overlay/constants' import {reactMajorVersion} from '../utils/environment' -export type AnchoredOverlayAnchorContext = { - ref: {readonly current: HTMLElement | null} -} - interface AnchoredOverlayPropsWithAnchor { /** * A custom function component used to render the anchor element. * Will receive the selected text as `children` prop when an item is activated. - * The second argument exposes the anchor's object ref for composing behaviors. */ renderAnchor: , 'aria-label' | 'aria-labelledby'>>( props: T, - context?: AnchoredOverlayAnchorContext, ) => JSX.Element /** @@ -413,7 +407,7 @@ export const AnchoredOverlay: React.FC {renderAnchor && renderAnchor( - // eslint-disable-next-line react-hooks/refs + // eslint-disable-next-line react-hooks/refs -- the callback forwards the object ref to an element { ref: anchorRef, id: anchorId, @@ -424,8 +418,6 @@ export const AnchoredOverlay: React.FC ( - ( - - )} - /> - )} + renderAnchor={Info} />} >

GitHub

- + ) } diff --git a/packages/react/src/AnchoredOverlay/index.ts b/packages/react/src/AnchoredOverlay/index.ts index 2db56bee504..0a89ba09da0 100644 --- a/packages/react/src/AnchoredOverlay/index.ts +++ b/packages/react/src/AnchoredOverlay/index.ts @@ -1,2 +1,2 @@ export {AnchoredOverlay} from './AnchoredOverlay' -export type {AnchoredOverlayProps, AnchoredOverlayAnchorContext} from './AnchoredOverlay' +export type {AnchoredOverlayProps} from './AnchoredOverlay' diff --git a/packages/react/src/SelectPanel/SelectPanel.test.tsx b/packages/react/src/SelectPanel/SelectPanel.test.tsx index b55a4b60fa7..6e4054a89e2 100644 --- a/packages/react/src/SelectPanel/SelectPanel.test.tsx +++ b/packages/react/src/SelectPanel/SelectPanel.test.tsx @@ -6,8 +6,7 @@ import {SelectPanel, type SelectPanelProps, type ItemInput, type GroupedListProp import {FeatureFlags} from '../FeatureFlags' import type {InitialLoadingType} from './SelectPanel' import type {LiveRegionElement} from '@primer/live-region-element' -import {Button, IconButton} from '../Button' -import {Tooltip} from '../TooltipV2' +import {IconButton} from '../Button' import {ArrowLeftIcon} from '@primer/octicons-react' import classes from './SelectPanel.test.module.css' import {implementsClassName} from '../utils/testing' @@ -100,37 +99,6 @@ function BasicSelectPanel(passthroughProps: Record) { globalThis.Element.prototype.scrollTo = vi.fn() -describe('SelectPanel renderAnchor context', () => { - it('forwards the unchanged object ref so a tooltip can compose with its trigger', async () => { - const anchorRef = React.createRef() - const renderAnchor = vi.fn>((props, context) => ( - ( - - )} - /> - )) - const {unmount} = render() - const button = page.getByRole('button', {name: 'Select items', exact: true}) - expect(renderAnchor.mock.calls[0][1]?.ref).toBe(anchorRef) - expect(anchorRef.current).toBe(button.element()) - const description = button.element().getAttribute('aria-describedby') - expect(document.getElementById(description ?? '')).toHaveTextContent('Choose items') - expect(document.querySelectorAll('button')).toHaveLength(1) - await userEvent.click(button) - await expect.element(button).toHaveAttribute('aria-expanded', 'true') - await userEvent.keyboard('{Escape}') - await expect.element(button).toHaveAttribute('aria-expanded', 'false') - await expect.element(button).toHaveFocus() - unmount() - expect(anchorRef.current).toBeNull() - }) -}) - describe('SelectPanel state synchronization', () => { const synchronizedItems: ItemInput[] = [ {id: 'one', text: 'item one'}, diff --git a/packages/react/src/SelectPanel/SelectPanel.tsx b/packages/react/src/SelectPanel/SelectPanel.tsx index c97079c9adc..f3b655065d1 100644 --- a/packages/react/src/SelectPanel/SelectPanel.tsx +++ b/packages/react/src/SelectPanel/SelectPanel.tsx @@ -1,6 +1,6 @@ import {SearchIcon, TriangleDownIcon, XIcon, type IconProps} from '@primer/octicons-react' import React, {useCallback, useEffect, useMemo, useRef, useState, type JSX} from 'react' -import type {AnchoredOverlayProps, AnchoredOverlayAnchorContext} from '../AnchoredOverlay' +import type {AnchoredOverlayProps} from '../AnchoredOverlay' import {AnchoredOverlay} from '../AnchoredOverlay' import type {AnchoredOverlayWrapperAnchorProps} from '../AnchoredOverlay/AnchoredOverlay' import type {FilteredActionListProps} from '../FilteredActionList' @@ -632,19 +632,16 @@ function Panel({ const selectedValueText = selectedItems.length ? selectedItems.map(item => item.text).join(', ') : placeholder const shouldAutoWireLabel = isReferenced === false && Boolean(labelId) && Boolean(selectedValueId) - return >(props: T, context?: AnchoredOverlayAnchorContext) => { - return renderAnchor( - { - ...props, - ...(shouldAutoWireLabel - ? { - 'aria-labelledby': [labelId, selectedValueId].filter(Boolean).join(' '), - } - : {}), - children: shouldAutoWireLabel ? {selectedValueText} : selectedValueText, - }, - context, - ) + return >(props: T) => { + return renderAnchor({ + ...props, + ...(shouldAutoWireLabel + ? { + 'aria-labelledby': [labelId, selectedValueId].filter(Boolean).join(' '), + } + : {}), + children: shouldAutoWireLabel ? {selectedValueText} : selectedValueText, + }) } }, [placeholder, renderAnchor, selected, isReferenced, labelId, selectedValueId]) diff --git a/packages/react/src/TooltipV2/Tooltip.docs.json b/packages/react/src/TooltipV2/Tooltip.docs.json index 07f2f161bf0..019128ada7d 100644 --- a/packages/react/src/TooltipV2/Tooltip.docs.json +++ b/packages/react/src/TooltipV2/Tooltip.docs.json @@ -35,11 +35,6 @@ } ], "props": [ - { - "name": "renderAnchor", - "type": "(props: TooltipAnchorProps) => React.ReactElement", - "description": "Render a single interactive trigger instead of children. Forward every supplied prop, including the callback ref, to that element. Tooltip combines its internal ref with any forwarded ref." - }, { "name": "direction", "type": "'n' | 'ne' | 'e' | 'se' | 's' | 'sw' | 'w' | 'nw'", diff --git a/packages/react/src/TooltipV2/Tooltip.tsx b/packages/react/src/TooltipV2/Tooltip.tsx index 18e5bc2e449..c1a2b0e063c 100644 --- a/packages/react/src/TooltipV2/Tooltip.tsx +++ b/packages/react/src/TooltipV2/Tooltip.tsx @@ -15,14 +15,10 @@ import useSafeTimeout from '../hooks/useSafeTimeout' import type {SlotMarker} from '../utils/types' import {TooltipContext} from './TooltipContext' import {reactMajorVersion} from '../utils/environment' +import type {AnchorRenderProps} from '../internal/components/RenderAnchor' export type TooltipDirection = 'nw' | 'n' | 'ne' | 'e' | 'se' | 's' | 'sw' | 'w' export type TooltipProps = React.PropsWithChildren<{ - /** - * Render a single interactive trigger instead of passing children. - * Forward all supplied props, including ref, to that element. - */ - renderAnchor?: (props: TooltipAnchorProps) => React.ReactElement direction?: TooltipDirection text: string type?: 'label' | 'description' @@ -47,10 +43,6 @@ export type TooltipProps = React.PropsWithChildren<{ }> & React.HTMLAttributes -export type TooltipAnchorProps = React.HTMLAttributes & { - ref: React.RefCallback -} - // map tooltip direction to anchoredPosition props const directionToPosition: Record = { nw: {side: 'outside-top', align: 'end'}, @@ -101,10 +93,12 @@ const isInteractive = (element: HTMLElement) => { } const emptyKeybindingHints: Array = [] -export const Tooltip: ForwardRefExoticComponent< - React.PropsWithoutRef & React.RefAttributes +type TooltipImplProps = TooltipProps & {renderAnchor?: (props: AnchorRenderProps) => React.ReactElement} + +export const TooltipImpl: ForwardRefExoticComponent< + React.PropsWithoutRef & React.RefAttributes > & - SlotMarker = React.forwardRef( + SlotMarker = React.forwardRef( ( { direction = 's', @@ -118,10 +112,11 @@ export const Tooltip: ForwardRefExoticComponent< delay = 'short', _privateDisableTooltip = false, ...rest - }: TooltipProps, + }: TooltipImplProps, forwardedRef, ) => { const tooltipId = useId(id) + const hasRenderAnchor = renderAnchor !== undefined invariant(!renderAnchor || children == null, 'Pass either `children` or `renderAnchor` to `Tooltip`, not both.') const child = renderAnchor ? undefined : Children.only(children) const isFragmentTrigger = React.isValidElement(child) && child.type === React.Fragment @@ -161,6 +156,7 @@ export const Tooltip: ForwardRefExoticComponent< ) { const tooltip = tooltipElRef.current const trigger = readTriggerRef.current + if (hasRenderAnchor && trigger.matches(':disabled')) return tooltip.showPopover() setIsPopoverOpen(true) /* @@ -246,7 +242,7 @@ export const Tooltip: ForwardRefExoticComponent< * ACCESSIBILITY CHECKS */ // Has trigger element or any of its children interactive elements? - const isTriggerInteractive = isInteractive(trigger) + const isTriggerInteractive = isInteractive(trigger) || (hasRenderAnchor && trigger.matches(':disabled')) const triggerChildren = trigger.childNodes // two levels deep const hasInteractiveDescendant = Array.from(triggerChildren).some(child => { @@ -283,7 +279,7 @@ export const Tooltip: ForwardRefExoticComponent< const tooltip = tooltipElRef.current tooltip.setAttribute('popover', 'auto') - }, [tooltipElRef, readTriggerRef, direction, type]) + }, [tooltipElRef, readTriggerRef, direction, type, hasRenderAnchor]) useOnEscapePress( (event: KeyboardEvent) => { @@ -418,4 +414,8 @@ export const Tooltip: ForwardRefExoticComponent< }, ) +export const Tooltip: ForwardRefExoticComponent< + React.PropsWithoutRef & React.RefAttributes +> & + SlotMarker = TooltipImpl Tooltip.__SLOT__ = Symbol('Tooltip') diff --git a/packages/react/src/TooltipV2/__tests__/Tooltip.test.tsx b/packages/react/src/TooltipV2/__tests__/Tooltip.test.tsx index df917f822e1..8ee1d4e105a 100644 --- a/packages/react/src/TooltipV2/__tests__/Tooltip.test.tsx +++ b/packages/react/src/TooltipV2/__tests__/Tooltip.test.tsx @@ -6,7 +6,7 @@ import {fireEvent, render as HTMLRender} from '@testing-library/react' import BaseStyles from '../../BaseStyles' import {Button, IconButton} from '../../Button' import {ActionMenu} from '../../ActionMenu' -import {AnchoredOverlay} from '../../AnchoredOverlay' +import {AnchoredOverlay, Tooltip as ExperimentalTooltip} from '../../experimental/TriggerComposition/TriggerComposition' import {ActionList} from '../../ActionList' import ButtonGroup from '../../ButtonGroup' import {XIcon} from '@primer/octicons-react' @@ -61,123 +61,117 @@ function ExampleWithActionMenu({actionMenuTrigger}: {actionMenuTrigger: React.Re } describe('Tooltip', () => { - it('preserves a composed trigger label and explicitly merges its existing description IDs', () => { - const anchorProps = {'aria-labelledby': 'anchor-label', 'aria-describedby': 'anchor-description'} - HTMLRender( - <> - Existing label - Existing description - ( - - )} - /> - )} - > - - + describe('experimental renderAnchor', () => { + const Tooltip = ExperimentalTooltip + it('preserves a composed trigger label and explicitly merges its existing description IDs', () => { + const anchorProps = {'aria-labelledby': 'anchor-label', 'aria-describedby': 'anchor-description'} + HTMLRender( + <> + Existing label + Existing description + ( + + type="label" + text="Existing label" + renderAnchor={tooltipProps => ( + } />} > - - , - ), - ).toThrow('Pass either `children` or `renderAnchor` to `Tooltip`, not both.') + + + ) + } + + HTMLRender() + const button = page.getByRole('button', {name: 'Pages', exact: true}) + expect(document.querySelectorAll('button')).toHaveLength(1) + await userEvent.click(button) + await expect.element(button).toHaveAttribute('aria-expanded', 'true') + const close = page.getByRole('button', {name: 'Close details', exact: true}) + await expect.element(close).toHaveFocus() + await userEvent.click(close) + await expect.element(button).toHaveAttribute('aria-expanded', 'false') + await expect.element(button).toHaveFocus() + }) + + it('renders and labels a custom trigger with a callback ref without requiring a feature flag', async () => { + const ref = createRef() + const {unmount} = HTMLRender( + + )} + > + + , + ), + ).toThrow('Pass either `children` or `renderAnchor` to `Tooltip`, not both.') + }) }) }) diff --git a/packages/react/src/TooltipV2/index.ts b/packages/react/src/TooltipV2/index.ts index e39076959e1..9a336ded38c 100644 --- a/packages/react/src/TooltipV2/index.ts +++ b/packages/react/src/TooltipV2/index.ts @@ -1,2 +1,2 @@ export {Tooltip} from './Tooltip' -export type {TooltipProps, TooltipDirection, TooltipAnchorProps} from './Tooltip' +export type {TooltipProps, TooltipDirection} from './Tooltip' diff --git a/packages/react/src/__tests__/__snapshots__/exports.test.ts.snap b/packages/react/src/__tests__/__snapshots__/exports.test.ts.snap index e81052616e4..3fb4af36182 100644 --- a/packages/react/src/__tests__/__snapshots__/exports.test.ts.snap +++ b/packages/react/src/__tests__/__snapshots__/exports.test.ts.snap @@ -18,11 +18,9 @@ exports[`@primer/react > should not update exports without a semver change 1`] = "type ActionListTrailingVisualProps", "ActionMenu", "type ActionMenuAnchorProps", - "type ActionMenuAnchorRenderProps", "type ActionMenuButtonProps", "type ActionMenuProps", "AnchoredOverlay", - "type AnchoredOverlayAnchorContext", "type AnchoredOverlayProps", "AnchoredPositionHookSettings", "AriaAlert", @@ -220,7 +218,6 @@ exports[`@primer/react > should not update exports without a semver change 1`] = "Token", "type TokenProps", "Tooltip", - "type TooltipAnchorProps", "type TooltipProps", "type TouchOrMouseEvent", "TreeView", @@ -305,6 +302,10 @@ exports[`@primer/react/experimental > should not update exports without a semver "type ActionBarMenuItemProps", "type ActionBarMenuProps", "type ActionBarProps", + "ActionMenu", + "type ActionMenuProps", + "AnchoredOverlay", + "type AnchoredOverlayProps", "Announce", "type AnnounceProps", "AriaAlert", diff --git a/packages/react/src/deprecated/ActionMenu.tsx b/packages/react/src/deprecated/ActionMenu.tsx index 36f34943bd9..fa0cf1de949 100644 --- a/packages/react/src/deprecated/ActionMenu.tsx +++ b/packages/react/src/deprecated/ActionMenu.tsx @@ -8,7 +8,6 @@ import {Button} from '../Button' import type React from 'react' import {useCallback, useMemo, type JSX} from 'react' import {AnchoredOverlay} from '../AnchoredOverlay' -import type {AnchoredOverlayAnchorContext} from '../AnchoredOverlay' import {useProvidedStateOrCreate} from '../hooks/useProvidedStateOrCreate' import type {OverlayProps} from '../Overlay' import {useProvidedRefOrCreate} from '../hooks' @@ -69,15 +68,12 @@ const ActionMenuBase = ({ if (renderAnchor === null) { return null } - return >(props: T, context?: AnchoredOverlayAnchorContext) => { - return renderAnchor( - { - 'aria-label': 'menu', - children: anchorContent, - ...props, - }, - context, - ) + return >(props: T) => { + return renderAnchor({ + 'aria-label': 'menu', + children: anchorContent, + ...props, + }) } }, [anchorContent, renderAnchor]) diff --git a/packages/react/src/experimental/TriggerComposition/TriggerComposition.test.tsx b/packages/react/src/experimental/TriggerComposition/TriggerComposition.test.tsx new file mode 100644 index 00000000000..c6755a652f1 --- /dev/null +++ b/packages/react/src/experimental/TriggerComposition/TriggerComposition.test.tsx @@ -0,0 +1,321 @@ +import type React from 'react' +import {createRef, useState} from 'react' +import {act, fireEvent, render} from '@testing-library/react' +import {describe, expect, it, vi} from 'vitest' +import {page, userEvent} from 'vitest/browser' +import {InfoIcon} from '@primer/octicons-react' +import {ActionMenu, AnchoredOverlay, Tooltip} from './TriggerComposition' +import {ActionMenu as StableActionMenu} from '../../ActionMenu' +import {ActionList} from '../../ActionList' +import {Button, IconButton} from '../../Button' +import {FeatureFlags} from '../../FeatureFlags' +import {withExpectedConsoleError} from '../../utils/testing' +import {reactMajorVersion} from '../../utils/environment' + +describe('experimental element renderAnchor', () => { + it.runIf(reactMajorVersion >= 19)('preserves React 19 cleanup functions through every composed ref', () => { + const cleanupTooltip = vi.fn() + const cleanupButton = vi.fn() + const tooltipRef = vi.fn((node: HTMLElement | null) => (node ? cleanupTooltip : undefined)) + const buttonRef = vi.fn((node: HTMLButtonElement | null) => (node ? cleanupButton : undefined)) + const {unmount} = render( + Owner} /> + } + > + Contents + , + ) + expect(tooltipRef).toHaveBeenLastCalledWith(page.getByRole('button', {name: 'Owner', exact: true}).element()) + unmount() + expect(cleanupTooltip).toHaveBeenCalledOnce() + expect(cleanupButton).toHaveBeenCalledOnce() + expect(tooltipRef).not.toHaveBeenCalledWith(null) + expect(buttonRef).not.toHaveBeenCalledWith(null) + }) + + it.each([false, true])('composes one menu button, all refs, and element props (merged-ref flag: %s)', async flag => { + const anchorRef = createRef() + const tooltipRef = vi.fn() + const buttonRef = createRef() + const calls: string[] = [] + const parentClick = vi.fn() + function Example() { + const [open, setOpen] = useState(false) + return ( +
{}}> + { + calls.push('menu') + setOpen(value) + }} + renderAnchor={ + { + calls.push('element') + event.stopPropagation() + }} + > + Owner + + } + /> + } + > + + + Assign me + + + +
+ ) + } + const {unmount} = render( + + + , + ) + const button = page.getByRole('button', {name: 'Owner', exact: true}) + const element = button.element() + expect(document.querySelectorAll('button')).toHaveLength(1) + expect(anchorRef.current).toBe(element) + expect(buttonRef.current).toBe(element) + expect(tooltipRef).toHaveBeenLastCalledWith(element) + expect(element).toHaveAttribute('id', 'owner-trigger') + expect(element).toHaveClass('inner', 'outer') + expect(element.className.indexOf('inner')).toBeLessThan(element.className.indexOf('outer')) + expect(element).toHaveStyle({color: 'rgb(0, 0, 255)', padding: '4px'}) + expect(element).toHaveAccessibleDescription('Choose owner') + + await userEvent.click(button) + expect(calls).toEqual(['element', 'menu']) + expect(parentClick).not.toHaveBeenCalled() + await expect.element(button).toHaveAttribute('aria-expanded', 'true') + await expect.element(page.getByRole('menu', {name: 'Owner', exact: true})).toBeVisible() + await expect.element(page.getByRole('menuitem', {name: 'Assign me', exact: true})).toHaveFocus() + await userEvent.keyboard('{Escape}') + await expect.element(button).toHaveAttribute('aria-expanded', 'false') + await expect.element(button).toHaveFocus() + await userEvent.keyboard('{ArrowDown}') + await expect.element(page.getByRole('menuitem', {name: 'Assign me', exact: true})).toHaveFocus() + + unmount() + expect(anchorRef.current).toBeNull() + expect(buttonRef.current).toBeNull() + expect(tooltipRef).toHaveBeenLastCalledWith(null) + }) + + it('lets the element prevent both pointer and keyboard menu activation without chaining handlers', async () => { + const onOpenChange = vi.fn() + render( + event.preventDefault()} onKeyDown={event => event.preventDefault()}> + Owner + + } + /> + } + > + Menu contents + , + ) + const button = page.getByRole('button', {name: 'Owner', exact: true}) + await userEvent.click(button) + await userEvent.keyboard('{ArrowDown}') + expect(onOpenChange).not.toHaveBeenCalled() + expect(button.element()).toHaveAttribute('aria-expanded', 'false') + }) + + it('merges and deduplicates inherited label and description ID lists', () => { + const {getByText} = render( + <> + Owner + Shared + Outer + Inner + + Fallback name + + } + /> + } + > + Menu contents + + , + ) + const button = page.getByRole('button', {name: 'Owner', exact: true}).element() + const tooltip = getByText('Choose owner') + expect(button).toHaveAttribute('aria-labelledby', 'label') + const ids = button.getAttribute('aria-describedby')?.split(' ') ?? [] + expect(ids).toEqual(['inner-description', 'shared', 'outer-description', tooltip.id]) + expect(button).toHaveAccessibleDescription('Inner Shared Outer Choose owner') + expect(new Set(ids).size).toBe(ids.length) + }) + + it('preserves descriptions when the tooltip supplies the accessible name', () => { + const {getByText} = render( + <> + Existing description + } + /> + , + ) + const button = page.getByRole('button', {name: 'Information', exact: true}).element() + expect(button).toHaveAttribute('aria-labelledby', getByText('Information').id) + expect(button).toHaveAccessibleDescription('Existing description') + }) + + it.each([false, true])('does not activate a disabled trigger (native: %s)', native => { + const onOpenChange = vi.fn() + render( + + Owner + + } + /> + } + > + Menu contents + , + ) + const button = page.getByRole('button', {name: 'Owner', exact: true}).element() + fireEvent.click(button) + fireEvent.keyDown(button, {key: 'Enter'}) + fireEvent.keyDown(button, {key: 'ArrowDown'}) + expect(onOpenChange).not.toHaveBeenCalled() + expect(button).toHaveAttribute('aria-expanded', 'false') + const tab = new KeyboardEvent('keydown', {key: 'Tab', bubbles: true, cancelable: true}) + expect(button.dispatchEvent(tab)).toBe(true) + }) + + it('composes a direct overlay with an icon button, description, and focus return', async () => { + const anchorRef = createRef() + const buttonRef = createRef() + function Example() { + const [open, setOpen] = useState(false) + return ( + setOpen(true)} + onClose={() => setOpen(false)} + renderAnchor={ + } + /> + } + > + + + ) + } + const {unmount} = render() + const button = page.getByRole('button', {name: 'View message info', exact: true}) + expect(document.querySelectorAll('button')).toHaveLength(1) + expect(button.element()).toHaveAccessibleDescription('Message details') + expect(anchorRef.current).toBe(button.element()) + expect(buttonRef.current).toBe(button.element()) + act(() => button.element().focus()) + await userEvent.keyboard('{ArrowDown}') + await expect.element(page.getByRole('button', {name: 'Close details', exact: true})).toHaveFocus() + await userEvent.keyboard('{Escape}') + await expect.element(button).toHaveAttribute('aria-expanded', 'false') + await expect.element(button).toHaveFocus() + unmount() + expect(anchorRef.current).toBeNull() + expect(buttonRef.current).toBeNull() + }) + + it('preserves the original overlay callback object ref and single argument', () => { + const anchorRef = createRef() + const renderAnchor = vi.fn((props: React.HTMLAttributes) => ( + + )) + render( + + Contents + , + ) + expect(renderAnchor.mock.calls[0]).toHaveLength(1) + expect(renderAnchor.mock.calls[0][0]).toHaveProperty('ref', anchorRef) + expect(anchorRef.current).toBe(page.getByRole('button', {name: 'Owner', exact: true}).element()) + }) + + it('preserves configured anchors and legacy Tooltip slot recognition', async () => { + render( + + + Owner + + + + Assign me + + + , + ) + await userEvent.click(page.getByRole('button', {name: 'Owner', exact: true})) + await expect.element(page.getByRole('menuitem', {name: 'Assign me', exact: true})).toHaveFocus() + }) + + it.each([false, true])('rejects a configured anchor together with renderAnchor (tooltip: %s)', withTooltip => { + withExpectedConsoleError(() => { + expect(() => + render( + Owner}> + {withTooltip ? ( + + Another anchor + + ) : ( + + + + )} + Contents + , + ), + ).toThrow('Pass either `renderAnchor` or `ActionMenu.Button`/`ActionMenu.Anchor`, not both.') + }) + }) +}) diff --git a/packages/react/src/experimental/TriggerComposition/TriggerComposition.tsx b/packages/react/src/experimental/TriggerComposition/TriggerComposition.tsx new file mode 100644 index 00000000000..5c4d48f9e7e --- /dev/null +++ b/packages/react/src/experimental/TriggerComposition/TriggerComposition.tsx @@ -0,0 +1,88 @@ +import React from 'react' +import {ActionMenu as StableActionMenu, ActionMenuImpl} from '../../ActionMenu/ActionMenu' +import type {ActionMenuProps as StableActionMenuProps} from '../../ActionMenu/ActionMenu' +import {AnchoredOverlay as StableAnchoredOverlay} from '../../AnchoredOverlay' +import type {AnchoredOverlayProps as StableAnchoredOverlayProps} from '../../AnchoredOverlay' +import {Tooltip as StableTooltip, TooltipImpl} from '../../TooltipV2/Tooltip' +import type {TooltipProps as StableTooltipProps} from '../../TooltipV2/Tooltip' +import {useProvidedRefOrCreate} from '../../hooks/useProvidedRefOrCreate' +import {RenderAnchor, type AnchorRenderer} from '../../internal/components/RenderAnchor' +import {mergeAnchorProps} from '../../internal/utils/mergeAnchorProps' +import {invariant} from '../../utils/invariant' +import {asSlot} from '../../utils/as-slot' +import type {SlotMarker} from '../../utils/types' + +export type ActionMenuProps = StableActionMenuProps & {renderAnchor?: AnchorRenderer} + +function Menu({renderAnchor, ...props}: ActionMenuProps) { + return ( + : undefined + } + /> + ) +} + +export const ActionMenu = Object.assign(Menu, { + Button: StableActionMenu.Button, + Anchor: StableActionMenu.Anchor, + Overlay: StableActionMenu.Overlay, + Divider: StableActionMenu.Divider, +}) + +export type AnchoredOverlayProps = + | StableAnchoredOverlayProps + | (Omit & {renderAnchor: React.ReactElement}) + +export function AnchoredOverlay(props: React.PropsWithChildren) { + const anchorRef = useProvidedRefOrCreate(props.anchorRef) + if (props.renderAnchor === null) { + return + } + if (typeof props.renderAnchor === 'function') { + return + } + const {renderAnchor} = props + return ( + } + /> + ) +} + +export type TooltipProps = StableTooltipProps & {renderAnchor?: AnchorRenderer} + +export const Tooltip: React.ForwardRefExoticComponent< + React.PropsWithoutRef & React.RefAttributes +> & + SlotMarker = React.forwardRef( + ( + {renderAnchor, children, text, type, direction, delay, keybindingHint, _privateDisableTooltip, ...anchorProps}, + ref, + ) => { + invariant(!renderAnchor || children == null, 'Pass either `children` or `renderAnchor` to `Tooltip`, not both.') + const tooltipProps = {text, type, direction, delay, keybindingHint, _privateDisableTooltip} + if (!renderAnchor) { + return ( + + {children} + + ) + } + return ( + ( + + )} + /> + ) + }, +) +Tooltip.displayName = 'Tooltip' +asSlot(Tooltip, StableTooltip) diff --git a/packages/react/src/experimental/TriggerComposition/TriggerComposition.types.test.tsx b/packages/react/src/experimental/TriggerComposition/TriggerComposition.types.test.tsx new file mode 100644 index 00000000000..d3189465607 --- /dev/null +++ b/packages/react/src/experimental/TriggerComposition/TriggerComposition.types.test.tsx @@ -0,0 +1,36 @@ +import {InfoIcon} from '@primer/octicons-react' +import {ActionMenu, AnchoredOverlay, Tooltip} from './TriggerComposition' +import {ActionMenu as StableActionMenu} from '../../ActionMenu' +import {Tooltip as StableTooltip} from '../../TooltipV2' +import {Button, IconButton} from '../../Button' + +export function elementComposition() { + return ( + Owner} />}> + Contents + + ) +} + +export function directOverlayComposition() { + return ( + } /> + } + > + Contents + + ) +} + +export function stableMenuContract() { + // @ts-expect-error renderAnchor is an experimental ActionMenu API + return Owner}>Contents +} + +export function stableTooltipContract() { + // @ts-expect-error renderAnchor is an experimental Tooltip API + return Owner} /> +} diff --git a/packages/react/src/experimental/index.ts b/packages/react/src/experimental/index.ts index 76298f3e514..32b9116abbe 100644 --- a/packages/react/src/experimental/index.ts +++ b/packages/react/src/experimental/index.ts @@ -83,8 +83,8 @@ export type { NavListDividerProps, } from '../NavList' export * from './SelectPanel2' -export {Tooltip} from '../TooltipV2' -export type {TooltipProps} from '../TooltipV2' +export {Tooltip, ActionMenu, AnchoredOverlay} from './TriggerComposition/TriggerComposition' +export type {TooltipProps, ActionMenuProps, AnchoredOverlayProps} from './TriggerComposition/TriggerComposition' export * from '../ActionBar' export {ScrollableRegion} from '../ScrollableRegion' diff --git a/packages/react/src/index.ts b/packages/react/src/index.ts index d1a4a4d4b09..48750d9e583 100644 --- a/packages/react/src/index.ts +++ b/packages/react/src/index.ts @@ -85,14 +85,9 @@ export type { ActionListTrailingVisualProps, } from './ActionList' export {ActionMenu} from './ActionMenu' -export type { - ActionMenuProps, - ActionMenuAnchorProps, - ActionMenuButtonProps, - ActionMenuAnchorRenderProps, -} from './ActionMenu' +export type {ActionMenuProps, ActionMenuAnchorProps, ActionMenuButtonProps} from './ActionMenu' export {AnchoredOverlay} from './AnchoredOverlay' -export type {AnchoredOverlayProps, AnchoredOverlayAnchorContext} from './AnchoredOverlay' +export type {AnchoredOverlayProps} from './AnchoredOverlay' export {default as Autocomplete} from './Autocomplete' export type {AutocompleteMenuProps, AutocompleteInputProps, AutocompleteOverlayProps} from './Autocomplete' export {default as Avatar} from './Avatar' @@ -214,7 +209,7 @@ export type { export {default as Token, IssueLabelToken} from './Token' export type {TokenProps, IssueLabelTokenProps} from './Token' export {Tooltip} from './TooltipV2' -export type {TooltipProps, TooltipAnchorProps} from './TooltipV2' +export type {TooltipProps} from './TooltipV2' export {default as Truncate} from './Truncate' export type {TruncateProps} from './Truncate' diff --git a/packages/react/src/internal/components/RenderAnchor.tsx b/packages/react/src/internal/components/RenderAnchor.tsx new file mode 100644 index 00000000000..72e1e9016c0 --- /dev/null +++ b/packages/react/src/internal/components/RenderAnchor.tsx @@ -0,0 +1,42 @@ +import React from 'react' +import {useMergedRefs} from '../../hooks/useMergedRefs' +import {reactMajorVersion} from '../../utils/environment' +import {invariant} from '../../utils/invariant' +import {mergeAnchorProps, type AnchorProps} from '../utils/mergeAnchorProps' + +export type AnchorRenderProps = React.HTMLAttributes & {ref: React.RefCallback} +export type AnchorRenderer = React.ReactElement | ((props: AnchorRenderProps) => React.ReactElement) + +export function RenderAnchor({render, props}: {render: AnchorRenderer; props: AnchorProps}) { + const element = typeof render === 'function' ? undefined : render + invariant( + typeof render === 'function' || (React.isValidElement(element) && element.type !== React.Fragment), + '`renderAnchor` must render a single interactive element, not a React Fragment.', + ) + const elementRef = + reactMajorVersion > 18 ? element?.props.ref : (element as {ref?: AnchorProps['ref']} | undefined)?.ref + const ref = useMergedRefs(props.ref, elementRef) + if (typeof render === 'function') return render({...props, ref}) + + const merged = mergeAnchorProps(props, render.props) + return React.cloneElement(render, { + ...merged, + onClick: preventDisabledActivation(merged.onClick), + onKeyDown: preventDisabledActivation(merged.onKeyDown), + ref, + }) +} + +function preventDisabledActivation>(handler?: (event: E) => void) { + return (event: E) => { + const isActivation = !('key' in event) || ['Enter', ' ', 'ArrowDown', 'ArrowUp'].some(key => event.key === key) + if ( + isActivation && + (event.currentTarget.matches(':disabled') || event.currentTarget.getAttribute('aria-disabled') === 'true') + ) { + event.preventDefault() + return + } + handler?.(event) + } +} diff --git a/packages/react/src/internal/utils/mergeAnchorProps.ts b/packages/react/src/internal/utils/mergeAnchorProps.ts new file mode 100644 index 00000000000..f57c5723585 --- /dev/null +++ b/packages/react/src/internal/utils/mergeAnchorProps.ts @@ -0,0 +1,22 @@ +import type React from 'react' +import {clsx} from 'clsx' +import {mergeProps} from '../../utils/mergeProps' + +export type AnchorProps = React.HTMLAttributes & { + ref?: React.RefCallback | {readonly current: HTMLElement | null} | null +} + +export function mergeAnchorProps(props: AnchorProps, elementProps: AnchorProps): AnchorProps { + const labelledBy = mergeIds(elementProps['aria-labelledby'], props['aria-labelledby']) + const describedBy = mergeIds(elementProps['aria-describedby'], props['aria-describedby']) + return { + ...mergeProps(props, elementProps, {eventHandlerOrder: 'consumer-first'}), + className: clsx(elementProps.className, props.className) || undefined, + ...(labelledBy ? {'aria-labelledby': labelledBy} : {}), + ...(describedBy ? {'aria-describedby': describedBy} : {}), + } +} + +function mergeIds(first?: string, second?: string) { + return [...new Set(`${first ?? ''} ${second ?? ''}`.split(/\s+/).filter(Boolean))].join(' ') || undefined +} diff --git a/packages/react/src/utils/mergeProps.ts b/packages/react/src/utils/mergeProps.ts index a7d6df5de9b..df18b5c55d0 100644 --- a/packages/react/src/utils/mergeProps.ts +++ b/packages/react/src/utils/mergeProps.ts @@ -13,7 +13,11 @@ type ClassValue = Parameters[number] * * In other cases, if a key exists in both `A` and `B` then the value in `B` will be used in the final result */ -function mergeProps(a: A, b: B): Merge { +function mergeProps( + a: A, + b: B, + options?: {eventHandlerOrder: 'consumer-first'}, +): Merge { const merged = {...a} as Record for (const [key, value] of Object.entries(b)) { @@ -24,7 +28,10 @@ function mergeProps(a: A, b: B): Merge Date: Fri, 13 Nov 2026 16:00:16 -0500 Subject: [PATCH 4/9] Colocate experimental trigger composition documentation Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../TriggerComposition/README.mdx | 18 +++------ .../TriggerComposition/Tooltip.mdx | 37 +++++++++++++++++++ 2 files changed, 43 insertions(+), 12 deletions(-) rename contributor-docs/extending-components.md => packages/react/src/experimental/TriggerComposition/README.mdx (90%) create mode 100644 packages/react/src/experimental/TriggerComposition/Tooltip.mdx diff --git a/contributor-docs/extending-components.md b/packages/react/src/experimental/TriggerComposition/README.mdx similarity index 90% rename from contributor-docs/extending-components.md rename to packages/react/src/experimental/TriggerComposition/README.mdx index ca76626f62f..a1dba92f1ce 100644 --- a/contributor-docs/extending-components.md +++ b/packages/react/src/experimental/TriggerComposition/README.mdx @@ -1,6 +1,8 @@ -# Extending components +import {Meta} from '@storybook/addon-docs/blocks' -## Experimental trigger composition + + +# Experimental trigger composition The `@primer/react/experimental` versions of `ActionMenu`, `AnchoredOverlay`, and `Tooltip` accept an element in `renderAnchor`. Like @@ -17,7 +19,6 @@ usage remains supported. `SelectPanel` is not part of this experiment. ```tsx import {ActionList, Button} from '@primer/react' import {ActionMenu, Tooltip} from '@primer/react/experimental' - ; event.stopPropagation()}>Owner} /> @@ -46,7 +47,6 @@ prop-merging contract. import {IconButton} from '@primer/react' import {AnchoredOverlay, Tooltip} from '@primer/react/experimental' import {InfoIcon} from '@primer/octicons-react' - ; setOpen(true)} @@ -85,11 +85,8 @@ Two deliberate differences from Base UI apply: Primer uses the existing them. Element-first event ordering is an intentional exception to Primer's usual component-first prop-merging convention for this experiment. -In the experimental Tooltip's **render mode**, HTML attributes such as `id`, -`className`, `style`, ARIA attributes, and handlers configure its **trigger**, not -its tooltip bubble. `text`, `type`, `direction`, `delay`, and `keybindingHint` -still configure the tooltip. The existing children mode retains its original -attribute routing. +See the [experimental Tooltip documentation](?path=/docs/experimental-components-tooltip--docs) +for tooltip attribute routing and accessible naming. ### Responsibilities and escape hatches @@ -97,9 +94,6 @@ attribute routing. either Tooltip children or `renderAnchor`. Never nest native buttons. - Custom render elements must forward their ref and received trigger props to the interactive DOM element, not a wrapper or imperative handle. -- Use a description tooltip for a button that already has an accessible name; - use a label tooltip for an otherwise unnamed trigger. Existing description IDs - on the button are preserved automatically in element mode. - Disabled behavior is not a generic boolean merge. Configure the actual rendered control using its normal `disabled` or `inactive` API. Native disabled controls cannot open the overlay or tooltip. `aria-disabled` controls cannot diff --git a/packages/react/src/experimental/TriggerComposition/Tooltip.mdx b/packages/react/src/experimental/TriggerComposition/Tooltip.mdx new file mode 100644 index 00000000000..1e56c751229 --- /dev/null +++ b/packages/react/src/experimental/TriggerComposition/Tooltip.mdx @@ -0,0 +1,37 @@ +import {Meta} from '@storybook/addon-docs/blocks' + + + +# Tooltip + +Import `Tooltip` from `@primer/react/experimental` to compose tooltip behavior +with another trigger using an element-valued `renderAnchor`. The stable export +and the existing children API remain unchanged. + +```tsx +import {Button} from '@primer/react' +import {Tooltip} from '@primer/react/experimental' + +;Run action} /> +``` + +See [trigger composition](?path=/docs/experimental-trigger-composition--docs) +for menu and overlay examples, merged refs, handler ordering, ARIA ID merging, +and callback escape hatches. + +## Attribute routing + +In **render mode**, HTML attributes such as `id`, `className`, `style`, `popover`, +ARIA attributes, and handlers configure the **trigger**, not the tooltip bubble. +`text`, `type`, `direction`, `delay`, and `keybindingHint` still configure the +tooltip. The existing children mode retains its original bubble attribute routing. +Bubble attribute overrides cannot be migrated unchanged to render mode. + +Provide either children or `renderAnchor`, not both. Custom render elements +must forward received props and their ref to the interactive DOM element. + +## Accessible naming + +Use a description tooltip for a button that already has an accessible name; +use a label tooltip for an otherwise unnamed trigger. Existing description IDs +on the button are preserved automatically in element mode. From 258b2fc4262f8f02c412d377dd4c126f0b9cf31a Mon Sep 17 00:00:00 2001 From: Adam Dierkens Date: Fri, 13 Nov 2026 16:02:25 -0500 Subject: [PATCH 5/9] Use supported MDX formatting for composition contract Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../TriggerComposition/README.mdx | 30 +++++++++++-------- 1 file changed, 17 insertions(+), 13 deletions(-) diff --git a/packages/react/src/experimental/TriggerComposition/README.mdx b/packages/react/src/experimental/TriggerComposition/README.mdx index a1dba92f1ce..f879920c8f8 100644 --- a/packages/react/src/experimental/TriggerComposition/README.mdx +++ b/packages/react/src/experimental/TriggerComposition/README.mdx @@ -14,7 +14,7 @@ The stable exports remain unchanged. The experimental `Tooltip` also retains its existing children API, and configured `ActionMenu.Button`/`ActionMenu.Anchor` usage remains supported. `SelectPanel` is not part of this experiment. -### Menu and tooltip +## Menu and tooltip ```tsx import {ActionList, Button} from '@primer/react' @@ -41,7 +41,7 @@ DOM nodes: element rendering removes the need for the menu to recognize specific tooltip/anchor nesting patterns, and gives composed behaviors a consistent prop-merging contract. -### Direct overlay and tooltip +## Direct overlay and tooltip ```tsx import {IconButton} from '@primer/react' @@ -68,16 +68,20 @@ handling, and focus return. No broad object ref needs to be spread into a more narrowly typed button ref. This does not introduce an independent focus-return target or change overlay state management. -### Element merging contract - -| Prop | Behavior | -| ------------------------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------- | -| `ref` | All behavior, forwarded, and render-element refs receive the same DOM node; React 19 cleanup functions are preserved. | -| Event handlers | The render element's handler runs first, followed by supplied handlers unless `defaultPrevented`. `stopPropagation()` does not cancel the internal handler. | -| `className` | Both values are combined, render element first. | -| `style` | Shallow merge; the render element's values win. | -| `aria-labelledby`, `aria-describedby` | ID lists are combined, render element first, removing duplicates. An omitted or undefined relationship does not erase another behavior's IDs. | -| Other attributes | The render element's value wins, including `id`. Menu labelling follows the actual anchor's ID or label relationship. | +## Element merging contract + +- **Refs:** All behavior, forwarded, and render-element refs receive the same DOM + node; React 19 cleanup functions are preserved. +- **Event handlers:** The render element's handler runs first, followed by + supplied handlers unless `defaultPrevented`. `stopPropagation()` does not + cancel the internal handler. +- **Classes:** Both `className` values are combined, render element first. +- **Styles:** Shallow merge; the render element's values win. +- **ARIA relationships:** `aria-labelledby` and `aria-describedby` ID lists are + combined, render element first, removing duplicates. An omitted or undefined + relationship does not erase another behavior's IDs. +- **Other attributes:** The render element's value wins, including `id`. Menu + labelling follows the actual anchor's ID or label relationship. Two deliberate differences from Base UI apply: Primer uses the existing `preventDefault()` cancellation convention rather than adding @@ -88,7 +92,7 @@ usual component-first prop-merging convention for this experiment. See the [experimental Tooltip documentation](?path=/docs/experimental-components-tooltip--docs) for tooltip attribute routing and accessible naming. -### Responsibilities and escape hatches +## Responsibilities and escape hatches - Use `renderAnchor` or a configured menu anchor, not both. Similarly, provide either Tooltip children or `renderAnchor`. Never nest native buttons. From a7c92fe05312798128bdbb618e98d376ee709985 Mon Sep 17 00:00:00 2001 From: Adam Dierkens Date: Fri, 13 Nov 2026 16:10:03 -0500 Subject: [PATCH 6/9] Keep stable Tooltip suppression out of experimental trigger props Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../experimental/TriggerComposition/TriggerComposition.tsx | 2 +- .../TriggerComposition/TriggerComposition.types.test.tsx | 5 +++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/packages/react/src/experimental/TriggerComposition/TriggerComposition.tsx b/packages/react/src/experimental/TriggerComposition/TriggerComposition.tsx index 5c4d48f9e7e..2a2ada59f27 100644 --- a/packages/react/src/experimental/TriggerComposition/TriggerComposition.tsx +++ b/packages/react/src/experimental/TriggerComposition/TriggerComposition.tsx @@ -54,7 +54,7 @@ export function AnchoredOverlay(props: React.PropsWithChildren & {renderAnchor?: AnchorRenderer} export const Tooltip: React.ForwardRefExoticComponent< React.PropsWithoutRef & React.RefAttributes diff --git a/packages/react/src/experimental/TriggerComposition/TriggerComposition.types.test.tsx b/packages/react/src/experimental/TriggerComposition/TriggerComposition.types.test.tsx index d3189465607..f9acb63693f 100644 --- a/packages/react/src/experimental/TriggerComposition/TriggerComposition.types.test.tsx +++ b/packages/react/src/experimental/TriggerComposition/TriggerComposition.types.test.tsx @@ -34,3 +34,8 @@ export function stableTooltipContract() { // @ts-expect-error renderAnchor is an experimental Tooltip API return Owner} /> } + +export function experimentalTooltipSuppressionContract() { + // @ts-expect-error stable tooltip suppression is not an experimental trigger prop + return Owner} /> +} From 515add0d06954240af65c1a065e7499961e5fbb7 Mon Sep 17 00:00:00 2001 From: Adam Dierkens Date: Fri, 13 Nov 2026 17:07:56 -0500 Subject: [PATCH 7/9] Add experimental Tooltip routing and visibility controls Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .changeset/experimental-tooltip-controls.md | 5 + packages/react/src/TooltipV2/Tooltip.tsx | 150 +++++++++-- .../__snapshots__/exports.test.ts.snap | 1 + .../TriggerComposition/README.mdx | 7 +- .../Tooltip.examples.stories.tsx | 56 +++++ .../Tooltip.features.stories.tsx | 67 +++++ .../TriggerComposition/Tooltip.mdx | 79 +++++- .../TriggerComposition/Tooltip.stories.tsx | 19 ++ .../TriggerComposition.test.tsx | 232 +++++++++++++++++- .../TriggerComposition/TriggerComposition.tsx | 71 +++++- .../TriggerComposition.types.test.tsx | 20 ++ packages/react/src/experimental/index.ts | 7 +- 12 files changed, 668 insertions(+), 46 deletions(-) create mode 100644 .changeset/experimental-tooltip-controls.md create mode 100644 packages/react/src/experimental/TriggerComposition/Tooltip.examples.stories.tsx create mode 100644 packages/react/src/experimental/TriggerComposition/Tooltip.features.stories.tsx create mode 100644 packages/react/src/experimental/TriggerComposition/Tooltip.stories.tsx diff --git a/.changeset/experimental-tooltip-controls.md b/.changeset/experimental-tooltip-controls.md new file mode 100644 index 00000000000..146a7ba366b --- /dev/null +++ b/.changeset/experimental-tooltip-controls.md @@ -0,0 +1,5 @@ +--- +'@primer/react': minor +--- + +Tooltip: Add experimental controlled and uncontrolled visibility with separate popup attributes. HTML attributes on the experimental component consistently configure its trigger. diff --git a/packages/react/src/TooltipV2/Tooltip.tsx b/packages/react/src/TooltipV2/Tooltip.tsx index c1a2b0e063c..00f461ae010 100644 --- a/packages/react/src/TooltipV2/Tooltip.tsx +++ b/packages/react/src/TooltipV2/Tooltip.tsx @@ -16,6 +16,7 @@ import type {SlotMarker} from '../utils/types' import {TooltipContext} from './TooltipContext' import {reactMajorVersion} from '../utils/environment' import type {AnchorRenderProps} from '../internal/components/RenderAnchor' +import {useEffectCallback} from '../internal/hooks/useEffectCallback' export type TooltipDirection = 'nw' | 'n' | 'ne' | 'e' | 'se' | 's' | 'sw' | 'w' export type TooltipProps = React.PropsWithChildren<{ @@ -93,7 +94,20 @@ const isInteractive = (element: HTMLElement) => { } const emptyKeybindingHints: Array = [] -type TooltipImplProps = TooltipProps & {renderAnchor?: (props: AnchorRenderProps) => React.ReactElement} +export type TooltipOpenChangeDetails = { + reason: 'trigger-hover' | 'trigger-focus' | 'trigger-press' | 'escape-key' | 'outside-press' | 'disabled' | 'none' + event?: Event | React.SyntheticEvent +} + +type TooltipImplProps = TooltipProps & { + popover?: 'auto' | 'manual' | '' + renderAnchor?: (props: AnchorRenderProps) => React.ReactElement + visibility?: { + open: boolean + controlled: boolean + onOpenChange: (open: boolean, details: TooltipOpenChangeDetails) => void + } +} export const TooltipImpl: ForwardRefExoticComponent< React.PropsWithoutRef & React.RefAttributes @@ -106,6 +120,8 @@ export const TooltipImpl: ForwardRefExoticComponent< type = 'description', children, renderAnchor, + visibility, + popover, id, className, keybindingHint = emptyKeybindingHints, @@ -115,7 +131,9 @@ export const TooltipImpl: ForwardRefExoticComponent< }: TooltipImplProps, forwardedRef, ) => { - const tooltipId = useId(id) + const hasAriaLabel = 'aria-label' in rest + const keybindingHints = Array.isArray(keybindingHint) ? keybindingHint : [keybindingHint] + const tooltipId = useId(visibility && keybindingHints.length > 0 && !hasAriaLabel ? undefined : id) const hasRenderAnchor = renderAnchor !== undefined invariant(!renderAnchor || children == null, 'Pass either `children` or `renderAnchor` to `Tooltip`, not both.') const child = renderAnchor ? undefined : Children.only(children) @@ -144,8 +162,9 @@ export const TooltipImpl: ForwardRefExoticComponent< const openTimeoutRef = React.useRef(null) const {safeSetTimeout, safeClearTimeout} = useSafeTimeout() + const tooltipPopover = visibility ? (visibility.controlled ? 'manual' : (popover ?? 'auto')) : 'auto' - const openTooltip = () => { + const showPopover = useEffectCallback(() => { try { if ( tooltipElRef.current && @@ -188,12 +207,15 @@ export const TooltipImpl: ForwardRefExoticComponent< throw error } } - } - const closeTooltip = () => { - if (openTimeoutRef.current) { + }) + const clearOpenTimeout = useEffectCallback(() => { + if (openTimeoutRef.current !== null) { safeClearTimeout(openTimeoutRef.current) openTimeoutRef.current = null } + }) + const hidePopover = useEffectCallback(() => { + clearOpenTimeout() try { if ( tooltipElRef.current && @@ -221,7 +243,24 @@ export const TooltipImpl: ForwardRefExoticComponent< throw error } } - } + }) + const openTooltip = useEffectCallback((details: TooltipOpenChangeDetails = {reason: 'trigger-hover'}) => { + if (visibility) { + if (!_privateDisableTooltip && !readTriggerRef.current?.matches(':disabled')) { + visibility.onOpenChange(true, details) + } + } else { + showPopover() + } + }) + const closeTooltip = useEffectCallback((details: TooltipOpenChangeDetails = {reason: 'trigger-hover'}) => { + clearOpenTimeout() + if (visibility) { + visibility.onOpenChange(false, details) + } else { + hidePopover() + } + }) // context value const value = useMemo(() => ({tooltipId}), [tooltipId]) @@ -278,8 +317,54 @@ export const TooltipImpl: ForwardRefExoticComponent< } const tooltip = tooltipElRef.current - tooltip.setAttribute('popover', 'auto') - }, [tooltipElRef, readTriggerRef, direction, type, hasRenderAnchor]) + tooltip.setAttribute('popover', tooltipPopover) + }, [tooltipElRef, readTriggerRef, direction, type, hasRenderAnchor, tooltipPopover]) + + const synchronizeVisibility = useEffectCallback(() => { + if (!visibility) return + if (visibility.open && !_privateDisableTooltip) { + showPopover() + } else { + hidePopover() + } + if (_privateDisableTooltip) closeTooltip({reason: 'disabled'}) + }) + useEffect(() => { + // Synchronize React-owned visibility with the native Popover API. + synchronizeVisibility() + }, [visibility?.open, _privateDisableTooltip, synchronizeVisibility]) + + const onNativeToggle = useEffectCallback((event: Event) => { + const tooltip = tooltipElRef.current + if (!tooltip || !visibility || visibility.controlled) return + if (!tooltip.matches(':popover-open') && visibility.open) { + clearOpenTimeout() + visibility.onOpenChange(false, {reason: 'none', event}) + } + }) + useEffect(() => { + const tooltip = tooltipElRef.current + if (!visibility || !tooltip) return + tooltip.addEventListener('toggle', onNativeToggle) + return () => tooltip.removeEventListener('toggle', onNativeToggle) + }, [visibility, onNativeToggle]) + + const onOutsidePress = useEffectCallback((event: PointerEvent) => { + if ( + visibility?.controlled && + tooltipElRef.current?.matches(':popover-open') && + event.target instanceof Node && + !readTriggerRef.current?.contains(event.target) && + !tooltipElRef.current.contains(event.target) + ) { + closeTooltip({reason: 'outside-press', event}) + } + }) + useEffect(() => { + if (!visibility?.controlled) return + document.addEventListener('pointerdown', onOutsidePress) + return () => document.removeEventListener('pointerdown', onOutsidePress) + }, [visibility?.controlled, onOutsidePress]) useOnEscapePress( (event: KeyboardEvent) => { @@ -288,17 +373,13 @@ export const TooltipImpl: ForwardRefExoticComponent< event.stopImmediatePropagation() event.preventDefault() } - closeTooltip() + closeTooltip({reason: 'escape-key', event}) } }, [isPopoverOpen, _privateDisableTooltip], ) 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 @@ -310,7 +391,7 @@ export const TooltipImpl: ForwardRefExoticComponent< : {}), ...(type === 'label' ? {'aria-labelledby': tooltipId} : {}), onBlur: (event: React.FocusEvent) => { - closeTooltip() + closeTooltip({reason: 'trigger-focus', event}) elementChild?.props.onBlur?.(event) }, onTouchEnd: (event: React.TouchEvent) => { @@ -318,7 +399,7 @@ export const TooltipImpl: ForwardRefExoticComponent< // Hide tooltips on tap to essentially disable them on touch devices; // this still allows viewing the tooltip on tap-and-hold - safeSetTimeout(() => closeTooltip(), 10) + safeSetTimeout(() => closeTooltip({reason: 'trigger-press', event}), 10) }, onFocus: (event: React.FocusEvent) => { // only show tooltip on :focus-visible, not on :focus @@ -329,10 +410,12 @@ export const TooltipImpl: ForwardRefExoticComponent< // https://github.com/jsdom/jsdom/issues/3426 } - openTooltip() + openTooltip({reason: 'trigger-focus', event}) elementChild?.props.onFocus?.(event) }, onMouseOverCapture: (event: React.MouseEvent) => { + if (visibility && _privateDisableTooltip) return + clearOpenTimeout() 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`) @@ -340,13 +423,18 @@ export const TooltipImpl: ForwardRefExoticComponent< // (prevent showing tooltip when mouse is just passing through) openTimeoutRef.current = safeSetTimeout(() => { // if the mouse is already moved out, do not show the tooltip - if (!openTimeoutRef.current) return - openTooltip() + if (openTimeoutRef.current === null) return + openTimeoutRef.current = null + openTooltip({reason: 'trigger-hover', event}) elementChild?.props.onMouseEnter?.(event) }, delayTime) }, onMouseLeave: (event: React.MouseEvent) => { - closeTooltip() + if ( + !(visibility && event.relatedTarget instanceof Node && tooltipElRef.current?.contains(event.relatedTarget)) + ) { + closeTooltip({reason: 'trigger-hover', event}) + } elementChild?.props.onMouseLeave?.(event) }, } @@ -364,15 +452,31 @@ export const TooltipImpl: ForwardRefExoticComponent< data-direction={calculatedDirection} data-component="Tooltip" {...rest} + {...{popover: visibility ? tooltipPopover : popover}} // Only need tooltip role if the tooltip is a description for supplementary information role={type === 'description' ? 'tooltip' : undefined} // stop AT from announcing the tooltip twice: when it is a label type it will be announced with "aria-labelledby",when it is a description type it will be announced with "aria-describedby" aria-hidden={true} // mouse leave and enter on the tooltip itself is needed to keep the tooltip open when the mouse is over the tooltip - onMouseEnter={openTooltip} - onMouseLeave={closeTooltip} + onMouseEnter={event => { + if (visibility) rest.onMouseEnter?.(event) + if (!event.defaultPrevented) openTooltip({reason: 'trigger-hover', event}) + }} + onMouseLeave={event => { + if (visibility) rest.onMouseLeave?.(event) + if ( + !event.defaultPrevented && + !( + visibility && + event.relatedTarget instanceof Node && + readTriggerRef.current?.contains(event.relatedTarget) + ) + ) { + closeTooltip({reason: 'trigger-hover', event}) + } + }} // If there is an aria-label prop, always assign the ID to the parent so the accessible label can be overridden - id={hasAriaLabel || keybindingHints.length === 0 ? tooltipId : undefined} + id={hasAriaLabel || keybindingHints.length === 0 ? tooltipId : visibility ? id : undefined} > {keybindingHints.length > 0 ? ( <> diff --git a/packages/react/src/__tests__/__snapshots__/exports.test.ts.snap b/packages/react/src/__tests__/__snapshots__/exports.test.ts.snap index 3fb4af36182..d08150ad1e5 100644 --- a/packages/react/src/__tests__/__snapshots__/exports.test.ts.snap +++ b/packages/react/src/__tests__/__snapshots__/exports.test.ts.snap @@ -411,6 +411,7 @@ exports[`@primer/react/experimental > should not update exports without a semver "type TabsProps", "type TitleProps", "Tooltip", + "type TooltipOpenChangeDetails", "type TooltipProps", "TopicTag", "type TopicTagGroupProps", diff --git a/packages/react/src/experimental/TriggerComposition/README.mdx b/packages/react/src/experimental/TriggerComposition/README.mdx index f879920c8f8..d87661051db 100644 --- a/packages/react/src/experimental/TriggerComposition/README.mdx +++ b/packages/react/src/experimental/TriggerComposition/README.mdx @@ -10,8 +10,8 @@ The `@primer/react/experimental` versions of `ActionMenu`, `AnchoredOverlay`, an composes behaviors onto one interactive element, automatically merging props and refs. It does not add a DOM wrapper or require separate Root/Trigger components. -The stable exports remain unchanged. The experimental `Tooltip` also retains its -existing children API, and configured `ActionMenu.Button`/`ActionMenu.Anchor` +The stable exports remain unchanged. The experimental `Tooltip` also accepts +children, with trigger-directed HTML props and separate `popupProps`, and configured `ActionMenu.Button`/`ActionMenu.Anchor` usage remains supported. `SelectPanel` is not part of this experiment. ## Menu and tooltip @@ -102,7 +102,8 @@ for tooltip attribute routing and accessible naming. rendered control using its normal `disabled` or `inactive` API. Native disabled controls cannot open the overlay or tooltip. `aria-disabled` controls cannot activate the overlay but remain focusable for an explanatory tooltip; Tab is - not cancelled. No new root-level disabled state is introduced. + not cancelled. The experimental Tooltip's root API does not expose a `disabled` + prop. - Manage controlled open state normally. Composing triggers does not arbitrate between independently controlled menus, dialogs, or overlays. diff --git a/packages/react/src/experimental/TriggerComposition/Tooltip.examples.stories.tsx b/packages/react/src/experimental/TriggerComposition/Tooltip.examples.stories.tsx new file mode 100644 index 00000000000..34dfe1b4e0a --- /dev/null +++ b/packages/react/src/experimental/TriggerComposition/Tooltip.examples.stories.tsx @@ -0,0 +1,56 @@ +import {useState} from 'react' +import type {Meta, StoryObj} from '@storybook/react-vite' +import {ActionList} from '../../ActionList' +import {Button} from '../../Button' +import {ActionMenu, Tooltip} from './TriggerComposition' + +const meta = { + title: 'Experimental/Components/Tooltip/Examples', + component: Tooltip, + args: {text: 'Choose an action'}, +} satisfies Meta + +export default meta +type Story = StoryObj + +export const CoordinateWithMenu: Story = { + render: function CoordinateWithMenu() { + const [menuOpen, setMenuOpen] = useState(false) + const [tooltipOpen, setTooltipOpen] = useState(false) + return ( + { + setMenuOpen(next) + if (next) setTooltipOpen(false) + }} + renderAnchor={ + { + if (!menuOpen) setTooltipOpen(next) + }} + popupProps={{id: 'action-tooltip'}} + renderAnchor={} + /> + } + > + + + Edit + Duplicate + + + + ) + }, + parameters: { + docs: { + description: { + story: + 'The application coordinates two visibility states on one composed trigger, without changing tooltip text, calling native popover methods, or merging refs and handlers. For tooltip-only suppression with the stable component, use its disabled prop instead.', + }, + }, + }, +} diff --git a/packages/react/src/experimental/TriggerComposition/Tooltip.features.stories.tsx b/packages/react/src/experimental/TriggerComposition/Tooltip.features.stories.tsx new file mode 100644 index 00000000000..85595200151 --- /dev/null +++ b/packages/react/src/experimental/TriggerComposition/Tooltip.features.stories.tsx @@ -0,0 +1,67 @@ +import {useState} from 'react' +import type {Meta, StoryObj} from '@storybook/react-vite' +import {Button} from '../../Button' +import {Tooltip} from './TriggerComposition' + +const meta = { + title: 'Experimental/Components/Tooltip/Features', + component: Tooltip, + args: { + text: 'Additional context', + renderAnchor: , + }, +} satisfies Meta + +export default meta +type Story = StoryObj + +export const ControlledVisibility: Story = { + render: function ControlledVisibility(args) { + const [open, setOpen] = useState(false) + const [lastRequest, setLastRequest] = useState('None') + return ( + <> + {' '} + {' '} + { + setLastRequest(`${next ? 'Open' : 'Close'}: ${reason}`) + setOpen(next) + }} + /> +

Last interaction request: {lastRequest}

+ + ) + }, + parameters: { + docs: { + description: { + story: + 'Show tooltip and Hide tooltip change visibility without focusing the trigger. Hover, keyboard focus, blur, and Escape request changes through onOpenChange; open remains authoritative. Updating open itself does not emit another request.', + }, + }, + }, +} + +export const SeparatePopupAttributes: Story = { + args: { + id: 'action-trigger', + className: 'action-trigger', + popupProps: { + id: 'action-tooltip', + className: 'action-tooltip', + style: {fontWeight: 'var(--base-text-weight-semibold)'}, + }, + text: 'The trigger and popup each receive their own HTML attributes.', + }, + parameters: { + docs: { + description: { + story: + 'Incoming HTML props configure the trigger. popupProps configures the bubble independently, including its className, id, style, and handlers. The ref still points to the trigger.', + }, + }, + }, +} diff --git a/packages/react/src/experimental/TriggerComposition/Tooltip.mdx b/packages/react/src/experimental/TriggerComposition/Tooltip.mdx index 1e56c751229..40e9bc6e334 100644 --- a/packages/react/src/experimental/TriggerComposition/Tooltip.mdx +++ b/packages/react/src/experimental/TriggerComposition/Tooltip.mdx @@ -6,12 +6,11 @@ import {Meta} from '@storybook/addon-docs/blocks' Import `Tooltip` from `@primer/react/experimental` to compose tooltip behavior with another trigger using an element-valued `renderAnchor`. The stable export -and the existing children API remain unchanged. +is unchanged. ```tsx import {Button} from '@primer/react' import {Tooltip} from '@primer/react/experimental' - ;Run action} /> ``` @@ -21,11 +20,32 @@ and callback escape hatches. ## Attribute routing -In **render mode**, HTML attributes such as `id`, `className`, `style`, `popover`, +HTML attributes such as `id`, `className`, `style`, `popover`, ARIA attributes, and handlers configure the **trigger**, not the tooltip bubble. `text`, `type`, `direction`, `delay`, and `keybindingHint` still configure the -tooltip. The existing children mode retains its original bubble attribute routing. -Bubble attribute overrides cannot be migrated unchanged to render mode. +tooltip. Use `popupProps` for the bubble's HTML attributes in both `renderAnchor` +and children mode: + +```tsx +Run action} +/> +``` + +This follows Base UI's separation between `Tooltip.Trigger` and `Tooltip.Popup`, +flattened into Primer's existing component shape. Base UI uses separate parts, +not a `popupProps` prop. + +When migrating the stable children API, move bubble attributes into `popupProps`. +The experimental children syntax remains supported, but its HTML props now +consistently configure the trigger. Tooltip ARIA relationships, bubble role, +and `aria-hidden` remain managed internally. The forwarded `ref` targets the +trigger. An explicit popup `id` stays on the bubble. With keybinding hints, the +accessible relationship may reference an internal text node instead, avoiding +duplicate key names. Provide either children or `renderAnchor`, not both. Custom render elements must forward received props and their ref to the interactive DOM element. @@ -35,3 +55,52 @@ must forward received props and their ref to the interactive DOM element. Use a description tooltip for a button that already has an accessible name; use a label tooltip for an otherwise unnamed trigger. Existing description IDs on the button are preserved automatically in element mode. + +## Visibility + +The tooltip is uncontrolled by default. `defaultOpen` sets only its initial +visibility. Pass `open` and `onOpenChange` to control visibility: + +```tsx +const [open, setOpen] = useState(false) + + setOpen(next)} + text="Run the action" + renderAnchor={} +/> +``` + +`onOpenChange(next, details)` reports interaction requests, not a notification +that a controlled bubble has changed. Hover requests opening after `delay`; +keyboard focus requests it immediately. Leaving the trigger, blur, touch, +Escape, and outside dismissal request closing. A supplied `open` remains +authoritative: if the owner ignores a request, visibility does not change. +Updating `open` directly and initial `defaultOpen` do not invoke the callback. +Choose controlled or uncontrolled mode for the component's lifetime. + +`details.reason` identifies `trigger-hover`, `trigger-focus`, `trigger-press`, +`escape-key`, `outside-press`, `disabled`, or `none` (native dismissal). +`details.event`, when available, contains the originating event. Accept closing +requests, especially Escape, to keep a controlled tooltip dismissible. + +Controlled tooltips use `popover="manual"` internally so native light dismissal +cannot override `open`; this takes precedence over `popupProps.popover`. +Uncontrolled tooltips default to `popover="auto"` and synchronize native +dismissal with their state. + +The existing `_privateDisableTooltip` compatibility prop takes precedence over +visibility, closes the bubble and cancels pending opens while retaining its +accessible text. In controlled mode it requests closing with reason `disabled`; +if the owner keeps `open={true}`, removing suppression makes that value effective +again. In uncontrolled mode a fresh interaction is required after re-enabling. +This does not disable the trigger. + +The stable Tooltip's public `disabled` API is not added to this experimental +trigger surface. Use the stable export for tooltip-only suppression; use +controlled visibility here when the application must own the visible state. + +See the **Controlled visibility**, **Separate popup attributes**, and +**Coordinate with menu** stories for interactive examples. These state controls +do not add positioning overrides, content portals, or new ARIA naming modes. diff --git a/packages/react/src/experimental/TriggerComposition/Tooltip.stories.tsx b/packages/react/src/experimental/TriggerComposition/Tooltip.stories.tsx new file mode 100644 index 00000000000..16a27a5f145 --- /dev/null +++ b/packages/react/src/experimental/TriggerComposition/Tooltip.stories.tsx @@ -0,0 +1,19 @@ +import type {Meta, StoryObj} from '@storybook/react-vite' +import {Button} from '../../Button' +import {Tooltip} from './TriggerComposition' + +const meta = { + title: 'Experimental/Components/Tooltip', + component: Tooltip, + args: { + text: 'Additional context', + renderAnchor: , + }, +} satisfies Meta + +export default meta +type Story = StoryObj + +export const Default: Story = {} + +export const Playground: Story = {} diff --git a/packages/react/src/experimental/TriggerComposition/TriggerComposition.test.tsx b/packages/react/src/experimental/TriggerComposition/TriggerComposition.test.tsx index c6755a652f1..b871f6dc3d0 100644 --- a/packages/react/src/experimental/TriggerComposition/TriggerComposition.test.tsx +++ b/packages/react/src/experimental/TriggerComposition/TriggerComposition.test.tsx @@ -1,7 +1,7 @@ import type React from 'react' import {createRef, useState} from 'react' import {act, fireEvent, render} from '@testing-library/react' -import {describe, expect, it, vi} from 'vitest' +import {afterEach, describe, expect, it, vi} from 'vitest' import {page, userEvent} from 'vitest/browser' import {InfoIcon} from '@primer/octicons-react' import {ActionMenu, AnchoredOverlay, Tooltip} from './TriggerComposition' @@ -12,6 +12,236 @@ import {FeatureFlags} from '../../FeatureFlags' import {withExpectedConsoleError} from '../../utils/testing' import {reactMajorVersion} from '../../utils/environment' +describe('experimental Tooltip routing and visibility', () => { + afterEach(() => vi.useRealTimers()) + + it.each(['children', 'renderAnchor'] as const)('separates trigger and popup attributes in %s mode', mode => { + const onPopupEnter = vi.fn() + const buttonRef = createRef() + const trigger = ( + + ) + const {getByText} = render( + , + ) + const button = page.getByRole('button', {name: 'Action', exact: true}).element() + const popup = getByText('Additional context') + expect(buttonRef.current).toBe(button) + expect(button).toHaveAttribute('id', 'action-trigger') + expect(button).toHaveClass('trigger-class') + expect(button).not.toHaveClass('popup-class') + expect(button).toHaveStyle({color: 'rgb(0, 0, 255)'}) + expect(button).not.toHaveAttribute('popover') + expect(button).toHaveAccessibleDescription('Additional context') + expect(button).toHaveAttribute('aria-describedby', 'action-tooltip') + expect(popup).toHaveClass('popup-class') + expect(popup).not.toHaveClass('trigger-class') + expect(popup).toHaveStyle({color: 'rgb(255, 0, 0)'}) + expect(popup).toHaveAttribute('popover', 'manual') + expect(popup).toHaveAttribute('aria-hidden', 'true') + fireEvent.mouseEnter(popup) + expect(onPopupEnter).toHaveBeenCalledOnce() + expect(popup.matches(':popover-open')).toBe(true) + }) + + it('honors controlled visibility and treats interactions as requests', () => { + vi.useFakeTimers() + const onOpenChange = vi.fn() + const props = { + text: 'Additional context', + onOpenChange, + delay: 'medium' as const, + renderAnchor: , + } + const {getByText, rerender} = render() + const button = page.getByRole('button', {name: 'Action', exact: true}).element() + const popup = getByText('Additional context') + fireEvent.mouseOver(button) + act(() => vi.advanceTimersByTime(399)) + expect(onOpenChange).not.toHaveBeenCalled() + expect(popup.matches(':popover-open')).toBe(false) + act(() => vi.advanceTimersByTime(1)) + expect(onOpenChange).toHaveBeenLastCalledWith(true, expect.objectContaining({reason: 'trigger-hover'})) + expect(popup.matches(':popover-open')).toBe(false) + + rerender() + expect(popup.matches(':popover-open')).toBe(true) + expect(onOpenChange).toHaveBeenCalledOnce() + fireEvent.focusOut(button) + expect(onOpenChange).toHaveBeenLastCalledWith(false, expect.objectContaining({reason: 'trigger-focus'})) + expect(popup.matches(':popover-open')).toBe(true) + fireEvent.keyDown(document, {key: 'Escape'}) + expect(onOpenChange).toHaveBeenLastCalledWith(false, expect.objectContaining({reason: 'escape-key'})) + expect(popup.matches(':popover-open')).toBe(true) + fireEvent.pointerDown(document.body) + expect(onOpenChange).toHaveBeenLastCalledWith(false, expect.objectContaining({reason: 'outside-press'})) + expect(popup.matches(':popover-open')).toBe(true) + rerender() + expect(popup.matches(':popover-open')).toBe(false) + expect(onOpenChange).toHaveBeenCalledTimes(4) + }) + + it('keeps an explicit popup ID on the bubble without duplicating keybinding labels', () => { + const {container} = render( + } + />, + ) + const button = page.getByRole('button', {name: 'Run (control k)', exact: true}).element() + const popup = container.querySelector('[data-component="Tooltip"]') + expect(popup).toHaveAttribute('id', 'run-tooltip') + expect(button.getAttribute('aria-labelledby')).not.toBe('run-tooltip') + const label = document.getElementById(button.getAttribute('aria-labelledby') ?? '') + expect(label).toHaveTextContent('Run') + expect(popup).toContainElement(label) + }) + + it('opens on keyboard focus only when the owner accepts the request', async () => { + const onOpenChange = vi.fn() + const {getByText} = render( + Action} />, + ) + await userEvent.tab() + expect(onOpenChange).toHaveBeenLastCalledWith(true, expect.objectContaining({reason: 'trigger-focus'})) + expect(getByText('Details').matches(':popover-open')).toBe(false) + }) + + it('keeps the popup open when moving between the trigger and popup', () => { + const onOpenChange = vi.fn() + const {getByText} = render( + Action} />, + ) + const button = page.getByRole('button', {name: 'Action', exact: true}).element() + const popup = getByText('Details') + fireEvent.mouseOut(button, {relatedTarget: popup}) + expect(popup.matches(':popover-open')).toBe(true) + fireEvent.mouseOut(popup, {relatedTarget: button}) + expect(popup.matches(':popover-open')).toBe(true) + expect(onOpenChange).not.toHaveBeenCalled() + fireEvent.mouseOut(button, {relatedTarget: document.body}) + expect(popup.matches(':popover-open')).toBe(false) + expect(onOpenChange).toHaveBeenLastCalledWith(false, expect.objectContaining({reason: 'trigger-hover'})) + }) + + it('supports an uncontrolled initial value, ignores later default changes, and closes with Escape', async () => { + const onOpenChange = vi.fn() + const {getByText, rerender} = render( + Action} />, + ) + const popup = getByText('Details') + expect(popup.matches(':popover-open')).toBe(true) + expect(onOpenChange).not.toHaveBeenCalled() + rerender( + Action} />, + ) + expect(popup.matches(':popover-open')).toBe(true) + await userEvent.keyboard('{Escape}') + expect(onOpenChange).toHaveBeenLastCalledWith(false, expect.objectContaining({reason: 'escape-key'})) + expect(popup.matches(':popover-open')).toBe(false) + }) + + it('synchronizes native light dismissal and can reopen on a fresh hover', async () => { + const onOpenChange = vi.fn() + const {getByText} = render( + Action} />, + ) + const popup = getByText('Details') + act(() => popup.hidePopover()) + await expect.poll(() => onOpenChange.mock.calls.length).toBe(1) + expect(onOpenChange).toHaveBeenLastCalledWith(false, expect.objectContaining({reason: 'none'})) + await userEvent.hover(page.getByRole('button', {name: 'Action', exact: true})) + await expect.poll(() => popup.matches(':popover-open')).toBe(true) + expect(onOpenChange).toHaveBeenLastCalledWith(true, expect.objectContaining({reason: 'trigger-hover'})) + }) + + it.each(['label', 'description'] as const)('private suppression overrides open while retaining the %s', type => { + vi.useFakeTimers() + const onOpenChange = vi.fn() + const props = { + text: 'Details', + type, + open: true, + onOpenChange, + renderAnchor: , + } + const {getByText, rerender} = render() + const popup = getByText('Details') + const button = page.getByRole('button', {name: type === 'label' ? 'Details' : 'Action', exact: true}).element() + expect(popup.matches(':popover-open')).toBe(true) + rerender() + expect(popup.matches(':popover-open')).toBe(false) + expect(onOpenChange).toHaveBeenLastCalledWith(false, expect.objectContaining({reason: 'disabled'})) + onOpenChange.mockClear() + fireEvent.mouseOver(button) + act(() => vi.advanceTimersByTime(1200)) + expect(onOpenChange).not.toHaveBeenCalled() + expect(button).toBeEnabled() + if (type === 'label') { + expect(button).toHaveAccessibleName('Details') + } else { + expect(button).toHaveAccessibleDescription('Details') + } + }) + + it('cancels a pending open on suppression and requires a fresh interaction when re-enabled', () => { + vi.useFakeTimers() + const onOpenChange = vi.fn() + const props = {text: 'Details', onOpenChange, renderAnchor: , delay: 'medium' as const} + const {getByText, rerender} = render() + const popup = getByText('Details') + const button = page.getByRole('button', {name: 'Action', exact: true}).element() + fireEvent.mouseOver(button) + act(() => vi.advanceTimersByTime(399)) + rerender() + act(() => vi.advanceTimersByTime(1)) + expect(onOpenChange).not.toHaveBeenCalled() + rerender() + act(() => vi.advanceTimersByTime(1200)) + expect(popup.matches(':popover-open')).toBe(false) + fireEvent.mouseOver(button) + act(() => vi.advanceTimersByTime(400)) + expect(popup.matches(':popover-open')).toBe(true) + expect(onOpenChange).toHaveBeenCalledOnce() + }) + + it('cancels delayed requests on pointer leave and unmount', () => { + vi.useFakeTimers() + const onOpenChange = vi.fn() + const {unmount} = render( + Action} />, + ) + const button = page.getByRole('button', {name: 'Action', exact: true}).element() + fireEvent.mouseOver(button) + fireEvent.mouseLeave(button) + act(() => vi.advanceTimersByTime(50)) + expect(onOpenChange).not.toHaveBeenCalled() + fireEvent.mouseOver(button) + unmount() + act(() => vi.advanceTimersByTime(50)) + expect(onOpenChange).not.toHaveBeenCalled() + }) +}) + describe('experimental element renderAnchor', () => { it.runIf(reactMajorVersion >= 19)('preserves React 19 cleanup functions through every composed ref', () => { const cleanupTooltip = vi.fn() diff --git a/packages/react/src/experimental/TriggerComposition/TriggerComposition.tsx b/packages/react/src/experimental/TriggerComposition/TriggerComposition.tsx index 2a2ada59f27..02d3c0942c7 100644 --- a/packages/react/src/experimental/TriggerComposition/TriggerComposition.tsx +++ b/packages/react/src/experimental/TriggerComposition/TriggerComposition.tsx @@ -4,10 +4,11 @@ import type {ActionMenuProps as StableActionMenuProps} from '../../ActionMenu/Ac import {AnchoredOverlay as StableAnchoredOverlay} from '../../AnchoredOverlay' import type {AnchoredOverlayProps as StableAnchoredOverlayProps} from '../../AnchoredOverlay' import {Tooltip as StableTooltip, TooltipImpl} from '../../TooltipV2/Tooltip' -import type {TooltipProps as StableTooltipProps} from '../../TooltipV2/Tooltip' +import type {TooltipProps as StableTooltipProps, TooltipOpenChangeDetails} from '../../TooltipV2/Tooltip' +import {useControllableState} from '../../hooks/useControllableState' import {useProvidedRefOrCreate} from '../../hooks/useProvidedRefOrCreate' import {RenderAnchor, type AnchorRenderer} from '../../internal/components/RenderAnchor' -import {mergeAnchorProps} from '../../internal/utils/mergeAnchorProps' +import {mergeAnchorProps, type AnchorProps} from '../../internal/utils/mergeAnchorProps' import {invariant} from '../../utils/invariant' import {asSlot} from '../../utils/as-slot' import type {SlotMarker} from '../../utils/types' @@ -54,31 +55,75 @@ export function AnchoredOverlay(props: React.PropsWithChildren & {renderAnchor?: AnchorRenderer} +export type TooltipProps = Omit & { + renderAnchor?: AnchorRenderer + /** HTML props for the tooltip bubble, separate from the trigger's HTML props. */ + popupProps?: Omit, 'children'> & {popover?: 'auto' | 'manual' | ''} + open?: boolean + defaultOpen?: boolean + onOpenChange?: (open: boolean, details: TooltipOpenChangeDetails) => void +} + +export type {TooltipOpenChangeDetails} export const Tooltip: React.ForwardRefExoticComponent< React.PropsWithoutRef & React.RefAttributes > & SlotMarker = React.forwardRef( ( - {renderAnchor, children, text, type, direction, delay, keybindingHint, _privateDisableTooltip, ...anchorProps}, + { + renderAnchor, + children, + text, + type, + direction, + delay, + keybindingHint, + _privateDisableTooltip, + popupProps, + open: controlledOpen, + defaultOpen = false, + onOpenChange, + ...anchorProps + }, ref, ) => { invariant(!renderAnchor || children == null, 'Pass either `children` or `renderAnchor` to `Tooltip`, not both.') - const tooltipProps = {text, type, direction, delay, keybindingHint, _privateDisableTooltip} - if (!renderAnchor) { - return ( - - {children} - - ) + const [open, setOpen] = useControllableState({ + name: 'Tooltip', + value: controlledOpen, + defaultValue: defaultOpen, + }) + const [controlled] = React.useState(controlledOpen !== undefined) + const visibility = React.useMemo( + () => ({ + open, + controlled, + onOpenChange(next: boolean, details: TooltipOpenChangeDetails) { + if (open === next) return + setOpen(next) + onOpenChange?.(next, details) + }, + }), + [open, controlled, setOpen, onOpenChange], + ) + const trigger = renderAnchor ?? React.Children.only(children) + if (!React.isValidElement(trigger) && typeof trigger !== 'function') { + throw new Error('Tooltip requires a single trigger.') } return ( ( - + )} /> ) diff --git a/packages/react/src/experimental/TriggerComposition/TriggerComposition.types.test.tsx b/packages/react/src/experimental/TriggerComposition/TriggerComposition.types.test.tsx index f9acb63693f..18acb378ae4 100644 --- a/packages/react/src/experimental/TriggerComposition/TriggerComposition.types.test.tsx +++ b/packages/react/src/experimental/TriggerComposition/TriggerComposition.types.test.tsx @@ -1,3 +1,4 @@ +import type React from 'react' import {InfoIcon} from '@primer/octicons-react' import {ActionMenu, AnchoredOverlay, Tooltip} from './TriggerComposition' import {ActionMenu as StableActionMenu} from '../../ActionMenu' @@ -39,3 +40,22 @@ export function experimentalTooltipSuppressionContract() { // @ts-expect-error stable tooltip suppression is not an experimental trigger prop return Owner} /> } + +export function experimentalTooltipControlContract() { + return ( + { + const next: boolean = open + const cause: string = reason + const source: Event | React.SyntheticEvent | undefined = event + return {next, cause, source} + }} + popupProps={{id: 'tooltip', className: 'tooltip-bubble', popover: 'manual'}} + id="trigger" + text="Details" + renderAnchor={} + /> + ) +} diff --git a/packages/react/src/experimental/index.ts b/packages/react/src/experimental/index.ts index 32b9116abbe..2f137955620 100644 --- a/packages/react/src/experimental/index.ts +++ b/packages/react/src/experimental/index.ts @@ -84,7 +84,12 @@ export type { } from '../NavList' export * from './SelectPanel2' export {Tooltip, ActionMenu, AnchoredOverlay} from './TriggerComposition/TriggerComposition' -export type {TooltipProps, ActionMenuProps, AnchoredOverlayProps} from './TriggerComposition/TriggerComposition' +export type { + TooltipProps, + TooltipOpenChangeDetails, + ActionMenuProps, + AnchoredOverlayProps, +} from './TriggerComposition/TriggerComposition' export * from '../ActionBar' export {ScrollableRegion} from '../ScrollableRegion' From 5e659ff6116c4fbc6feee2458daa045494e78616 Mon Sep 17 00:00:00 2001 From: Adam Dierkens Date: Fri, 13 Nov 2026 17:21:10 -0500 Subject: [PATCH 8/9] Expose experimental tooltip-only suppression Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .changeset/experimental-tooltip-controls.md | 2 +- .../TriggerComposition/README.mdx | 5 +- .../Tooltip.examples.stories.tsx | 13 ++--- .../TriggerComposition/Tooltip.mdx | 31 ++++++++--- .../TriggerComposition.test.tsx | 53 +++++++++++++++++-- .../TriggerComposition/TriggerComposition.tsx | 5 +- .../TriggerComposition.types.test.tsx | 1 - 7 files changed, 84 insertions(+), 26 deletions(-) diff --git a/.changeset/experimental-tooltip-controls.md b/.changeset/experimental-tooltip-controls.md index 146a7ba366b..cf9da06b702 100644 --- a/.changeset/experimental-tooltip-controls.md +++ b/.changeset/experimental-tooltip-controls.md @@ -2,4 +2,4 @@ '@primer/react': minor --- -Tooltip: Add experimental controlled and uncontrolled visibility with separate popup attributes. HTML attributes on the experimental component consistently configure its trigger. +Tooltip: Add experimental tooltip-only suppression, controlled and uncontrolled visibility, and separate popup attributes. HTML attributes on the experimental component consistently configure its trigger. diff --git a/packages/react/src/experimental/TriggerComposition/README.mdx b/packages/react/src/experimental/TriggerComposition/README.mdx index d87661051db..669edf2af23 100644 --- a/packages/react/src/experimental/TriggerComposition/README.mdx +++ b/packages/react/src/experimental/TriggerComposition/README.mdx @@ -102,8 +102,9 @@ for tooltip attribute routing and accessible naming. rendered control using its normal `disabled` or `inactive` API. Native disabled controls cannot open the overlay or tooltip. `aria-disabled` controls cannot activate the overlay but remain focusable for an explanatory tooltip; Tab is - not cancelled. The experimental Tooltip's root API does not expose a `disabled` - prop. + not cancelled. Tooltip's own `disabled` prop suppresses only the tooltip; + it never disables the rendered control. Set `disabled` on the render element + itself to disable that control. - Manage controlled open state normally. Composing triggers does not arbitrate between independently controlled menus, dialogs, or overlays. diff --git a/packages/react/src/experimental/TriggerComposition/Tooltip.examples.stories.tsx b/packages/react/src/experimental/TriggerComposition/Tooltip.examples.stories.tsx index 34dfe1b4e0a..d382c399fa1 100644 --- a/packages/react/src/experimental/TriggerComposition/Tooltip.examples.stories.tsx +++ b/packages/react/src/experimental/TriggerComposition/Tooltip.examples.stories.tsx @@ -16,21 +16,14 @@ type Story = StoryObj export const CoordinateWithMenu: Story = { render: function CoordinateWithMenu() { const [menuOpen, setMenuOpen] = useState(false) - const [tooltipOpen, setTooltipOpen] = useState(false) return ( { - setMenuOpen(next) - if (next) setTooltipOpen(false) - }} + onOpenChange={setMenuOpen} renderAnchor={ { - if (!menuOpen) setTooltipOpen(next) - }} + disabled={menuOpen} popupProps={{id: 'action-tooltip'}} renderAnchor={} /> @@ -49,7 +42,7 @@ export const CoordinateWithMenu: Story = { docs: { description: { story: - 'The application coordinates two visibility states on one composed trigger, without changing tooltip text, calling native popover methods, or merging refs and handlers. For tooltip-only suppression with the stable component, use its disabled prop instead.', + 'Use the menu state to suppress its tooltip with disabled, without a second visibility state or private API. The composed trigger stays enabled, its accessible description is preserved, and refs and handlers merge automatically.', }, }, }, diff --git a/packages/react/src/experimental/TriggerComposition/Tooltip.mdx b/packages/react/src/experimental/TriggerComposition/Tooltip.mdx index 40e9bc6e334..45c94983655 100644 --- a/packages/react/src/experimental/TriggerComposition/Tooltip.mdx +++ b/packages/react/src/experimental/TriggerComposition/Tooltip.mdx @@ -90,16 +90,31 @@ cannot override `open`; this takes precedence over `popupProps.popover`. Uncontrolled tooltips default to `popover="auto"` and synchronize native dismissal with their state. -The existing `_privateDisableTooltip` compatibility prop takes precedence over -visibility, closes the bubble and cancels pending opens while retaining its -accessible text. In controlled mode it requests closing with reason `disabled`; -if the owner keeps `open={true}`, removing suppression makes that value effective +### Tooltip-only suppression + +Use `disabled` to suppress the tooltip without disabling or remounting its +trigger. For example, reuse a menu's existing state without adding tooltip state: + +```tsx +Actions} /> +``` + +Like Base UI's `Tooltip.Root disabled`, this is tooltip behavior, not a native +trigger attribute. Base UI's `Tooltip.Trigger disabled` also suppresses tooltip +interactions rather than disabling its DOM element. To disable the control +itself, put `disabled` on the rendered element, such as +`renderAnchor={}`. + +Suppression takes precedence over `open` and `defaultOpen`, closes a visible bubble, +cancels pending opens, and blocks hover/focus reopening while retaining accessible +labels and descriptions. `onOpenChange` requests closing with reason `disabled` +when an open tooltip is suppressed, even if a controlled owner ignores it. +If the owner keeps `open={true}`, removing suppression makes that value effective again. In uncontrolled mode a fresh interaction is required after re-enabling. -This does not disable the trigger. -The stable Tooltip's public `disabled` API is not added to this experimental -trigger surface. Use the stable export for tooltip-only suppression; use -controlled visibility here when the application must own the visible state. +The existing `_privateDisableTooltip` prop remains a compatibility alias. +Either prop being `true` suppresses the tooltip, even when the other is `false`. +Use public `disabled` for new code; neither prop is forwarded to the trigger. See the **Controlled visibility**, **Separate popup attributes**, and **Coordinate with menu** stories for interactive examples. These state controls diff --git a/packages/react/src/experimental/TriggerComposition/TriggerComposition.test.tsx b/packages/react/src/experimental/TriggerComposition/TriggerComposition.test.tsx index b871f6dc3d0..8f4fcca968e 100644 --- a/packages/react/src/experimental/TriggerComposition/TriggerComposition.test.tsx +++ b/packages/react/src/experimental/TriggerComposition/TriggerComposition.test.tsx @@ -174,7 +174,7 @@ describe('experimental Tooltip routing and visibility', () => { expect(onOpenChange).toHaveBeenLastCalledWith(true, expect.objectContaining({reason: 'trigger-hover'})) }) - it.each(['label', 'description'] as const)('private suppression overrides open while retaining the %s', type => { + it.each(['label', 'description'] as const)('suppression overrides open while retaining the %s', type => { vi.useFakeTimers() const onOpenChange = vi.fn() const props = { @@ -188,7 +188,7 @@ describe('experimental Tooltip routing and visibility', () => { const popup = getByText('Details') const button = page.getByRole('button', {name: type === 'label' ? 'Details' : 'Action', exact: true}).element() expect(popup.matches(':popover-open')).toBe(true) - rerender() + rerender() expect(popup.matches(':popover-open')).toBe(false) expect(onOpenChange).toHaveBeenLastCalledWith(false, expect.objectContaining({reason: 'disabled'})) onOpenChange.mockClear() @@ -201,8 +201,55 @@ describe('experimental Tooltip routing and visibility', () => { } else { expect(button).toHaveAccessibleDescription('Details') } + rerender() + expect(popup.matches(':popover-open')).toBe(true) + expect(onOpenChange).not.toHaveBeenCalled() }) + it.each(['children', 'renderAnchor'] as const)( + 'keeps suppression separate from trigger disabled in %s mode', + async mode => { + const ref = createRef() + const onClick = vi.fn() + const onOpenChange = vi.fn() + const trigger = ( + + ) + const props = { + text: 'Details', + defaultOpen: true, + onOpenChange, + ...(mode === 'renderAnchor' ? {renderAnchor: trigger} : {children: trigger}), + } + const {getByText, rerender} = render() + const popup = getByText('Details') + const button = page.getByRole('button', {name: 'Action', exact: true}) + for (const suppression of [ + {disabled: true, _privateDisableTooltip: false}, + {disabled: false, _privateDisableTooltip: true}, + {disabled: true, _privateDisableTooltip: true}, + ]) { + rerender() + expect(popup.matches(':popover-open')).toBe(false) + expect(button.element()).toBe(ref.current) + expect(button.element()).toBeEnabled() + expect(button.element()).toHaveAccessibleDescription('Details') + await userEvent.click(button) + } + expect(onClick).toHaveBeenCalledTimes(3) + expect(onOpenChange.mock.calls.some(([open]) => open)).toBe(false) + + vi.useFakeTimers() + rerender(Action} />) + expect(button.element()).toBeDisabled() + fireEvent.mouseOver(button.element()) + act(() => vi.advanceTimersByTime(50)) + expect(popup.matches(':popover-open')).toBe(false) + }, + ) + it('cancels a pending open on suppression and requires a fresh interaction when re-enabled', () => { vi.useFakeTimers() const onOpenChange = vi.fn() @@ -212,7 +259,7 @@ describe('experimental Tooltip routing and visibility', () => { const button = page.getByRole('button', {name: 'Action', exact: true}).element() fireEvent.mouseOver(button) act(() => vi.advanceTimersByTime(399)) - rerender() + rerender() act(() => vi.advanceTimersByTime(1)) expect(onOpenChange).not.toHaveBeenCalled() rerender() diff --git a/packages/react/src/experimental/TriggerComposition/TriggerComposition.tsx b/packages/react/src/experimental/TriggerComposition/TriggerComposition.tsx index 02d3c0942c7..ed081940f9d 100644 --- a/packages/react/src/experimental/TriggerComposition/TriggerComposition.tsx +++ b/packages/react/src/experimental/TriggerComposition/TriggerComposition.tsx @@ -56,6 +56,8 @@ export function AnchoredOverlay(props: React.PropsWithChildren & { + /** Suppresses the tooltip, not its trigger. Either suppression prop being true wins. */ + disabled?: boolean renderAnchor?: AnchorRenderer /** HTML props for the tooltip bubble, separate from the trigger's HTML props. */ popupProps?: Omit, 'children'> & {popover?: 'auto' | 'manual' | ''} @@ -79,6 +81,7 @@ export const Tooltip: React.ForwardRefExoticComponent< direction, delay, keybindingHint, + disabled = false, _privateDisableTooltip, popupProps, open: controlledOpen, @@ -119,7 +122,7 @@ export const Tooltip: React.ForwardRefExoticComponent< direction={direction} delay={delay} keybindingHint={keybindingHint} - _privateDisableTooltip={_privateDisableTooltip} + _privateDisableTooltip={disabled || _privateDisableTooltip} visibility={visibility} ref={ref} renderAnchor={tooltipAnchorProps => ( diff --git a/packages/react/src/experimental/TriggerComposition/TriggerComposition.types.test.tsx b/packages/react/src/experimental/TriggerComposition/TriggerComposition.types.test.tsx index 18acb378ae4..675bd48f559 100644 --- a/packages/react/src/experimental/TriggerComposition/TriggerComposition.types.test.tsx +++ b/packages/react/src/experimental/TriggerComposition/TriggerComposition.types.test.tsx @@ -37,7 +37,6 @@ export function stableTooltipContract() { } export function experimentalTooltipSuppressionContract() { - // @ts-expect-error stable tooltip suppression is not an experimental trigger prop return Owner} /> } From 6e415ac708a2b0decd408594919417c70b2f9906 Mon Sep 17 00:00:00 2001 From: Adam Dierkens Date: Fri, 13 Nov 2026 17:47:52 -0500 Subject: [PATCH 9/9] Cover experimental Tooltip trigger className with shared tests Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../TriggerComposition.test.tsx | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/packages/react/src/experimental/TriggerComposition/TriggerComposition.test.tsx b/packages/react/src/experimental/TriggerComposition/TriggerComposition.test.tsx index 8f4fcca968e..219e548c6fc 100644 --- a/packages/react/src/experimental/TriggerComposition/TriggerComposition.test.tsx +++ b/packages/react/src/experimental/TriggerComposition/TriggerComposition.test.tsx @@ -9,12 +9,26 @@ import {ActionMenu as StableActionMenu} from '../../ActionMenu' import {ActionList} from '../../ActionList' import {Button, IconButton} from '../../Button' import {FeatureFlags} from '../../FeatureFlags' -import {withExpectedConsoleError} from '../../utils/testing' +import {implementsClassName, withExpectedConsoleError} from '../../utils/testing' import {reactMajorVersion} from '../../utils/environment' describe('experimental Tooltip routing and visibility', () => { afterEach(() => vi.useRealTimers()) + describe.each(['children', 'renderAnchor'] as const)('trigger className in %s mode', mode => { + implementsClassName(({className}: {className?: string}) => { + const trigger = + return ( + + ) + }, 'tooltip-trigger') + }) + it.each(['children', 'renderAnchor'] as const)('separates trigger and popup attributes in %s mode', mode => { const onPopupEnter = vi.fn() const buttonRef = createRef()