Skip to content
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
8a507be
IconButton: introduce tooltip behaviuor behind a prop
broccolinisoup Feb 6, 2024
3169fe4
changeset add
broccolinisoup Feb 6, 2024
71c9b6f
add comment
broccolinisoup Feb 6, 2024
d6f26f1
Merge branch 'main' into icon-button-tooltip-feature
broccolinisoup Feb 8, 2024
7ca980a
Merge branch 'main' into icon-button-tooltip-feature
broccolinisoup Feb 9, 2024
bb3c92a
rename the tooltip prop
broccolinisoup Feb 9, 2024
2c75e8c
rename the prop again 🙃
broccolinisoup Feb 9, 2024
b30e480
remove redundant stories
broccolinisoup Feb 9, 2024
841c517
clean up the mess
broccolinisoup Feb 9, 2024
20a0c4f
test(vrt): update snapshots
broccolinisoup Feb 9, 2024
ed6759d
Merge branch 'main' into icon-button-tooltip-feature
broccolinisoup Feb 14, 2024
8c5a010
toolbar button disable tooltip
broccolinisoup Feb 14, 2024
9604383
Merge branch 'main' into icon-button-tooltip-feature
broccolinisoup Feb 21, 2024
6841b54
icon button as menu anchor & external tooltip & tooltipDirection
broccolinisoup Feb 21, 2024
ce78708
update snaps
broccolinisoup Feb 21, 2024
12132a2
Merge branch 'main' into icon-button-tooltip-feature
broccolinisoup Feb 26, 2024
480b6aa
Merge branch 'main' into icon-button-tooltip-feature
broccolinisoup Mar 1, 2024
290e33e
code review comments
broccolinisoup Mar 1, 2024
f8ad63a
show tooltip only on focus-visible
broccolinisoup Mar 5, 2024
7be1721
Merge branch 'main' into icon-button-tooltip-feature
broccolinisoup Mar 5, 2024
c20bb37
catch focus-visible for jestdom
siddharthkp Mar 6, 2024
11e60b4
rename the prop and update the default version to be true
broccolinisoup Mar 8, 2024
8c48b2f
fix tests
broccolinisoup Mar 8, 2024
bb428b7
already default true
broccolinisoup Mar 8, 2024
579cab5
test(vrt): update snapshots
broccolinisoup Mar 8, 2024
b3f4f26
improve test and example
broccolinisoup Mar 8, 2024
c990fc9
Merge branch 'main' into icon-button-tooltip-feature
broccolinisoup Mar 13, 2024
2a8d931
Merge branch 'main' into icon-button-tooltip-feature
broccolinisoup Apr 5, 2024
414c662
test(vrt): update snapshots
broccolinisoup Apr 5, 2024
8ba386f
Merge branch 'main' into icon-button-tooltip-feature
broccolinisoup Apr 8, 2024
5b5fa94
fix linting
broccolinisoup Apr 8, 2024
a44ab67
Merge branch 'main' into icon-button-tooltip-feature
broccolinisoup Apr 11, 2024
a4cef3b
pass the id down and add test
broccolinisoup Apr 12, 2024
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
5 changes: 5 additions & 0 deletions .changeset/smart-chefs-fail.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@primer/react': minor
---

IconButton: introduce tooltip behavior behind the prop `_enableTooltip_`
Comment thread
broccolinisoup marked this conversation as resolved.
Outdated
Comment thread
broccolinisoup marked this conversation as resolved.
Outdated
15 changes: 14 additions & 1 deletion packages/react/src/Button/IconButton.features.stories.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import {HeartIcon} from '@primer/octicons-react'
import {HeartIcon, InboxIcon} from '@primer/octicons-react'
import React from 'react'
import {IconButton} from '.'

Expand All @@ -19,3 +19,16 @@ export const Small = () => <IconButton size="small" icon={HeartIcon} aria-label=
export const Medium = () => <IconButton size="medium" icon={HeartIcon} aria-label="Favorite" />

