Tooltip keybinding hints (#5252)

* Make keybinding hint font-size dynamic to parent font-size

* Add keybinding hint support to Tooltip

* Move `IconButton` to `keybindingHint`

* Add `small` size and update design of `onEmphasis` variant

* Fix unit tests

* Add changeset

* Update story IDs in JSON

* Update test config

* Reduce right spacing

* Still not quite right on the spacing...

* Revert deps

* test(vrt): update snapshots

---------

Co-authored-by: iansan5653 <iansan5653@users.noreply.github.com>

.changeset/cyan-glasses-sit.md

@@ -0,0 +1,5 @@
+---
+"@primer/react": minor
+---
+
+Adds `keybindingHint` prop to `TooltipV2` and `IconButton`, updates `onEmphasis` design variant for `KeybindingHint`, and adds `size` prop with `small` option to `KeybindingHint`.

e2e/components/IconButton.test.ts

@@ -308,12 +308,12 @@ test.describe('IconButton', () => {
       })
     }
   })
-  test.describe('Keyshortcuts', () => {
+  test.describe('Keybinding hint', () => {
     for (const theme of themes) {
       test.describe(theme, () => {
         test('default @vrt', async ({page}) => {
           await visit(page, {
-            id: 'components-iconbutton-features--keyshortcuts',
+            id: 'components-iconbutton-features--keybinding-hint',
             globals: {
               colorScheme: theme,
             },
@@ -322,13 +322,13 @@ test.describe('IconButton', () => {
           // Default state
           await page.keyboard.press('Tab') // focus on icon button
           expect(await page.screenshot({animations: 'disabled'})).toMatchSnapshot(
-            `IconButton.Keyshortcuts.${theme}.png`,
+            `IconButton.Keybinding Hint.${theme}.png`,
           )
         })
 
         test('axe @aat', async ({page}) => {
           await visit(page, {
-            id: 'components-iconbutton-features--keyshortcuts',
+            id: 'components-iconbutton-features--keybinding-hint',
             globals: {
               colorScheme: theme,
             },
@@ -340,12 +340,12 @@ test.describe('IconButton', () => {
     }
   })
 
-  test.describe('Keyshortcuts on Description', () => {
+  test.describe('Keybinding hint on Description', () => {
     for (const theme of themes) {
       test.describe(theme, () => {
         test('default @vrt', async ({page}) => {
           await visit(page, {
-            id: 'components-iconbutton-features--keyshortcuts-on-description',
+            id: 'components-iconbutton-features--keybinding-hint-on-description',
             globals: {
               colorScheme: theme,
             },
@@ -354,13 +354,13 @@ test.describe('IconButton', () => {
           // Default state
           await page.keyboard.press('Tab') // focus on icon button
           expect(await page.screenshot({animations: 'disabled'})).toMatchSnapshot(
-            `IconButton.Keyshortcuts on Description.${theme}.png`,
+            `IconButton.Keybinding Hint on Description.${theme}.png`,
           )
         })
 
         test('axe @aat', async ({page}) => {
           await visit(page, {
-            id: 'components-iconbutton-features--keyshortcuts-on-description',
+            id: 'components-iconbutton-features--keybinding-hint-on-description',
             globals: {
               colorScheme: theme,
             },

packages/react/src/Button/IconButton.docs.json

@@ -47,10 +47,10 @@
       "id": "components-iconbutton-features--loading-trigger"
     },
     {
-      "id": "components-iconbutton-features--keyshortcuts-on-description"
+      "id": "components-iconbutton-features--keybinding-hint-on-description"
     },
     {
-      "id": "components-iconbutton-features--keyshortcuts"
+      "id": "components-iconbutton-features--keybinding-hint"
     }
   ],
   "importPath": "@primer/react",
@@ -95,8 +95,13 @@
       "name": "keyshortcuts",
       "type": "string",
       "defaultValue": "",
-      "required": false,
-      "description": "Keyboard shortcuts that trigger the button. Keyboard shortcut will be appended to the accessible name or description (depending on the tooltip type) of the button with a comma (i.e. Bold, Command+B) and it will be displayed in the tooltip."
+      "deprecated": true,
+      "description": "Use `keybindingHint` instead."
+    },
+    {
+      "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."
     },
     {
       "name": "tooltipDirection",

packages/react/src/Button/IconButton.features.stories.tsx

@@ -84,13 +84,13 @@ export const LoadingTrigger = () => {
 
   return <IconButton loading={isLoading} onClick={handleClick} icon={DownloadIcon} aria-label="Download" />
 }
-export const KeyshortcutsOnDescription = () => (
+export const KeybindingHintOnDescription = () => (
   <IconButton
     icon={InboxIcon}
     aria-label="Notifications"
     description="You have unread notifications"
-    keyshortcuts="G+N"
+    keybindingHint="G+N"
   />
 )
 
-export const Keyshortcuts = () => <IconButton icon={BoldIcon} aria-label="Bold" keyshortcuts="Command+B" />
+export const KeybindingHint = () => <IconButton icon={BoldIcon} aria-label="Bold" keybindingHint="Mod+B" />

packages/react/src/Button/IconButton.tsx

@@ -21,6 +21,7 @@ const IconButton = forwardRef(
       // This is planned to be a temporary prop until the default tooltip on icon buttons are fully rolled out.
       unsafeDisableTooltip = false,
       keyshortcuts,
+      keybindingHint,
       className,
       ...props
     },
@@ -58,15 +59,14 @@ const IconButton = forwardRef(
         />
       )
     } else {
-      // Does it have keyshortcuts?
-      const tooltipSuffix = keyshortcuts ? `, ${keyshortcuts}` : ''
       const tooltipText = description ?? ariaLabel
       return (
         <Tooltip
           ref={forwardedRef}
-          text={`${tooltipText}${tooltipSuffix}`}
+          text={tooltipText}
           type={description ? undefined : 'label'}
           direction={tooltipDirection}
+          keybindingHint={keybindingHint ?? keyshortcuts}
         >
           <ButtonBase
             icon={Icon}

packages/react/src/Button/__tests__/Button.test.tsx

@@ -340,26 +340,22 @@ describe('Button', () => {
     expect(triggerEl).toHaveAttribute('aria-keyshortcuts', 'Command+H')
   })
   it('should append the keyshortcuts to the tooltip text that labels the icon button when keyshortcuts prop is passed', () => {
-    const {getByRole, getByText} = render(<IconButton icon={HeartIcon} aria-label="Heart" keyshortcuts="Command+H" />)
-    const triggerEl = getByRole('button')
-    const tooltipEl = getByText('Heart, Command+H')
-    expect(tooltipEl).toBeInTheDocument()
-    expect(triggerEl).toHaveAttribute('aria-labelledby', tooltipEl.id)
+    const {getByRole} = render(<IconButton icon={HeartIcon} aria-label="Heart" keyshortcuts="Command+H" />)
+    const triggerEl = getByRole('button', {name: 'Heart ( command h )'})
+    expect(triggerEl).toBeInTheDocument()
   })
-  it('should render aria-keyshorts on an icon button when keyshortcuts prop is passed (Description Type)', () => {
+  it('should render aria-keyshortcuts on an icon button when keyshortcuts prop is passed (Description Type)', () => {
     const {getByRole} = render(
       <IconButton icon={HeartIcon} aria-label="Heart" description="Love is all around" keyshortcuts="Command+H" />,
     )
     const triggerEl = getByRole('button')
     expect(triggerEl).toHaveAttribute('aria-keyshortcuts', 'Command+H')
   })
   it('should append the keyshortcuts to the tooltip text that describes the icon button when keyshortcuts prop is passed (Description Type)', () => {
-    const {getByRole, getByText} = render(
+    const {getByRole} = render(
       <IconButton icon={HeartIcon} aria-label="Heart" description="Love is all around" keyshortcuts="Command+H" />,
     )
-    const triggerEl = getByRole('button')
-    const tooltipEl = getByText('Love is all around, Command+H')
-    expect(tooltipEl).toBeInTheDocument()
-    expect(triggerEl.getAttribute('aria-describedby')).toEqual(expect.stringContaining(tooltipEl.id))
+    const triggerEl = getByRole('button', {name: 'Heart'})
+    expect(triggerEl).toHaveAccessibleDescription('Love is all around ( command h )')
   })
 })

packages/react/src/Button/types.ts

@@ -94,7 +94,9 @@ export type IconButtonProps = ButtonA11yProps & {
   unsafeDisableTooltip?: boolean
   description?: string
   tooltipDirection?: TooltipDirection
+  /** @deprecated Use `keybindingHint` instead. */
   keyshortcuts?: string
+  keybindingHint?: string
 } & Omit<ButtonBaseProps, 'aria-label' | 'aria-labelledby'>
 
 // adopted from React.AnchorHTMLAttributes

packages/react/src/KeybindingHint/KeybindingHint.docs.json

@@ -22,6 +22,12 @@
       "type": "'normal' | 'onEmphasis'",
       "defaultValue": "'normal'",
       "description": "Set to `onEmphasis` for display on 'emphasis' colors."
+    },
+    {
+      "name": "size",
+      "type": "'small' | 'normal'",
+      "defaultValue": "'normal'",
+      "description": "Control the size of the hint."
     }
   ],
   "subcomponents": []

packages/react/src/KeybindingHint/KeybindingHint.features.stories.tsx

@@ -28,3 +28,5 @@ export const OnEmphasis: StoryObj<KeybindingHintProps> = {
   ),
   args: {keys: chord, variant: 'onEmphasis'},
 }
+
+export const Small = {args: {keys: chord, size: 'small'}}

packages/react/src/KeybindingHint/components/Chord.tsx

@@ -29,24 +29,26 @@ const splitChord = (chord: string) =>
     .map(k => k.toLowerCase())
     .sort(compareLowercaseKeys)
 
-export const Chord = ({keys, format = 'condensed', variant = 'normal'}: KeybindingHintProps) => (
+export const Chord = ({keys, format = 'condensed', variant = 'normal', size = 'normal'}: KeybindingHintProps) => (
   <Text
     sx={{
       display: 'inline-flex',
-      bg: 'transparent',
-      color: variant === 'onEmphasis' ? 'fg.onEmphasis' : 'fg.muted',
+      bg: variant === 'onEmphasis' ? 'var(--counter-bgColor-emphasis)' : 'var(--bgColor-transparent)',
+      color: variant === 'onEmphasis' ? 'var(--fgColor-onEmphasis)' : 'var(--fgColor-muted)',
       border: '1px solid',
-      borderColor: 'border.default',
-      borderRadius: 2,
+      borderColor: variant === 'onEmphasis' ? 'transparent' : 'var(--borderColor-default)',
+      borderRadius: size === 'small' ? 1 : 2,
       fontWeight: 'normal',
       fontFamily: 'normal',
-      fontSize: 0,
-      p: 1,
+      fontSize: size === 'small' ? '11px' : 0,
+      p: size === 'small' ? '2px' : 1,
       gap: '0.5ch',
       boxShadow: 'none',
       verticalAlign: 'baseline',
       overflow: 'hidden',
-      lineHeight: '10px',
+      lineHeight: size === 'small' ? '8px' : '10px',
+      minWidth: size === 'small' ? 'var(--base-size-16)' : 'var(--base-size-20)',
+      justifyContent: 'center',
     }}
   >
     {splitChord(keys).map((k, i) => (

packages/react/src/KeybindingHint/components/Sequence.tsx

@@ -5,7 +5,7 @@ import {accessibleChordString, Chord} from './Chord'
 
 const splitSequence = (sequence: string) => sequence.split(' ')
 
-export const Sequence = ({keys, format = 'condensed', variant = 'normal'}: KeybindingHintProps) =>
+export const Sequence = ({keys, ...chordProps}: KeybindingHintProps) =>
   splitSequence(keys).map((c, i) => (
     <Fragment key={i}>
       {
@@ -16,7 +16,7 @@ export const Sequence = ({keys, format = 'condensed', variant = 'normal'}: Keybi
           </>
         )
       }
-      <Chord keys={c} format={format} variant={variant} />
+      <Chord keys={c} {...chordProps} />
     </Fragment>
   ))
 

packages/react/src/KeybindingHint/props.ts

@@ -27,4 +27,8 @@ export interface KeybindingHintProps {
    * Set to `onEmphasis` for display on emphasis colors.
    */
   variant?: KeybindingHintVariant
+  /**
+   * Control the size of the hint.
+   */
+  size?: 'small' | 'normal'
 }

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

@@ -53,6 +53,11 @@
       "defaultValue": "description",
       "description": "The type of tooltip. `label` is used for labelling the element that triggers tooltip. `description` is used for describing or adding a suplementary information to the element that triggers the tooltip."
     },
+    {
+      "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."
+    },
     {
       "name": "sx",
       "type": "SystemStyleObject"

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

@@ -2,7 +2,7 @@ import React from 'react'
 import {IconButton, Button, Box, Link, ActionMenu, ActionList} from '..'
 import Octicon from '../Octicon'
 import {Tooltip} from './Tooltip'
-import {SearchIcon, BookIcon, CheckIcon, TriangleDownIcon, GitBranchIcon} from '@primer/octicons-react'
+import {SearchIcon, BookIcon, CheckIcon, TriangleDownIcon, GitBranchIcon, InfoIcon} from '@primer/octicons-react'
 
 export default {
   title: 'Components/TooltipV2/Features',
@@ -174,3 +174,13 @@ export const OnActionMenuAnchor = () => (
     </ActionMenu>
   </Box>
 )
+
+export const KeybindingHint = () => (
+  <Box sx={{p: 6}}>
+    <Tooltip text="Learn more" keybindingHint="Shift+?" type="label">
+      <Link href="#">
+        <InfoIcon />
+      </Link>
+    </Tooltip>
+  </Box>
+)

packages/react/src/TooltipV2/Tooltip.module.css

@@ -118,3 +118,10 @@
     animation-delay: 0s;
   }
 }
+
+.keybindingHintContainer.hasTextBefore {
+  /* Offset negative on the right right to make right spacing look like top and bottom spacing, only for keybinding hints and only when there is text */
+  /* stylelint-disable-next-line primer/spacing */
+  margin-right: -0.125em;
+  margin-left: var(--base-size-6);
+}

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

@@ -11,6 +11,7 @@ const meta: Meta<typeof Tooltip> = {
     text: 'This is the tooltip text',
     direction: 's',
     type: 'description',
+    keybindingHint: undefined,
   },
   argTypes: {
     text: {control: {type: 'text'}},
@@ -21,6 +22,9 @@ const meta: Meta<typeof Tooltip> = {
     type: {
       control: false,
     },
+    keybindingHint: {
+      control: {type: 'text'},
+    },
   },
 }
 

packages/react/src/TooltipV2/Tooltip.tsx

@@ -13,6 +13,8 @@ import {toggleStyledComponent} from '../internal/utils/toggleStyledComponent'
 import {clsx} from 'clsx'
 import classes from './Tooltip.module.css'
 import {useFeatureFlag} from '../FeatureFlags'
+import {KeybindingHint, type KeybindingHintProps} from '../KeybindingHint'
+import VisuallyHidden from '../_VisuallyHidden'
 
 const CSS_MODULE_FEATURE_FLAG = 'primer_react_css_modules_team'
 
@@ -138,6 +140,7 @@ export type TooltipProps = React.PropsWithChildren<
     direction?: TooltipDirection
     text: string
     type?: 'label' | 'description'
+    keybindingHint?: KeybindingHintProps['keys']
   } & SxProp
 > &
   React.HTMLAttributes<HTMLElement>
@@ -196,7 +199,10 @@ const isInteractive = (element: HTMLElement) => {
 export const TooltipContext = React.createContext<{tooltipId?: string}>({})
 
 export const Tooltip = React.forwardRef(
-  ({direction = 's', text, type = 'description', children, id, className, ...rest}: TooltipProps, forwardedRef) => {
+  (
+    {direction = 's', text, type = 'description', children, id, className, keybindingHint, ...rest}: TooltipProps,
+    forwardedRef,
+  ) => {
     const tooltipId = useId(id)
     const child = Children.only(children)
     const triggerRef = useProvidedRefOrCreate(forwardedRef as React.RefObject<HTMLElement>)
@@ -379,6 +385,13 @@ export const Tooltip = React.forwardRef(
             onMouseLeave={closeTooltip}
           >
             {text}
+            {keybindingHint && (
+              <span className={clsx(classes.keybindingHintContainer, text && classes.hasTextBefore)}>
+                <VisuallyHidden>(</VisuallyHidden>
+                <KeybindingHint keys={keybindingHint} format="condensed" variant="onEmphasis" size="small" />
+                <VisuallyHidden>)</VisuallyHidden>
+              </span>
+            )}
           </StyledTooltip>
         </>
       </TooltipContext.Provider>