IconButton: introduce tooltips on icon buttons behind the `unsafeDisa…

…bleTooltip` prop for an incremental rollout (#4224)

* IconButton: introduce tooltip behaviuor behind a prop

* changeset add

* add comment

* rename the tooltip prop

* rename the prop again 🙃

* remove redundant stories

* clean up the mess

* test(vrt): update snapshots

* toolbar button disable tooltip

* icon button as menu anchor & external tooltip & tooltipDirection

* update snaps

* code review comments

* show tooltip only on focus-visible

* catch focus-visible for jestdom

* rename the prop and update the default version to be true

* fix tests

* already default true

* test(vrt): update snapshots

* improve test and example

* test(vrt): update snapshots

* fix linting

* pass the id down and add test

---------

Co-authored-by: broccolinisoup <[email protected]>
Co-authored-by: Siddharth Kshetrapal <[email protected]>

.changeset/smart-chefs-fail.md

@@ -0,0 +1,7 @@
+---
+'@primer/react': minor
+---
+
+IconButton: introduce tooltips on icon buttons behind the `unsafeDisableTooltip` prop for an incremental rollout.
+
+In the next release, we plan to update the default value of `unsafeDisableTooltip` to `false` so that the tooltip behaviour becomes the default. 

packages/react/src/ActionMenu/ActionMenu.tsx

@@ -1,4 +1,4 @@
-import React from 'react'
+import React, {useEffect, useState} from 'react'
 import {TriangleDownIcon} from '@primer/octicons-react'
 import type {AnchoredOverlayProps} from '../AnchoredOverlay'
 import {AnchoredOverlay} from '../AnchoredOverlay'
@@ -147,6 +147,17 @@ const Overlay: React.FC<React.PropsWithChildren<MenuOverlayProps>> = ({
   const containerRef = React.useRef<HTMLDivElement>(null)
   useMenuKeyboardNavigation(open, onClose, containerRef, anchorRef)
 
+  // If the menu anchor is an icon button, we need to label the menu by tooltip that also labelled the anchor.
+  const [anchorAriaLabelledby, setAnchorAriaLabelledby] = useState<null | string>(null)
+  useEffect(() => {
+    if (anchorRef.current) {
+      const ariaLabelledby = anchorRef.current.getAttribute('aria-labelledby')
+      if (ariaLabelledby) {
+        setAnchorAriaLabelledby(ariaLabelledby)
+      }
+    }
+  }, [anchorRef])
+
   return (
     <AnchoredOverlay
       anchorRef={anchorRef}
@@ -165,7 +176,8 @@ const Overlay: React.FC<React.PropsWithChildren<MenuOverlayProps>> = ({
           value={{
             container: 'ActionMenu',
             listRole: 'menu',
-            listLabelledBy: ariaLabelledby || anchorId,
+            // If there is a custom aria-labelledby, use that. Otherwise, if exists, use the id that labels the anchor such as tooltip. If none of them exist, use anchor id.
+            listLabelledBy: ariaLabelledby || anchorAriaLabelledby || anchorId,
             selectionAttribute: 'aria-checked', // Should this be here?
             afterSelect: onClose,
           }}

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

@@ -7,7 +7,14 @@ export default {
 }
 
 export const CustomSize = () => (
-  <IconButton aria-label="Expand" variant="primary" size="small" icon={ChevronDownIcon} sx={{width: 16, height: 16}} />
+  <IconButton
+    aria-label="Expand"
+    variant="primary"
+    size="small"
+    icon={ChevronDownIcon}
+    unsafeDisableTooltip={false}
+    sx={{width: 16, height: 16}}
+  />
 )
 
 export const CustomSizeWithMedia = () => {
@@ -17,11 +24,19 @@ export const CustomSizeWithMedia = () => {
       variant="primary"
       size="small"
       icon={ChevronDownIcon}
+      unsafeDisableTooltip={false}
       sx={{'@media (min-width: 123px)': {width: 16, height: 16}}}
     />
   )
 }
 
 export const CustomIconColor = () => (
-  <IconButton aria-label="Expand" variant="invisible" size="small" icon={ChevronDownIcon} sx={{color: 'red'}} />
+  <IconButton
+    aria-label="Expand"
+    variant="invisible"
+    size="small"
+    icon={ChevronDownIcon}
+    unsafeDisableTooltip={false}
+    sx={{color: 'red'}}
+  />
 )

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

@@ -1,21 +1,90 @@
-import {HeartIcon} from '@primer/octicons-react'
+import {HeartIcon, InboxIcon, ChevronDownIcon} from '@primer/octicons-react'
 import React from 'react'
 import {IconButton} from '.'
+import {ActionMenu} from '../ActionMenu'
+import {ActionList} from '../ActionList'
+import {Tooltip} from '../TooltipV2'
+import {default as TooltipV1} from '../Tooltip'
 
 export default {
   title: 'Components/IconButton/Features',
 }
 
-export const Primary = () => <IconButton icon={HeartIcon} variant="primary" aria-label="Favorite" />
+export const Primary = () => (
+  <IconButton icon={HeartIcon} variant="primary" aria-label="Favorite" unsafeDisableTooltip={false} />
+)
 
-export const Danger = () => <IconButton icon={HeartIcon} variant="danger" aria-label="Favorite" />
+export const Danger = () => (
+  <IconButton icon={HeartIcon} variant="danger" aria-label="Favorite" unsafeDisableTooltip={false} />
+)
 
-export const Invisible = () => <IconButton icon={HeartIcon} variant="invisible" aria-label="Favorite" />
+export const Invisible = () => (
+  <IconButton icon={HeartIcon} variant="invisible" aria-label="Favorite" unsafeDisableTooltip={false} />
+)
 
-export const Disabled = () => <IconButton disabled icon={HeartIcon} aria-label="Favorite" />
+export const Disabled = () => (
+  <IconButton disabled icon={HeartIcon} aria-label="Favorite" unsafeDisableTooltip={false} />
+)
 
-export const Small = () => <IconButton size="small" icon={HeartIcon} aria-label="Favorite" />
+export const Small = () => (
+  <IconButton size="small" icon={HeartIcon} aria-label="Favorite" unsafeDisableTooltip={false} />
+)
 
-export const Medium = () => <IconButton size="medium" icon={HeartIcon} aria-label="Favorite" />
+export const Medium = () => (
+  <IconButton size="medium" icon={HeartIcon} aria-label="Favorite" unsafeDisableTooltip={false} />
+)
 
-export const Large = () => <IconButton size="large" icon={HeartIcon} aria-label="Favorite" />
+export const Large = () => (
+  <IconButton size="large" icon={HeartIcon} aria-label="Favorite" unsafeDisableTooltip={false} />
+)
+
+export const WithDescription = () => (
+  <IconButton
+    icon={InboxIcon}
+    aria-label="Notifications"
+    description="You have no unread notifications."
+    unsafeDisableTooltip={false}
+  />
+)
+
+export const ExternalTooltip = () => (
+  <Tooltip text="this is a supportive description for icon button" direction="se">
+    <IconButton icon={HeartIcon} aria-label="HeartIcon" />
+  </Tooltip>
+)
+
+export const ExternalTooltipVersion1 = () => (
+  <TooltipV1 text="this is a supportive description for icon button" direction="se">
+    <IconButton icon={HeartIcon} aria-label="HeartIcon" />
+  </TooltipV1>
+)
+
+export const AsAMenuAnchor = () => (
+  <ActionMenu>
+    <ActionMenu.Anchor>
+      <IconButton icon={ChevronDownIcon} aria-label="Something" unsafeDisableTooltip={false} />
+    </ActionMenu.Anchor>
+
+    <ActionMenu.Overlay width="medium">
+      <ActionList>
+        <ActionList.Item onSelect={() => alert('Copy link clicked')}>
+          Copy link
+          <ActionList.TrailingVisual>⌘C</ActionList.TrailingVisual>
+        </ActionList.Item>
+        <ActionList.Item onSelect={() => alert('Quote reply clicked')}>
+          Quote reply
+          <ActionList.TrailingVisual>⌘Q</ActionList.TrailingVisual>
+        </ActionList.Item>
+        <ActionList.Item onSelect={() => alert('Edit comment clicked')}>
+          Edit comment
+          <ActionList.TrailingVisual>⌘E</ActionList.TrailingVisual>
+        </ActionList.Item>
+        <ActionList.Divider />
+        <ActionList.Item variant="danger" onSelect={() => alert('Delete file clicked')}>
+          Delete file
+          <ActionList.TrailingVisual>⌘D</ActionList.TrailingVisual>
+        </ActionList.Item>
+      </ActionList>
+    </ActionMenu.Overlay>
+  </ActionMenu>
+)

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

@@ -46,4 +46,4 @@ Playground.args = {
   icon: HeartIcon,
 }
 
-export const Default = () => <IconButton icon={HeartIcon} aria-label="Favorite" />
+export const Default = () => <IconButton icon={HeartIcon} aria-label="Favorite" unsafeDisableTooltip={false} />

packages/react/src/Button/IconButton.tsx

@@ -4,20 +4,75 @@ import type {ForwardRefComponent as PolymorphicForwardRefComponent} from '../uti
 import {ButtonBase} from './ButtonBase'
 import {defaultSxProp} from '../utils/defaultSxProp'
 import {generateCustomSxProp} from './Button'
+import {TooltipContext, Tooltip} from '../TooltipV2/Tooltip'
+import {TooltipContext as TooltipContextV1} from '../Tooltip/Tooltip'
 
-const IconButton = forwardRef(({sx: sxProp = defaultSxProp, icon: Icon, ...props}, forwardedRef): JSX.Element => {
-  let sxStyles = sxProp
-  // grap the button props that have associated data attributes in the styles
-  const {size} = props
+const IconButton = forwardRef(
+  (
+    {
+      sx: sxProp = defaultSxProp,
+      icon: Icon,
+      'aria-label': ariaLabel,
+      description,
+      disabled,
+      tooltipDirection,
+      // This is planned to be a temporary prop until the default tooltip on icon buttons are fully rolled out.
+      unsafeDisableTooltip = true,
+      ...props
+    },
+    forwardedRef,
+  ): JSX.Element => {
+    let sxStyles = sxProp
+    // grap the button props that have associated data attributes in the styles
+    const {size} = props
 
-  if (sxProp !== null && Object.keys(sxProp).length > 0) {
-    sxStyles = generateCustomSxProp({size}, sxProp)
-  }
+    if (sxProp !== null && Object.keys(sxProp).length > 0) {
+      sxStyles = generateCustomSxProp({size}, sxProp)
+    }
 
-  return (
-    // @ts-expect-error StyledButton wants both Anchor and Button refs
-    <ButtonBase icon={Icon} data-component="IconButton" sx={sxStyles} type="button" {...props} ref={forwardedRef} />
-  )
-}) as PolymorphicForwardRefComponent<'button' | 'a', IconButtonProps>
+    // If the icon button is already wrapped in a tooltip, do not add one.
+    const {tooltipId} = React.useContext(TooltipContext) // Tooltip v2
+    const {tooltipId: tooltipIdV1} = React.useContext(TooltipContextV1) // Tooltip v1
+
+    const hasExternalTooltip = tooltipId || tooltipIdV1
+    const withoutTooltip =
+      unsafeDisableTooltip || disabled || ariaLabel === undefined || ariaLabel === '' || hasExternalTooltip
+
+    if (withoutTooltip) {
+      return (
+        <ButtonBase
+          icon={Icon}
+          data-component="IconButton"
+          sx={sxStyles}
+          type="button"
+          aria-label={ariaLabel}
+          disabled={disabled}
+          {...props}
+          // @ts-expect-error StyledButton wants both Anchor and Button refs
+          ref={forwardedRef}
+        />
+      )
+    } else {
+      return (
+        <Tooltip
+          ref={forwardedRef}
+          text={description ?? ariaLabel}
+          type={description ? undefined : 'label'}
+          direction={tooltipDirection}
+        >
+          <ButtonBase
+            icon={Icon}
+            data-component="IconButton"
+            sx={sxStyles}
+            type="button"
+            // If description is provided, we will use the tooltip to describe the button, so we need to keep the aria-label to label the button.
+            aria-label={description ? ariaLabel : undefined}
+            {...props}
+          />
+        </Tooltip>
+      )
+    }
+  },
+) as PolymorphicForwardRefComponent<'button' | 'a', IconButtonProps>
 
 export {IconButton}

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

@@ -1,4 +1,4 @@
-import {SearchIcon} from '@primer/octicons-react'
+import {SearchIcon, HeartIcon} from '@primer/octicons-react'
 import {render, screen, fireEvent} from '@testing-library/react'
 import {axe} from 'jest-axe'
 import React from 'react'
@@ -113,4 +113,28 @@ describe('Button', () => {
     const position = screen.getByText('content').compareDocumentPosition(screen.getByTestId('trailingVisual'))
     expect(position).toBe(Node.DOCUMENT_POSITION_FOLLOWING)
   })
+
+  it('should render tooltip on an icon button when unsafeDisableTooltip prop is passed as false', () => {
+    const {getByRole, getByText} = render(
+      <IconButton icon={HeartIcon} aria-label="Heart" unsafeDisableTooltip={false} />,
+    )
+    const triggerEL = getByRole('button')
+    const tooltipEl = getByText('Heart')
+    expect(triggerEL).toHaveAttribute('aria-labelledby', tooltipEl.id)
+  })
+  it('should render description type tooltip on an icon button when unsafeDisableTooltip prop is passed as false', () => {
+    const {getByRole, getByText} = render(
+      <IconButton icon={HeartIcon} aria-label="Heart" description="Love is all around" unsafeDisableTooltip={false} />,
+    )
+    const triggerEL = getByRole('button')
+    expect(triggerEL).toHaveAttribute('aria-label', 'Heart')
+    const tooltipEl = getByText('Love is all around')
+    expect(triggerEL).toHaveAttribute('aria-describedby', tooltipEl.id)
+  })
+  it('should not render tooltip on an icon button by default', () => {
+    const {getByRole} = render(<IconButton icon={HeartIcon} aria-label="Heart" />)
+    const triggerEl = getByRole('button')
+    expect(triggerEl).not.toHaveAttribute('aria-labelledby')
+    expect(triggerEl).toHaveAttribute('aria-label', 'Heart')
+  })
 })

packages/react/src/Button/types.ts

@@ -3,6 +3,7 @@ import styled from 'styled-components'
 import type {SxProp} from '../sx'
 import sx from '../sx'
 import getGlobalFocusStyles from '../internal/utils/getGlobalFocusStyles'
+import type {TooltipDirection} from '../TooltipV2'
 
 export const StyledButton = styled.button<SxProp>`
   ${getGlobalFocusStyles('-2px')};
@@ -77,6 +78,9 @@ export type ButtonProps = {
 
 export type IconButtonProps = ButtonA11yProps & {
   icon: React.ElementType
+  unsafeDisableTooltip?: boolean
+  description?: string
+  tooltipDirection?: TooltipDirection
 } & Omit<ButtonBaseProps, 'aria-label' | 'aria-labelledby'>
 
 // adopted from React.AnchorHTMLAttributes

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

@@ -2,8 +2,9 @@ import React from 'react'
 import type {TooltipProps} from './Tooltip'
 import Tooltip from './Tooltip'
 import {render, renderClasses, rendersClass, behavesAsComponent, checkExports} from '../utils/testing'
-import {render as HTMLRender} from '@testing-library/react'
+import {render as HTMLRender, screen} from '@testing-library/react'
 import {axe} from 'jest-axe'
+import {CodeIcon} from '@primer/octicons-react'
 
 /* Tooltip v1 */
 
@@ -49,4 +50,14 @@ describe('Tooltip', () => {
   it('respects the "wrap" prop', () => {
     expect(rendersClass(<Tooltip wrap />, 'tooltipped-multiline')).toBe(true)
   })
+  it('should label the link', () => {
+    HTMLRender(
+      <Tooltip aria-label="Tooltip text" id="tooltip-unique-id">
+        <a aria-labelledby="tooltip-unique-id" href="#href">
+          <CodeIcon />
+        </a>
+      </Tooltip>,
+    )
+    expect(screen.getByRole('link')).toHaveAccessibleName('Tooltip text')
+  })
 })

packages/react/src/Tooltip/Tooltip.tsx

@@ -1,10 +1,11 @@
 import clsx from 'clsx'
-import React from 'react'
+import React, {useMemo} from 'react'
 import styled from 'styled-components'
 import {get} from '../constants'
 import type {SxProp} from '../sx'
 import sx from '../sx'
 import type {ComponentProps} from '../utils/types'
+import {useId} from '../hooks'
 
 /* Tooltip v1 */
 
@@ -193,18 +194,25 @@ export type TooltipProps = {
   wrap?: boolean
 } & ComponentProps<typeof TooltipBase>
 
-function Tooltip({direction = 'n', children, className, text, noDelay, align, wrap, ...rest}: TooltipProps) {
+export const TooltipContext = React.createContext<{tooltipId?: string}>({})
+function Tooltip({direction = 'n', children, className, text, noDelay, align, wrap, id, ...rest}: TooltipProps) {
+  const tooltipId = useId(id)
   const classes = clsx(
     className,
     `tooltipped-${direction}`,
     align && `tooltipped-align-${align}-2`,
     noDelay && 'tooltipped-no-delay',
     wrap && 'tooltipped-multiline',
   )
+
+  const value = useMemo(() => ({tooltipId}), [tooltipId])
   return (
-    <TooltipBase role="tooltip" aria-label={text} {...rest} className={classes}>
-      {children}
-    </TooltipBase>
+    // This provider is used to check if an icon button is wrapped with tooltip or not.
+    <TooltipContext.Provider value={value}>
+      <TooltipBase role="tooltip" aria-label={text} id={tooltipId} {...rest} className={classes}>
+        {children}
+      </TooltipBase>
+    </TooltipContext.Provider>
   )
 }