Skip to content

IconButton: Add tooltip to icon buttons by default #4046

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 19 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/ActionMenu/ActionMenu.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ export type MenuContextProps = Pick<
> & {
onClose?: (gesture: 'anchor-click' | 'click-outside' | 'escape' | 'tab') => void
}
const MenuContext = React.createContext<MenuContextProps>({renderAnchor: null, open: false})
export const MenuContext = React.createContext<MenuContextProps>({renderAnchor: null, open: false})

export type ActionMenuProps = {
/**
Expand Down
59 changes: 58 additions & 1 deletion src/Button/IconButton.features.stories.tsx
Original file line number Diff line number Diff line change
@@ -1,13 +1,70 @@
import {HeartIcon} from '@primer/octicons-react'
import {InboxIcon, HeartIcon, ChevronDownIcon} from '@primer/octicons-react'
import React from 'react'
import {IconButton} from '.'
import {Tooltip} from '../drafts/Tooltip'
import {ActionMenu, ActionList} from '..'

export default {
title: 'Components/IconButton/Features',
}

export const Primary = () => <IconButton 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 AsMenuAnchor = () => (
<ActionMenu>
<ActionMenu.Anchor>
<IconButton icon={ChevronDownIcon} aria-label="Open Menu" />
</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>
)

export const HasExternalTooltip = () => (
<Tooltip
text="Notifications"
direction="e"
className="custom-class"
sx={
{
/* custom styling */
}
}
>
<IconButton icon={InboxIcon} aria-label="Notifications" />
</Tooltip>
)

export const Danger = () => <IconButton icon={HeartIcon} variant="danger" aria-label="Favorite" />

export const Invisible = () => <IconButton icon={HeartIcon} variant="invisible" aria-label="Favorite" />
Expand Down
80 changes: 68 additions & 12 deletions src/Button/IconButton.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,20 +4,76 @@ import {ForwardRefComponent as PolymorphicForwardRefComponent} from '../utils/po
import {ButtonBase} from './ButtonBase'
import {defaultSxProp} from '../utils/defaultSxProp'
import {generateCustomSxProp} from './Button'
import {Tooltip, TooltipContext} from '../drafts/Tooltip/Tooltip'
import {TooltipContext as TooltipV1Context} from '../Tooltip/Tooltip'
import {MenuContext} from '../ActionMenu/ActionMenu'

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,
disabled,
'aria-label': ariaLabel,
description,
hideTooltip = true,
...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)
}
// If the icon button is already wrapped in a tooltip, do not add one.
const {tooltipId} = React.useContext(TooltipContext) // Tooltip v2
const {container} = React.useContext(TooltipV1Context) // Tooltip v1
const {anchorRef, anchorId} = React.useContext(MenuContext)

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>
// we need to know if the icon button is used as a menu anchor to render the tooltip and aria properties correctly.
const isIconButtonAnchor = anchorRef !== undefined && anchorId !== undefined
// aria-label is going to be deprecated in favor of label but for now we are supporting both.
const iconButtonLabel = ariaLabel
if (!tooltipId && container !== 'Tooltip' && !disabled && !hideTooltip) {
return (
// if description exists, we use tooltip for adding description to the icon button. Otherwise, we use tooltip for labelling the icon button.
// @ts-ignore for now
<Tooltip text={description ?? iconButtonLabel} type={description ? 'description' : 'label'} ref={forwardedRef}>
<ButtonBase
icon={Icon}
data-component="IconButton"
data-anchor={isIconButtonAnchor ? 'true' : undefined}
sx={sxStyles}
type="button"
{...props}
// If description exists, aria-label will be used to label the icon button whereas tooltip will be used to describe the button.
// Is icon button is used as a menu anchor, aria-label will be used to label the icon button and the menu.)
aria-label={description || isIconButtonAnchor ? iconButtonLabel : undefined}
/>
</Tooltip>
)
} else {
return (
// If icon button is already wrapped in a tooltip, we do not need to add another tooltip.
<ButtonBase
icon={Icon}
data-component="IconButton"
data-anchor={isIconButtonAnchor ? 'true' : undefined}
sx={sxStyles}
disabled={disabled}
type="button"
{...props}
// @ts-expect-error StyledButton wants both Anchor and Button refs
ref={forwardedRef}
// External tooltip can only be used to set description. Icon button should always have aria-label or label
aria-label={iconButtonLabel}
/>
)
}
},
) as PolymorphicForwardRefComponent<'button' | 'a', IconButtonProps>

