Skip to content

Commit 2317ca8

Browse files
owenniblockCopilot
andauthored
Fixes issue with Tooltip description id overriding existing description ids (#6200)
Co-authored-by: Copilot <[email protected]>
1 parent 4cca0e9 commit 2317ca8

File tree

4 files changed

+52
-2
lines changed

4 files changed

+52
-2
lines changed

.changeset/two-aliens-wait.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@primer/react': patch
3+
---
4+
5+
Fixes issue with Tooltip description id overriding existing description ids

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

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import {IconButton, Button, Box, Link, ActionMenu, ActionList} from '..'
1+
import {IconButton, Button, Box, Link, ActionMenu, ActionList, VisuallyHidden} from '..'
22
import Octicon from '../Octicon'
33
import {Tooltip} from './Tooltip'
44
import {SearchIcon, BookIcon, CheckIcon, TriangleDownIcon, GitBranchIcon, InfoIcon} from '@primer/octicons-react'
@@ -35,6 +35,16 @@ export const DescriptionType = () => (
3535
</Box>
3636
)
3737

38+
// As a supplementary description for a button
39+
export const DescriptionTypeWithExternalDescription = () => (
40+
<Box sx={{p: 5}}>
41+
<Tooltip text="Supplementary text" direction="n">
42+
<Button aria-describedby="external-description">Save</Button>
43+
</Tooltip>
44+
<VisuallyHidden id="external-description">External description</VisuallyHidden>
45+
</Box>
46+
)
47+
3848
// As a supplementary description for an IconButton
3949
export const IconButtonWithDescription = () => (
4050
<Box sx={{p: 5}}>

packages/react/src/TooltipV2/Tooltip.tsx

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -247,7 +247,21 @@ export const Tooltip = React.forwardRef(
247247
React.cloneElement(child as React.ReactElement<TriggerPropsType>, {
248248
ref: triggerRef,
249249
// If it is a type description, we use tooltip to describe the trigger
250-
'aria-describedby': type === 'description' ? tooltipId : child.props['aria-describedby'],
250+
'aria-describedby': (() => {
251+
// If tooltip is not a description type, keep the original aria-describedby
252+
if (type !== 'description') {
253+
return child.props['aria-describedby']
254+
}
255+
256+
// If tooltip is a description type, append our tooltipId
257+
const existingDescribedBy = child.props['aria-describedby']
258+
if (existingDescribedBy) {
259+
return `${existingDescribedBy} ${tooltipId}`
260+
}
261+
262+
// If no existing aria-describedby, use our tooltipId
263+
return tooltipId
264+
})(),
251265
// If it is a label type, we use tooltip to label the trigger
252266
'aria-labelledby': type === 'label' ? tooltipId : child.props['aria-labelledby'],
253267
onBlur: (event: React.FocusEvent) => {

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

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,15 @@ const TooltipComponent = (props: Omit<TooltipProps, 'text'> & {text?: string}) =
1313
</Tooltip>
1414
)
1515

16+
const TooltipComponentWithExistingDescription = (props: Omit<TooltipProps, 'text'> & {text?: string}) => (
17+
<>
18+
<span id="external-description">External description</span>
19+
<Tooltip text="Tooltip text" {...props}>
20+
<Button aria-describedby="external-description">Button Text</Button>
21+
</Tooltip>
22+
</>
23+
)
24+
1625
function ExampleWithActionMenu(actionMenuTrigger: React.ReactElement): JSX.Element {
1726
return (
1827
<ThemeProvider theme={theme}>
@@ -162,4 +171,16 @@ describe('Tooltip', () => {
162171
)
163172
expect(getByRole('button', {name: 'Overridden label'})).toBeInTheDocument()
164173
})
174+
175+
it('should append tooltip id to existing aria-describedby value on the trigger element', () => {
176+
const {getByRole, getByText} = HTMLRender(<TooltipComponentWithExistingDescription />)
177+
const triggerEL = getByRole('button')
178+
const tooltipEl = getByText('Tooltip text')
179+
const externalDescription = getByText('External description')
180+
181+
// Check that aria-describedby contains both the external description ID and the tooltip ID
182+
const describedBy = triggerEL.getAttribute('aria-describedby')
183+
expect(describedBy).toContain(externalDescription.id)
184+
expect(describedBy).toContain(tooltipEl.id)
185+
})
165186
})

0 commit comments

Comments
 (0)