Add workaround for Chrome bug where KeybindingHint in combination with aria-labelledby results in incorrect label

.changeset/curvy-ties-breathe.md

@@ -0,0 +1,5 @@
+---
+"@primer/react": patch
+---
+
+Add workaround for Chrome bug where `KeybindingHint` in combination with `aria-labelledby` results in incorrect label

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

@@ -338,7 +338,7 @@ describe('Button', () => {
   })
   it('should append the keyshortcuts to the tooltip text that labels the icon button when keyshortcuts prop is passed', () => {
     const {getByRole} = render(<IconButton icon={HeartIcon} aria-label="Heart" keyshortcuts="Command+H" />)
-    const triggerEl = getByRole('button', {name: 'Heart ( command h )'})
+    const triggerEl = getByRole('button', {name: 'Heart (command h)'})
     expect(triggerEl).toBeInTheDocument()
   })
   it('should render aria-keyshortcuts on an icon button when keyshortcuts prop is passed (Description Type)', () => {
@@ -353,6 +353,6 @@ describe('Button', () => {
       <IconButton icon={HeartIcon} aria-label="Heart" description="Love is all around" keyshortcuts="Command+H" />,
     )
     const triggerEl = getByRole('button', {name: 'Heart'})
-    expect(triggerEl).toHaveAccessibleDescription('Love is all around ( command h )')
+    expect(triggerEl).toHaveAccessibleDescription('Love is all around (command h)')
   })
 })

packages/react/src/KeybindingHint/KeybindingHint.tsx

@@ -26,8 +26,7 @@ export const KeybindingHint = memo(({className, ...props}: KeybindingHintProps)
 KeybindingHint.displayName = 'KeybindingHint'
 
 /**
- * AVOID: `KeybindingHint` is nearly always sufficient for providing both visible and accessible keyboard hints, and
- * will result in a good screen reader experience when used as the target for `aria-describedby` and `aria-labelledby`.
+ * AVOID: `KeybindingHint` is nearly always sufficient for providing both visible and accessible keyboard hints.
  * However, there may be cases where we need a plain string version, such as when building `aria-label` or
  * `aria-description`. In that case, this plain string builder can be used instead.
  *

packages/react/src/TooltipV2/Tooltip.tsx

@@ -1,7 +1,7 @@
 import React, {Children, useEffect, useRef, useState, useMemo} from 'react'
 import type {SxProp} from '../sx'
 import sx from '../sx'
-import {useId, useProvidedRefOrCreate, useOnEscapePress} from '../hooks'
+import {useId, useProvidedRefOrCreate, useOnEscapePress, useIsMacOS} from '../hooks'
 import {invariant} from '../utils/invariant'
 import {warning} from '../utils/warning'
 import styled from 'styled-components'
@@ -13,7 +13,7 @@ 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 {getAccessibleKeybindingHintString, KeybindingHint, type KeybindingHintProps} from '../KeybindingHint'
 import VisuallyHidden from '../_VisuallyHidden'
 import useSafeTimeout from '../hooks/useSafeTimeout'
 
@@ -348,6 +348,8 @@ export const Tooltip = React.forwardRef(
       [isPopoverOpen],
     )
 
+    const isMacOS = useIsMacOS()
+
     return (
       <TooltipContext.Provider value={value}>
         <>
@@ -400,17 +402,24 @@ export const Tooltip = React.forwardRef(
             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}
-            id={tooltipId}
             // 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}
           >
-            {text}
+            <span id={tooltipId}>
+              {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. */}
+              {keybindingHint && (
+                <VisuallyHidden>({getAccessibleKeybindingHintString(keybindingHint, isMacOS)})</VisuallyHidden>
+              )}
+            </span>
             {keybindingHint && (
-              <span className={clsx(classes.KeybindingHintContainer, text && classes.HasTextBefore)}>
-                <VisuallyHidden>(</VisuallyHidden>
+              <span className={clsx(classes.KeybindingHintContainer, text && classes.HasTextBefore)} aria-hidden>
                 <KeybindingHint keys={keybindingHint} format="condensed" variant="onEmphasis" size="small" />
-                <VisuallyHidden>)</VisuallyHidden>
               </span>
             )}
           </StyledTooltip>

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

@@ -38,11 +38,11 @@ describe('Tooltip', () => {
 
   it('renders `data-direction="s"` by default', () => {
     const {getByText} = HTMLRender(<TooltipComponent />)
-    expect(getByText('Tooltip text')).toHaveAttribute('data-direction', 's')
+    expect(getByText('Tooltip text').closest('[role=tooltip]')).toHaveAttribute('data-direction', 's')
   })
   it('renders `data-direction` attribute with the correct value when the `direction` prop is specified', () => {
     const {getByText} = HTMLRender(<TooltipComponent direction="n" />)
-    expect(getByText('Tooltip text')).toHaveAttribute('data-direction', 'n')
+    expect(getByText('Tooltip text').closest('[role=tooltip]')).toHaveAttribute('data-direction', 'n')
   })
   it('should label the trigger element by its tooltip when the tooltip type is label', () => {
     const {getByRole, getByText} = HTMLRender(<TooltipComponent type="label" />)
@@ -52,11 +52,11 @@ describe('Tooltip', () => {
   })
   it('should render aria-hidden on the tooltip element when the tooltip is label type', () => {
     const {getByText} = HTMLRender(<TooltipComponent type="label" />)
-    expect(getByText('Tooltip text')).toHaveAttribute('aria-hidden', 'true')
+    expect(getByText('Tooltip text').parentElement).toHaveAttribute('aria-hidden', 'true')
   })
   it('should render aria-hidden on the tooltip element when the tooltip is description type', () => {
     const {getByText} = HTMLRender(<TooltipComponent type="description" />)
-    expect(getByText('Tooltip text')).toHaveAttribute('aria-hidden', 'true')
+    expect(getByText('Tooltip text').closest('[role=tooltip]')).toHaveAttribute('aria-hidden', 'true')
   })
   it('should describe the trigger element by its tooltip when the tooltip type is description (by default)', () => {
     const {getByRole, getByText} = HTMLRender(<TooltipComponent />)
@@ -66,7 +66,7 @@ describe('Tooltip', () => {
   })
   it('should render the tooltip element with role="tooltip" when the tooltip type is description (by default)', () => {
     const {getByText} = HTMLRender(<TooltipComponent />)
-    expect(getByText('Tooltip text')).toHaveAttribute('role', 'tooltip')
+    expect(getByText('Tooltip text').closest('[role=tooltip]')).toHaveAttribute('role', 'tooltip')
   })
 
   it('should spread the accessibility attributes correctly on the trigger (ActionMenu.Button) when tooltip is used in an action menu', () => {

packages/react/src/__tests__/__snapshots__/TextInput.test.tsx.snap

@@ -3354,11 +3354,14 @@ exports[`TextInput renders trailingAction icon button 1`] = `
       aria-hidden={true}
       className="c5"
       data-direction="s"
-      id=":r2j:"
       onMouseEnter={[Function]}
       onMouseLeave={[Function]}
     >
-      Icon label
+      <span
+        id=":r2j:"
+      >
+        Icon label
+      </span>
     </span>
   </span>
 </span>
@@ -4115,12 +4118,15 @@ exports[`TextInput renders trailingAction text button with a tooltip 1`] = `
       aria-hidden={true}
       className="c6"
       data-direction="s"
-      id=":r2d:"
       onMouseEnter={[Function]}
       onMouseLeave={[Function]}
       role="tooltip"
     >
-      Clear input
+      <span
+        id=":r2d:"
+      >
+        Clear input
+      </span>
     </span>
   </span>
 </span>