export {IconButton}
32 changes: 31 additions & 1 deletion src/Button/__tests__/Button.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,9 @@ import {axe} from 'jest-axe'
import React from 'react'
import {IconButton, Button} from '../../Button'
import {behavesAsComponent} from '../../utils/testing'

import {ActionMenu, theme, ActionList, BaseStyles, ThemeProvider, SSRProvider} from '../..'
import userEvent from '@testing-library/user-event'
// import theme from '../../'
describe('Button', () => {
behavesAsComponent({Component: Button, options: {skipSx: true}})

Expand Down Expand Up @@ -113,4 +115,32 @@ describe('Button', () => {
const position = screen.getByText('content').compareDocumentPosition(screen.getByTestId('trailingVisual'))
expect(position).toBe(Node.DOCUMENT_POSITION_FOLLOWING)
})
it('should render the anchor icon button and correctly name the menu', async () => {
render(
<ThemeProvider theme={theme}>
<SSRProvider>
<BaseStyles>
<ActionMenu>
<ActionMenu.Anchor>
<IconButton icon={SearchIcon} aria-label="More actions" />
</ActionMenu.Anchor>

<ActionMenu.Overlay width="medium">
<ActionList>
<ActionList.Item onSelect={() => alert('Copy link clicked')}>
Copy link
<ActionList.TrailingVisual>⌘C</ActionList.TrailingVisual>
</ActionList.Item>
</ActionList>
</ActionMenu.Overlay>
</ActionMenu>
</BaseStyles>
</SSRProvider>
</ThemeProvider>,
)

const toggleButton = screen.getByRole('button', {name: 'More actions'})
await userEvent.click(toggleButton)
expect(screen.getByRole('menu', {name: 'More actions'})).toBeInTheDocument()
})
})
19 changes: 16 additions & 3 deletions src/Button/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,20 @@ export type Size = 'small' | 'medium' | 'large'

export type AlignContent = 'start' | 'center'

type ButtonA11yProps =
| {'aria-label': string; 'aria-labelledby'?: undefined}
| {'aria-label'?: undefined; 'aria-labelledby': string}
type ButtonA11yProps = (
| {
// @deprecated use label prop instead
'aria-label': string
'aria-labelledby'?: undefined
}
| {
// @deprecated use label prop instead
'aria-label'?: undefined
'aria-labelledby': string
}
) & {
description?: string
}

export type ButtonBaseProps = {
/**
Expand Down Expand Up @@ -76,6 +87,8 @@ export type ButtonProps = {

export type IconButtonProps = ButtonA11yProps & {
icon: React.ElementType
// default to true until the major version bump
hideTooltip?: boolean
} & Omit<ButtonBaseProps, 'aria-label' | 'aria-labelledby'>

// adopted from React.AnchorHTMLAttributes
Expand Down
7 changes: 3 additions & 4 deletions src/ButtonGroup/ButtonGroup.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,21 +8,20 @@ const ButtonGroup = styled.div`
vertical-align: middle;
isolation: isolate;

&& > * {
&& > button {
margin-inline-end: -1px;
position: relative;
border-radius: 0;

:first-child {
:first-of-type {
border-top-left-radius: ${get('radii.2')};
border-bottom-left-radius: ${get('radii.2')};
}

:last-child {
:last-of-type {
border-top-right-radius: ${get('radii.2')};
border-bottom-right-radius: ${get('radii.2')};
}

:focus,
:active,
:hover {
Expand Down
11 changes: 8 additions & 3 deletions src/Tooltip/Tooltip.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -247,6 +247,8 @@ export type TooltipProps = {
wrap?: boolean
} & ComponentProps<typeof TooltipBase>

export const TooltipContext = React.createContext<{container?: string}>({})

function Tooltip({direction = 'n', children, className, text, noDelay, align, wrap, ...rest}: TooltipProps) {
const classes = clsx(
className,
Expand All @@ -256,9 +258,12 @@ function Tooltip({direction = 'n', children, className, text, noDelay, align, wr
wrap && 'tooltipped-multiline',
)
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={{container: 'Tooltip'}}>
<TooltipBase role="tooltip" aria-label={text} {...rest} className={classes}>
{children}
</TooltipBase>
</TooltipContext.Provider>
)
}

Expand Down
Loading