export const Large = () => <IconButton size="large" icon={HeartIcon} aria-label="Favorite" />

export const TooltipEnabled = () => (
<IconButton _enableTooltip_={true} icon={HeartIcon} variant="primary" aria-label="Favorite" />
)

export const WithDescription = () => (
<IconButton
_enableTooltip_={true}
icon={InboxIcon}
aria-label="Notifications"
description="You have no unread notifications."
/>
)
66 changes: 54 additions & 12 deletions packages/react/src/Button/IconButton.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,20 +4,62 @@ import {ForwardRefComponent as PolymorphicForwardRefComponent} from '../utils/po
import {ButtonBase} from './ButtonBase'
import {defaultSxProp} from '../utils/defaultSxProp'
import {generateCustomSxProp} from './Button'
import {Tooltip} from '../drafts/Tooltip/Tooltip'

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,
'aria-label': ariaLabel,
description,
disabled,
// This is a temporary prop to incrementally introduce tooltips to the IconButton component.
_enableTooltip_ = false,
...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)
}

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>
const displayTooltip = _enableTooltip_ && !disabled && ariaLabel

if (displayTooltip) {
return (
<Tooltip ref={forwardedRef} text={description ?? ariaLabel} type={description ? undefined : 'label'}>
<ButtonBase
icon={Icon}
data-component="IconButton"
sx={sxStyles}
type="button"
// 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.
aria-label={description ? ariaLabel : undefined}
{...props}
/>
</Tooltip>
)
} else {
return (
<ButtonBase
icon={Icon}
data-component="IconButton"
sx={sxStyles}
type="button"
aria-label={ariaLabel}
disabled={disabled}
{...props}
// @ts-expect-error StyledButton wants both Anchor and Button refs
ref={forwardedRef}
/>
)
}
},
) as PolymorphicForwardRefComponent<'button' | 'a', IconButtonProps>

export {IconButton}
18 changes: 17 additions & 1 deletion packages/react/src/Button/__tests__/Button.test.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import {SearchIcon} from '@primer/octicons-react'
import {SearchIcon, HeartIcon} from '@primer/octicons-react'
import {render, screen, fireEvent} from '@testing-library/react'
import {axe} from 'jest-axe'
import React from 'react'
Expand Down Expand Up @@ -113,4 +113,20 @@ describe('Button', () => {
const position = screen.getByText('content').compareDocumentPosition(screen.getByTestId('trailingVisual'))
expect(position).toBe(Node.DOCUMENT_POSITION_FOLLOWING)
})

it('should render tooltip on an icon button when the _enableTooltip_ prop is passed true', () => {
const {getByRole, getByText} = render(<IconButton icon={HeartIcon} aria-label="Heart" _enableTooltip_={true} />)
const triggerEL = getByRole('button')
const tooltipEl = getByText('Heart')
expect(triggerEL).toHaveAttribute('aria-labelledby', tooltipEl.id)
})
it('should render description type tooltip on an icon button when the description prop is provided and the _enableTooltip_ prop is passed true', () => {
const {getByRole, getByText} = render(
<IconButton icon={HeartIcon} aria-label="Heart" description="Love is all around" _enableTooltip_={true} />,
)
const triggerEL = getByRole('button')
expect(triggerEL).toHaveAttribute('aria-label', 'Heart')
const tooltipEl = getByText('Love is all around')
expect(triggerEL).toHaveAttribute('aria-describedby', tooltipEl.id)
})
})
2 changes: 2 additions & 0 deletions packages/react/src/Button/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,8 @@ export type ButtonProps = {

export type IconButtonProps = ButtonA11yProps & {
icon: React.ElementType
_enableTooltip_?: boolean
description?: string
} & Omit<ButtonBaseProps, 'aria-label' | 'aria-labelledby'>

// adopted from React.AnchorHTMLAttributes
Expand Down