IconButton: introduce tooltips on icon buttons behind the unsafeDisableTooltip prop for an incremental rollout

.changeset/smart-chefs-fail.md

@@ -0,0 +1,7 @@
+---
+'@primer/react': minor
+---
+
+IconButton: introduce tooltips on icon buttons by default with an option to disable with `disableTooltip` prop when necessary.
+
+This change is considered potentially breaking due to introducing a new behaviour by default. Please upgrade with caution.

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.features.stories.tsx

@@ -1,6 +1,10 @@
-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',
@@ -19,3 +23,53 @@ export const Small = () => <IconButton size="small" icon={HeartIcon} aria-label=
 export const Medium = () => <IconButton size="medium" icon={HeartIcon} aria-label="Favorite" />
 
 export const Large = () => <IconButton size="large" icon={HeartIcon} aria-label="Favorite" />
+
+export const DisabledTooltip = () => (
+  <IconButton disableTooltip={true} icon={HeartIcon} variant="primary" aria-label="Favorite" />
+)
+
+export const WithDescription = () => (
+  <IconButton icon={InboxIcon} aria-label="Notifications" description="You have no unread notifications." />
+)
+
+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" />
+    </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.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.
+      disableTooltip = false,
+      ...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 =
+      disableTooltip || 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,26 @@ 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 by default', () => {
+    const {getByRole, getByText} = render(<IconButton icon={HeartIcon} aria-label="Heart" />)
+    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 by default', () => {
+    const {getByRole, getByText} = render(
+      <IconButton icon={HeartIcon} aria-label="Heart" description="Love is all around" />,
+    )
+    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 when disableTooltip prop is passed as true', () => {
+    const {getByRole} = render(<IconButton icon={HeartIcon} aria-label="Heart" disableTooltip={true} />)
+    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
+  disableTooltip?: boolean
+  description?: string
+  tooltipDirection?: TooltipDirection
 } & Omit<ButtonBaseProps, 'aria-label' | 'aria-labelledby'>
 
 // adopted from React.AnchorHTMLAttributes

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} {...rest} className={classes}>
+        {children}
+      </TooltipBase>
+    </TooltipContext.Provider>
   )
 }
 

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

@@ -1,7 +1,8 @@
-import React from 'react'
+import React, {useState, useCallback, useRef} from 'react'
 import {Button, IconButton, Breadcrumbs, ActionMenu, ActionList, NavList} from '..'
 import {PageHeader} from '../PageHeader'
 import {Tooltip} from './Tooltip'
+import {Dialog} from '../drafts'
 import {
   GitBranchIcon,
   KebabHorizontalIcon,
@@ -186,3 +187,37 @@ export const Hyperlist = () => (
     </NavList.Item>
   </NavList>
 )
+
+export const DialogTrigger = () => {
+  const [isOpen, setIsOpen] = useState(false)
+  const [secondOpen, setSecondOpen] = useState(false)
+  const buttonRef = useRef<HTMLButtonElement>(null)
+  const onDialogClose = useCallback(() => setIsOpen(false), [])
+  const onSecondDialogClose = useCallback(() => setSecondOpen(false), [])
+  const openSecondDialog = useCallback(() => setSecondOpen(true), [])
+  return (
+    <>
+      <Tooltip text="Ready to merge">
+        <IconButton ref={buttonRef} onClick={() => setIsOpen(!isOpen)} icon={CheckIcon} aria-label="Check mark" />
+      </Tooltip>
+      {isOpen && (
+        <Dialog
+          title="My Dialog"
+          onClose={onDialogClose}
+          footerButtons={[
+            {buttonType: 'default', content: 'Open Second Dialog', onClick: openSecondDialog},
+            {buttonType: 'danger', content: 'Delete the universe', onClick: onDialogClose},
+            {buttonType: 'primary', content: 'Proceed', onClick: openSecondDialog, autoFocus: true},
+          ]}
+        >
+          Dialog content
+          {secondOpen && (
+            <Dialog title="Inner dialog!" onClose={onSecondDialogClose} width="small">
+              Hello world
+            </Dialog>
+          )}
+        </Dialog>
+      )}
+    </>
+  )
+}

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

@@ -3,7 +3,7 @@ import {Button, Box} from '..'
 import {Tooltip} from './Tooltip'
 import type {Meta, StoryFn} from '@storybook/react'
 
-export default {
+const meta: Meta<typeof Tooltip> = {
   title: 'Components/TooltipV2/Playground',
   component: Tooltip,
 
@@ -22,7 +22,9 @@ export default {
       control: false,
     },
   },
-} as Meta<typeof Tooltip>
+}
+
+export default meta
 
 // Description type, north direction by default
 export const Playground: StoryFn = args => {