Skip to content

Commit 8e9267f

Browse files
broccolinisoupsiddharthkp
broccolinisoup
and
siddharthkp
authored
IconButton: introduce tooltips on icon buttons behind the unsafeDisableTooltip 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]>
1 parent 8a2486d commit 8e9267f

16 files changed

+325
-39
lines changed

‎.changeset/smart-chefs-fail.md

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

‎.playwright/snapshots/components/Dialogv1.test.ts-snapshots/Dialogv1-Default-dark-high-contrast-linux.png

-1 Bytes
Loading

‎packages/react/src/ActionMenu/ActionMenu.tsx

+14-2
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import React from 'react'
1+
import React, {useEffect, useState} from 'react'
22
import {TriangleDownIcon} from '@primer/octicons-react'
33
import type {AnchoredOverlayProps} from '../AnchoredOverlay'
44
import {AnchoredOverlay} from '../AnchoredOverlay'
@@ -147,6 +147,17 @@ const Overlay: React.FC<React.PropsWithChildren<MenuOverlayProps>> = ({
147147
const containerRef = React.useRef<HTMLDivElement>(null)
148148
useMenuKeyboardNavigation(open, onClose, containerRef, anchorRef)
149149

150+
// If the menu anchor is an icon button, we need to label the menu by tooltip that also labelled the anchor.
151+
const [anchorAriaLabelledby, setAnchorAriaLabelledby] = useState<null | string>(null)
152+
useEffect(() => {
153+
if (anchorRef.current) {
154+
const ariaLabelledby = anchorRef.current.getAttribute('aria-labelledby')
155+
if (ariaLabelledby) {
156+
setAnchorAriaLabelledby(ariaLabelledby)
157+
}
158+
}
159+
}, [anchorRef])
160+
150161
return (
151162
<AnchoredOverlay
152163
anchorRef={anchorRef}
@@ -165,7 +176,8 @@ const Overlay: React.FC<React.PropsWithChildren<MenuOverlayProps>> = ({
165176
value={{
166177
container: 'ActionMenu',
167178
listRole: 'menu',
168-
listLabelledBy: ariaLabelledby || anchorId,
179+
// 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.
180+
listLabelledBy: ariaLabelledby || anchorAriaLabelledby || anchorId,
169181
selectionAttribute: 'aria-checked', // Should this be here?
170182
afterSelect: onClose,
171183
}}

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

+17-2
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,14 @@ export default {
77
}
88

99
export const CustomSize = () => (
10-
<IconButton aria-label="Expand" variant="primary" size="small" icon={ChevronDownIcon} sx={{width: 16, height: 16}} />
10+
<IconButton
11+
aria-label="Expand"
12+
variant="primary"
13+
size="small"
14+
icon={ChevronDownIcon}
15+
unsafeDisableTooltip={false}
16+
sx={{width: 16, height: 16}}
17+
/>
1118
)
1219

1320
export const CustomSizeWithMedia = () => {
@@ -17,11 +24,19 @@ export const CustomSizeWithMedia = () => {
1724
variant="primary"
1825
size="small"
1926
icon={ChevronDownIcon}
27+
unsafeDisableTooltip={false}
2028
sx={{'@media (min-width: 123px)': {width: 16, height: 16}}}
2129
/>
2230
)
2331
}
2432

2533
export const CustomIconColor = () => (
26-
<IconButton aria-label="Expand" variant="invisible" size="small" icon={ChevronDownIcon} sx={{color: 'red'}} />
34+
<IconButton
35+
aria-label="Expand"
36+
variant="invisible"
37+
size="small"
38+
icon={ChevronDownIcon}
39+
unsafeDisableTooltip={false}
40+
sx={{color: 'red'}}
41+
/>
2742
)

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

+77-8
Original file line numberDiff line numberDiff line change
@@ -1,21 +1,90 @@
1-
import {HeartIcon} from '@primer/octicons-react'
1+
import {HeartIcon, InboxIcon, ChevronDownIcon} from '@primer/octicons-react'
22
import React from 'react'
33
import {IconButton} from '.'
4+
import {ActionMenu} from '../ActionMenu'
5+
import {ActionList} from '../ActionList'
6+
import {Tooltip} from '../TooltipV2'
7+
import {default as TooltipV1} from '../Tooltip'
48

59
export default {
610
title: 'Components/IconButton/Features',
711
}
812

9-
export const Primary = () => <IconButton icon={HeartIcon} variant="primary" aria-label="Favorite" />
13+
export const Primary = () => (
14+
<IconButton icon={HeartIcon} variant="primary" aria-label="Favorite" unsafeDisableTooltip={false} />
15+
)
1016

11-
export const Danger = () => <IconButton icon={HeartIcon} variant="danger" aria-label="Favorite" />
17+
export const Danger = () => (
18+
<IconButton icon={HeartIcon} variant="danger" aria-label="Favorite" unsafeDisableTooltip={false} />
19+
)
1220

13-
export const Invisible = () => <IconButton icon={HeartIcon} variant="invisible" aria-label="Favorite" />
21+
export const Invisible = () => (
22+
<IconButton icon={HeartIcon} variant="invisible" aria-label="Favorite" unsafeDisableTooltip={false} />
23+
)
1424

15-
export const Disabled = () => <IconButton disabled icon={HeartIcon} aria-label="Favorite" />
25+
export const Disabled = () => (
26+
<IconButton disabled icon={HeartIcon} aria-label="Favorite" unsafeDisableTooltip={false} />
27+
)
1628

17-
export const Small = () => <IconButton size="small" icon={HeartIcon} aria-label="Favorite" />
29+
export const Small = () => (
30+
<IconButton size="small" icon={HeartIcon} aria-label="Favorite" unsafeDisableTooltip={false} />
31+
)
1832

19-
export const Medium = () => <IconButton size="medium" icon={HeartIcon} aria-label="Favorite" />
33+
export const Medium = () => (
34+
<IconButton size="medium" icon={HeartIcon} aria-label="Favorite" unsafeDisableTooltip={false} />
35+
)
2036

21-
export const Large = () => <IconButton size="large" icon={HeartIcon} aria-label="Favorite" />
37+
export const Large = () => (
38+
<IconButton size="large" icon={HeartIcon} aria-label="Favorite" unsafeDisableTooltip={false} />
39+
)
40+
41+
export const WithDescription = () => (
42+
<IconButton
43+
icon={InboxIcon}
44+
aria-label="Notifications"
45+
description="You have no unread notifications."
46+
unsafeDisableTooltip={false}
47+
/>
48+
)
49+
50+
export const ExternalTooltip = () => (
51+
<Tooltip text="this is a supportive description for icon button" direction="se">
52+
<IconButton icon={HeartIcon} aria-label="HeartIcon" />
53+
</Tooltip>
54+
)
55+
56+
export const ExternalTooltipVersion1 = () => (
57+
<TooltipV1 text="this is a supportive description for icon button" direction="se">
58+
<IconButton icon={HeartIcon} aria-label="HeartIcon" />
59+
</TooltipV1>
60+
)
61+
62+
export const AsAMenuAnchor = () => (
63+
<ActionMenu>
64+
<ActionMenu.Anchor>
65+
<IconButton icon={ChevronDownIcon} aria-label="Something" unsafeDisableTooltip={false} />
66+
</ActionMenu.Anchor>
67+
68+
<ActionMenu.Overlay width="medium">
69+
<ActionList>
70+
<ActionList.Item onSelect={() => alert('Copy link clicked')}>
71+
Copy link
72+
<ActionList.TrailingVisual>⌘C</ActionList.TrailingVisual>
73+
</ActionList.Item>
74+
<ActionList.Item onSelect={() => alert('Quote reply clicked')}>
75+
Quote reply
76+
<ActionList.TrailingVisual>⌘Q</ActionList.TrailingVisual>
77+
</ActionList.Item>
78+
<ActionList.Item onSelect={() => alert('Edit comment clicked')}>
79+
Edit comment
80+
<ActionList.TrailingVisual>⌘E</ActionList.TrailingVisual>
81+
</ActionList.Item>
82+
<ActionList.Divider />
83+
<ActionList.Item variant="danger" onSelect={() => alert('Delete file clicked')}>
84+
Delete file
85+
<ActionList.TrailingVisual>⌘D</ActionList.TrailingVisual>
86+
</ActionList.Item>
87+
</ActionList>
88+
</ActionMenu.Overlay>
89+
</ActionMenu>
90+
)

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

+1-1
Original file line numberDiff line numberDiff line change
@@ -46,4 +46,4 @@ Playground.args = {
4646
icon: HeartIcon,
4747
}
4848

49-
export const Default = () => <IconButton icon={HeartIcon} aria-label="Favorite" />
49+
export const Default = () => <IconButton icon={HeartIcon} aria-label="Favorite" unsafeDisableTooltip={false} />

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

+67-12
Original file line numberDiff line numberDiff line change
@@ -4,20 +4,75 @@ import type {ForwardRefComponent as PolymorphicForwardRefComponent} from '../uti
44
import {ButtonBase} from './ButtonBase'
55
import {defaultSxProp} from '../utils/defaultSxProp'
66
import {generateCustomSxProp} from './Button'
7+
import {TooltipContext, Tooltip} from '../TooltipV2/Tooltip'
8+
import {TooltipContext as TooltipContextV1} from '../Tooltip/Tooltip'
79

8-
const IconButton = forwardRef(({sx: sxProp = defaultSxProp, icon: Icon, ...props}, forwardedRef): JSX.Element => {
9-
let sxStyles = sxProp
10-
// grap the button props that have associated data attributes in the styles
11-
const {size} = props
10+
const IconButton = forwardRef(
11+
(
12+
{
13+
sx: sxProp = defaultSxProp,
14+
icon: Icon,
15+
'aria-label': ariaLabel,
16+
description,
17+
disabled,
18+
tooltipDirection,
19+
// This is planned to be a temporary prop until the default tooltip on icon buttons are fully rolled out.
20+
unsafeDisableTooltip = true,
21+
...props
22+
},
23+
forwardedRef,
24+
): JSX.Element => {
25+
let sxStyles = sxProp
26+
// grap the button props that have associated data attributes in the styles
27+
const {size} = props
1228

13-
if (sxProp !== null && Object.keys(sxProp).length > 0) {
14-
sxStyles = generateCustomSxProp({size}, sxProp)
15-
}
29+
if (sxProp !== null && Object.keys(sxProp).length > 0) {
30+
sxStyles = generateCustomSxProp({size}, sxProp)
31+
}
1632

17-
return (
18-
// @ts-expect-error StyledButton wants both Anchor and Button refs
19-
<ButtonBase icon={Icon} data-component="IconButton" sx={sxStyles} type="button" {...props} ref={forwardedRef} />
20-
)
21-
}) as PolymorphicForwardRefComponent<'button' | 'a', IconButtonProps>
33+
// If the icon button is already wrapped in a tooltip, do not add one.
34+
const {tooltipId} = React.useContext(TooltipContext) // Tooltip v2
35+
const {tooltipId: tooltipIdV1} = React.useContext(TooltipContextV1) // Tooltip v1
36+
37+
const hasExternalTooltip = tooltipId || tooltipIdV1
38+
const withoutTooltip =
39+
unsafeDisableTooltip || disabled || ariaLabel === undefined || ariaLabel === '' || hasExternalTooltip
40+
41+
if (withoutTooltip) {
42+
return (
43+
<ButtonBase
44+
icon={Icon}
45+
data-component="IconButton"
46+
sx={sxStyles}
47+
type="button"
48+
aria-label={ariaLabel}
49+
disabled={disabled}
50+
{...props}
51+
// @ts-expect-error StyledButton wants both Anchor and Button refs
52+
ref={forwardedRef}
53+
/>
54+
)
55+
} else {
56+
return (
57+
<Tooltip
58+
ref={forwardedRef}
59+
text={description ?? ariaLabel}
60+
type={description ? undefined : 'label'}
61+
direction={tooltipDirection}
62+
>
63+
<ButtonBase
64+
icon={Icon}
65+
data-component="IconButton"
66+
sx={sxStyles}
67+
type="button"
68+
// 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.
69+
aria-label={description ? ariaLabel : undefined}
70+
{...props}
71+
/>
72+
</Tooltip>
73+
)
74+
}
75+
},
76+
) as PolymorphicForwardRefComponent<'button' | 'a', IconButtonProps>
2277

2378
export {IconButton}

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

+25-1
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import {SearchIcon} from '@primer/octicons-react'
1+
import {SearchIcon, HeartIcon} from '@primer/octicons-react'
22
import {render, screen, fireEvent} from '@testing-library/react'
33
import {axe} from 'jest-axe'
44
import React from 'react'
@@ -113,4 +113,28 @@ describe('Button', () => {
113113
const position = screen.getByText('content').compareDocumentPosition(screen.getByTestId('trailingVisual'))
114114
expect(position).toBe(Node.DOCUMENT_POSITION_FOLLOWING)
115115
})
116+
117+
it('should render tooltip on an icon button when unsafeDisableTooltip prop is passed as false', () => {
118+
const {getByRole, getByText} = render(
119+
<IconButton icon={HeartIcon} aria-label="Heart" unsafeDisableTooltip={false} />,
120+
)
121+
const triggerEL = getByRole('button')
122+
const tooltipEl = getByText('Heart')
123+
expect(triggerEL).toHaveAttribute('aria-labelledby', tooltipEl.id)
124+
})
125+
it('should render description type tooltip on an icon button when unsafeDisableTooltip prop is passed as false', () => {
126+
const {getByRole, getByText} = render(
127+
<IconButton icon={HeartIcon} aria-label="Heart" description="Love is all around" unsafeDisableTooltip={false} />,
128+
)
129+
const triggerEL = getByRole('button')
130+
expect(triggerEL).toHaveAttribute('aria-label', 'Heart')
131+
const tooltipEl = getByText('Love is all around')
132+
expect(triggerEL).toHaveAttribute('aria-describedby', tooltipEl.id)
133+
})
134+
it('should not render tooltip on an icon button by default', () => {
135+
const {getByRole} = render(<IconButton icon={HeartIcon} aria-label="Heart" />)
136+
const triggerEl = getByRole('button')
137+
expect(triggerEl).not.toHaveAttribute('aria-labelledby')
138+
expect(triggerEl).toHaveAttribute('aria-label', 'Heart')
139+
})
116140
})

‎packages/react/src/Button/types.ts

+4
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import styled from 'styled-components'
33
import type {SxProp} from '../sx'
44
import sx from '../sx'
55
import getGlobalFocusStyles from '../internal/utils/getGlobalFocusStyles'
6+
import type {TooltipDirection} from '../TooltipV2'
67

78
export const StyledButton = styled.button<SxProp>`
89
${getGlobalFocusStyles('-2px')};
@@ -77,6 +78,9 @@ export type ButtonProps = {
7778

7879
export type IconButtonProps = ButtonA11yProps & {
7980
icon: React.ElementType
81+
unsafeDisableTooltip?: boolean
82+
description?: string
83+
tooltipDirection?: TooltipDirection
8084
} & Omit<ButtonBaseProps, 'aria-label' | 'aria-labelledby'>
8185

8286
// adopted from React.AnchorHTMLAttributes

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

+12-1
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,9 @@ import React from 'react'
22
import type {TooltipProps} from './Tooltip'
33
import Tooltip from './Tooltip'
44
import {render, renderClasses, rendersClass, behavesAsComponent, checkExports} from '../utils/testing'
5-
import {render as HTMLRender} from '@testing-library/react'
5+
import {render as HTMLRender, screen} from '@testing-library/react'
66
import {axe} from 'jest-axe'
7+
import {CodeIcon} from '@primer/octicons-react'
78

89
/* Tooltip v1 */
910

@@ -49,4 +50,14 @@ describe('Tooltip', () => {
4950
it('respects the "wrap" prop', () => {
5051
expect(rendersClass(<Tooltip wrap />, 'tooltipped-multiline')).toBe(true)
5152
})
53+
it('should label the link', () => {
54+
HTMLRender(
55+
<Tooltip aria-label="Tooltip text" id="tooltip-unique-id">
56+
<a aria-labelledby="tooltip-unique-id" href="#href">
57+
<CodeIcon />
58+
</a>
59+
</Tooltip>,
60+
)
61+
expect(screen.getByRole('link')).toHaveAccessibleName('Tooltip text')
62+
})
5263
})

‎packages/react/src/Tooltip/Tooltip.tsx

+13-5
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,11 @@
11
import clsx from 'clsx'
2-
import React from 'react'
2+
import React, {useMemo} from 'react'
33
import styled from 'styled-components'
44
import {get} from '../constants'
55
import type {SxProp} from '../sx'
66
import sx from '../sx'
77
import type {ComponentProps} from '../utils/types'
8+
import {useId} from '../hooks'
89

910
/* Tooltip v1 */
1011

@@ -193,18 +194,25 @@ export type TooltipProps = {
193194
wrap?: boolean
194195
} & ComponentProps<typeof TooltipBase>
195196

196-
function Tooltip({direction = 'n', children, className, text, noDelay, align, wrap, ...rest}: TooltipProps) {
197+
export const TooltipContext = React.createContext<{tooltipId?: string}>({})
198+
function Tooltip({direction = 'n', children, className, text, noDelay, align, wrap, id, ...rest}: TooltipProps) {
199+
const tooltipId = useId(id)
197200
const classes = clsx(
198201
className,
199202
`tooltipped-${direction}`,
200203
align && `tooltipped-align-${align}-2`,
201204
noDelay && 'tooltipped-no-delay',
202205
wrap && 'tooltipped-multiline',
203206
)
207+
208+
const value = useMemo(() => ({tooltipId}), [tooltipId])
204209
return (
205-
<TooltipBase role="tooltip" aria-label={text} {...rest} className={classes}>
206-
{children}
207-
</TooltipBase>
210+
// This provider is used to check if an icon button is wrapped with tooltip or not.
211+
<TooltipContext.Provider value={value}>
212+
<TooltipBase role="tooltip" aria-label={text} id={tooltipId} {...rest} className={classes}>
213+
{children}
214+
</TooltipBase>
215+
</TooltipContext.Provider>
208216
)
209217
}
210218

0 commit comments

Comments
 (0)