Skip to content

Commit 1fdf0eb

Browse files
mperrottimperrotti
committed
Merge branch 'main' of github.com:primer/react into action-list-break-word
2 parents 2872d33 + a215b06 commit 1fdf0eb

File tree

20 files changed

+336
-43
lines changed

20 files changed

+336
-43
lines changed

‎.changeset/smart-chefs-fail.md‎

Lines changed: 7 additions & 0 deletions
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.

‎.github/workflows/deploy_production.yml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ jobs:
5151
name: Production
5252
needs: [guard]
5353
if: ${{ needs.guard.outputs.should_deploy == 'true' }}
54-
uses: primer/.github/.github/workflows/deploy.yml@bb0b18fb589038a93a49a24624be0559c01ed0b9
54+
uses: primer/.github/.github/workflows/deploy.yml@5d641c4a200cecaddcb067822ea54dd17ae0a821
5555
secrets:
5656
gh_token: ${{ secrets.GITHUB_TOKEN }}
5757
with:

‎.github/workflows/release.yml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ jobs:
5252
release-next-major:
5353
if: github.ref_name == 'next-major'
5454
name: Next major
55-
uses: primer/.github/.github/workflows/release_with_app.yml@main
55+
uses: primer/.github/.github/workflows/release_with_app.yml@v2.0.0
5656
with:
5757
title: Release tracking (next major)
5858
secrets:

‎.github/workflows/release_candidate.yml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ jobs:
6767
release-candidate-next-major:
6868
if: github.ref_name == 'changeset-release/next-major'
6969
name: Candidate (@next-major)
70-
uses: primer/.github/.github/workflows/release_candidate.yml@main
70+
uses: primer/.github/.github/workflows/release_candidate.yml@v2.0.0
7171
with:
7272
tag: next-major
7373
secrets:

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

-1 Bytes
Loading

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

Lines changed: 14 additions & 2 deletions
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‎

Lines changed: 17 additions & 2 deletions
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‎

Lines changed: 77 additions & 8 deletions
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‎

Lines changed: 1 addition & 1 deletion
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‎

Lines changed: 67 additions & 12 deletions
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}

0 commit comments

Comments
 (0)