diff --git a/.changeset/iconbutton-keybinding-hint-array.md b/.changeset/iconbutton-keybinding-hint-array.md new file mode 100644 index 00000000000..9c8126df9c1 --- /dev/null +++ b/.changeset/iconbutton-keybinding-hint-array.md @@ -0,0 +1,5 @@ +--- +"@primer/react": minor +--- + +`IconButton`: `keybindingHint` now accepts `string[]` in addition to `string`. Multiple hints are rendered joined with "or". diff --git a/packages/react/src/Button/IconButton.docs.json b/packages/react/src/Button/IconButton.docs.json index 58ec3573306..4af4062608e 100644 --- a/packages/react/src/Button/IconButton.docs.json +++ b/packages/react/src/Button/IconButton.docs.json @@ -107,8 +107,8 @@ }, { "name": "keybindingHint", - "type": "string", - "description": "Optional keybinding hint to show in the tooltip for this button. See the `KeybindingHint` component documentation for the correct format for this string. Has no effect if tooltip is overridden or disabled. Does **not** bind any keybindings for this button - this is only for visual hints." + "type": "string | string[]", + "description": "Optional keybinding hint to show in the tooltip for this button. Pass a string for a single shortcut or an array of strings to show multiple shortcuts joined with \"or\". See the `KeybindingHint` component documentation for the correct format. Has no effect if tooltip is overridden or disabled. Does **not** bind any keybindings for this button - this is only for visual hints." }, { "name": "tooltipDirection", diff --git a/packages/react/src/Button/IconButton.features.stories.tsx b/packages/react/src/Button/IconButton.features.stories.tsx index 937ad43bb45..27147337b90 100644 --- a/packages/react/src/Button/IconButton.features.stories.tsx +++ b/packages/react/src/Button/IconButton.features.stories.tsx @@ -88,6 +88,10 @@ export const KeybindingHintOnDescription = () => ( export const KeybindingHint = () => +export const MultipleKeybindingHints = () => ( + +) + export const LongDelayedTooltip = () => ( // Ideal for cases where we don't want to show the tooltip immediately — for example, when the user is just passing over the element. diff --git a/packages/react/src/Button/types.ts b/packages/react/src/Button/types.ts index 6870562b516..acdd7190527 100644 --- a/packages/react/src/Button/types.ts +++ b/packages/react/src/Button/types.ts @@ -91,7 +91,7 @@ export type IconButtonProps = ButtonA11yProps & { tooltipDirection?: TooltipDirection /** @deprecated Use `keybindingHint` instead. */ keyshortcuts?: string - keybindingHint?: string + keybindingHint?: string | string[] } & Omit // adopted from React.AnchorHTMLAttributes diff --git a/packages/react/src/TooltipV2/Tooltip.docs.json b/packages/react/src/TooltipV2/Tooltip.docs.json index 5b56bc7d269..019128ada7d 100644 --- a/packages/react/src/TooltipV2/Tooltip.docs.json +++ b/packages/react/src/TooltipV2/Tooltip.docs.json @@ -55,8 +55,8 @@ }, { "name": "keybindingHint", - "type": "string", - "description": "Optional keybinding hint to indicate the availability of a keyboard shortcut. Supported syntax is described in the docs for the `KeybindingHint` component." + "type": "string | string[]", + "description": "Optional keybinding hint to indicate the availability of a keyboard shortcut. Pass a string for a single shortcut or an array of strings to show multiple shortcuts joined with \"or\". Supported syntax is described in the docs for the `KeybindingHint` component." }, { "name": "delay", diff --git a/packages/react/src/TooltipV2/Tooltip.module.css b/packages/react/src/TooltipV2/Tooltip.module.css index 0c51f0a5cae..d95789c1299 100644 --- a/packages/react/src/TooltipV2/Tooltip.module.css +++ b/packages/react/src/TooltipV2/Tooltip.module.css @@ -126,3 +126,7 @@ .KeybindingHintContainer.HasTextBefore { margin-left: var(--base-size-6); } + +.KeybindingHintContainer.HasTextBefore.HasMultipleHints { + margin-left: var(--base-size-8); +} diff --git a/packages/react/src/TooltipV2/Tooltip.tsx b/packages/react/src/TooltipV2/Tooltip.tsx index bd33f49c562..92a516d75c5 100644 --- a/packages/react/src/TooltipV2/Tooltip.tsx +++ b/packages/react/src/TooltipV2/Tooltip.tsx @@ -17,7 +17,7 @@ export type TooltipProps = React.PropsWithChildren<{ direction?: TooltipDirection text: string type?: 'label' | 'description' - keybindingHint?: KeybindingHintProps['keys'] + keybindingHint?: KeybindingHintProps['keys'] | Array /** * Delay in milliseconds before showing the tooltip * @default short (50ms) @@ -103,6 +103,8 @@ const isInteractive = (element: HTMLElement) => { } export const TooltipContext = React.createContext<{tooltipId?: string}>({}) +const emptyKeybindingHints: Array = [] + export const Tooltip: ForwardRefExoticComponent< React.PropsWithoutRef & React.RefAttributes > & @@ -115,7 +117,7 @@ export const Tooltip: ForwardRefExoticComponent< children, id, className, - keybindingHint, + keybindingHint = emptyKeybindingHints, delay = 'short', _privateDisableTooltip = false, ...rest @@ -273,6 +275,9 @@ export const Tooltip: ForwardRefExoticComponent< const isMacOS = useIsMacOS() const hasAriaLabel = 'aria-label' in rest + // Normalize keybindingHint to an array for uniform rendering + const keybindingHints = Array.isArray(keybindingHint) ? keybindingHint : [keybindingHint] + return ( <> @@ -353,21 +358,35 @@ export const Tooltip: ForwardRefExoticComponent< onMouseEnter={openTooltip} onMouseLeave={closeTooltip} // If there is an aria-label prop, always assign the ID to the parent so the accessible label can be overridden - id={hasAriaLabel || !keybindingHint ? tooltipId : undefined} + id={hasAriaLabel || keybindingHints.length === 0 ? tooltipId : undefined} > - {keybindingHint ? ( + {keybindingHints.length > 0 ? ( <> {text} {/* There is a bug in Chrome browsers where `aria-hidden` text inside the target of an `aria-labelledby` - still gets included in the accessible label. `KeybindingHint` renders the symbols as `aria-hidden` text - and renders full key names as `VisuallyHidden` text. Due to the browser bug this causes the label text - to duplicate the symbols and key names. To work around this, we exclude the hint from being part of the - label and instead render the plain keybinding description string. */} - ({getAccessibleKeybindingHintString(keybindingHint, isMacOS)}) + still gets included in the accessible label. `KeybindingHint` renders the symbols as `aria-hidden` text + and renders full key names as `VisuallyHidden` text. Due to the browser bug this causes the label text + to duplicate the symbols and key names. To work around this, we exclude the hint from being part of the + label and instead render the plain keybinding description string. */} + + ({keybindingHints.map(hint => getAccessibleKeybindingHintString(hint, isMacOS)).join(' or ')}) + - - + 1 && classes.HasMultipleHints, + )} + aria-hidden + > + {keybindingHints.map((hint, i) => ( + + {i > 0 && ' or '} + + + ))} ) : ( diff --git a/packages/react/src/TooltipV2/__tests__/Tooltip.test.tsx b/packages/react/src/TooltipV2/__tests__/Tooltip.test.tsx index 8960e76f7a5..9d914c6b007 100644 --- a/packages/react/src/TooltipV2/__tests__/Tooltip.test.tsx +++ b/packages/react/src/TooltipV2/__tests__/Tooltip.test.tsx @@ -172,6 +172,23 @@ describe('Tooltip', () => { ) expect(getByRole('button', {name: 'Overridden label'})).toBeInTheDocument() }) + it('includes multiple keybinding hints joined with "or" in the label text', () => { + const {getByRole} = HTMLRender() + expect(getByRole('button', {name: 'Tooltip text (control k or control shift k)'})).toBeInTheDocument() + }) + it('renders multiple keybinding hints when an array is provided', () => { + const {getAllByTestId, container} = HTMLRender( + , + ) + expect(getAllByTestId('keybinding-hint')).toHaveLength(2) + // Verify the "or" separator is rendered between keybinding hints + const hintContainer = container.querySelector('[aria-hidden="true"] [aria-hidden="true"]') + expect(hintContainer?.textContent).toContain(' or ') + }) + it('treats an empty array keybindingHint as if no hint was provided', () => { + const {queryByTestId} = HTMLRender() + expect(queryByTestId('keybinding-hint')).not.toBeInTheDocument() + }) it('should append tooltip id to existing aria-describedby value on the trigger element', () => { const {getByRole, getByText} = HTMLRender()