diff --git a/src/ActionMenu/ActionMenu.tsx b/src/ActionMenu/ActionMenu.tsx index d579694cd3f..d07d3eab4e5 100644 --- a/src/ActionMenu/ActionMenu.tsx +++ b/src/ActionMenu/ActionMenu.tsx @@ -17,7 +17,7 @@ export type MenuContextProps = Pick< > & { onClose?: (gesture: 'anchor-click' | 'click-outside' | 'escape' | 'tab') => void } -const MenuContext = React.createContext({renderAnchor: null, open: false}) +export const MenuContext = React.createContext({renderAnchor: null, open: false}) export type ActionMenuProps = { /** diff --git a/src/Button/IconButton.features.stories.tsx b/src/Button/IconButton.features.stories.tsx index 0f27d91c397..641033681ab 100644 --- a/src/Button/IconButton.features.stories.tsx +++ b/src/Button/IconButton.features.stories.tsx @@ -1,6 +1,8 @@ -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', @@ -8,6 +10,61 @@ export default { export const Primary = () => +export const WithDescription = () => ( + +) + +export const ExternalTooltip = () => ( + + + +) + +export const AsMenuAnchor = () => ( + + + + + + + + alert('Copy link clicked')}> + Copy link + ⌘C + + alert('Quote reply clicked')}> + Quote reply + ⌘Q + + alert('Edit comment clicked')}> + Edit comment + ⌘E + + + alert('Delete file clicked')}> + Delete file + ⌘D + + + + +) + +export const HasExternalTooltip = () => ( + + + +) + export const Danger = () => export const Invisible = () => diff --git a/src/Button/IconButton.tsx b/src/Button/IconButton.tsx index e313aa1a4e7..3930e1d8734 100644 --- a/src/Button/IconButton.tsx +++ b/src/Button/IconButton.tsx @@ -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 - - ) -}) 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 + + + + ) + } else { + return ( + // If icon button is already wrapped in a tooltip, we do not need to add another tooltip. + + ) + } + }, +) as PolymorphicForwardRefComponent<'button' | 'a', IconButtonProps> export {IconButton} diff --git a/src/Button/__tests__/Button.test.tsx b/src/Button/__tests__/Button.test.tsx index 186f4a029a7..30c415e4084 100644 --- a/src/Button/__tests__/Button.test.tsx +++ b/src/Button/__tests__/Button.test.tsx @@ -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}}) @@ -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( + + + + + + + + + + + alert('Copy link clicked')}> + Copy link + ⌘C + + + + + + + , + ) + + const toggleButton = screen.getByRole('button', {name: 'More actions'}) + await userEvent.click(toggleButton) + expect(screen.getByRole('menu', {name: 'More actions'})).toBeInTheDocument() + }) }) diff --git a/src/Button/types.ts b/src/Button/types.ts index 49ad7969eb9..e17e9fe9c25 100644 --- a/src/Button/types.ts +++ b/src/Button/types.ts @@ -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 = { /** @@ -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 // adopted from React.AnchorHTMLAttributes diff --git a/src/ButtonGroup/ButtonGroup.tsx b/src/ButtonGroup/ButtonGroup.tsx index 9e49b669aca..0278e75aee2 100644 --- a/src/ButtonGroup/ButtonGroup.tsx +++ b/src/ButtonGroup/ButtonGroup.tsx @@ -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 { diff --git a/src/Tooltip/Tooltip.tsx b/src/Tooltip/Tooltip.tsx index a2b1de01bde..9e0301fb668 100644 --- a/src/Tooltip/Tooltip.tsx +++ b/src/Tooltip/Tooltip.tsx @@ -247,6 +247,8 @@ export type TooltipProps = { wrap?: boolean } & ComponentProps +export const TooltipContext = React.createContext<{container?: string}>({}) + function Tooltip({direction = 'n', children, className, text, noDelay, align, wrap, ...rest}: TooltipProps) { const classes = clsx( className, @@ -256,9 +258,12 @@ function Tooltip({direction = 'n', children, className, text, noDelay, align, wr wrap && 'tooltipped-multiline', ) return ( - - {children} - + // This provider is used to check if an icon button is wrapped with tooltip or not. + + + {children} + + ) } diff --git a/src/__tests__/__snapshots__/TextInput.test.tsx.snap b/src/__tests__/__snapshots__/TextInput.test.tsx.snap index 87bdf2dc0ba..975dadffff0 100644 --- a/src/__tests__/__snapshots__/TextInput.test.tsx.snap +++ b/src/__tests__/__snapshots__/TextInput.test.tsx.snap @@ -1824,85 +1824,6 @@ exports[`TextInput renders trailingAction icon button 1`] = ` height: var(--inner-action-size); } -.c0 { - font-size: 14px; - line-height: 20px; - color: var(--fgColor-default,var(--color-fg-default,#1F2328)); - vertical-align: middle; - background-color: var(--bgColor-default,var(--color-canvas-default,#ffffff)); - border: 1px solid var(--control-borderColor-rest,var(--borderColor-default,var(--color-border-default,#d0d7de))); - border-radius: 6px; - outline: none; - box-shadow: var(--shadow-inset,var(--color-primer-shadow-inset,inset 0 1px 0 rgba(208,215,222,0.2))); - display: -webkit-inline-box; - display: -webkit-inline-flex; - display: -ms-inline-flexbox; - display: inline-flex; - -webkit-align-items: stretch; - -webkit-box-align: stretch; - -ms-flex-align: stretch; - align-items: stretch; - min-height: 32px; - overflow: hidden; -} - -.c0 input, -.c0 textarea { - cursor: text; -} - -.c0 select { - cursor: pointer; -} - -.c0::-webkit-input-placeholder { - color: var(--fgColor-muted,var(--color-fg-subtle,#6e7781)); -} - -.c0::-moz-placeholder { - color: var(--fgColor-muted,var(--color-fg-subtle,#6e7781)); -} - -.c0:-ms-input-placeholder { - color: var(--fgColor-muted,var(--color-fg-subtle,#6e7781)); -} - -.c0::placeholder { - color: var(--fgColor-muted,var(--color-fg-subtle,#6e7781)); -} - -.c0 > textarea { - padding: 12px; -} - -.c1 { - background-repeat: no-repeat; - background-position: right 8px center; - padding-left: 0; - padding-right: 0; -} - -.c1 > :not(:last-child) { - margin-right: 8px; -} - -.c1 .TextInput-icon, -.c1 .TextInput-action { - -webkit-align-self: center; - -ms-flex-item-align: center; - align-self: center; - color: var(--fgColor-muted,var(--color-fg-muted,#656d76)); - -webkit-flex-shrink: 0; - -ms-flex-negative: 0; - flex-shrink: 0; -} - -.c1 > input, -.c1 > select { - padding-left: 12px; - padding-right: 0; -} - .c4 { position: relative; display: inline-block; @@ -2128,6 +2049,85 @@ exports[`TextInput renders trailingAction icon button 1`] = ` left: 10px; } +.c0 { + font-size: 14px; + line-height: 20px; + color: var(--fgColor-default,var(--color-fg-default,#1F2328)); + vertical-align: middle; + background-color: var(--bgColor-default,var(--color-canvas-default,#ffffff)); + border: 1px solid var(--control-borderColor-rest,var(--borderColor-default,var(--color-border-default,#d0d7de))); + border-radius: 6px; + outline: none; + box-shadow: var(--shadow-inset,var(--color-primer-shadow-inset,inset 0 1px 0 rgba(208,215,222,0.2))); + display: -webkit-inline-box; + display: -webkit-inline-flex; + display: -ms-inline-flexbox; + display: inline-flex; + -webkit-align-items: stretch; + -webkit-box-align: stretch; + -ms-flex-align: stretch; + align-items: stretch; + min-height: 32px; + overflow: hidden; +} + +.c0 input, +.c0 textarea { + cursor: text; +} + +.c0 select { + cursor: pointer; +} + +.c0::-webkit-input-placeholder { + color: var(--fgColor-muted,var(--color-fg-subtle,#6e7781)); +} + +.c0::-moz-placeholder { + color: var(--fgColor-muted,var(--color-fg-subtle,#6e7781)); +} + +.c0:-ms-input-placeholder { + color: var(--fgColor-muted,var(--color-fg-subtle,#6e7781)); +} + +.c0::placeholder { + color: var(--fgColor-muted,var(--color-fg-subtle,#6e7781)); +} + +.c0 > textarea { + padding: 12px; +} + +.c1 { + background-repeat: no-repeat; + background-position: right 8px center; + padding-left: 0; + padding-right: 0; +} + +.c1 > :not(:last-child) { + margin-right: 8px; +} + +.c1 .TextInput-icon, +.c1 .TextInput-action { + -webkit-align-self: center; + -ms-flex-item-align: center; + align-self: center; + color: var(--fgColor-muted,var(--color-fg-muted,#656d76)); + -webkit-flex-shrink: 0; + -ms-flex-negative: 0; + flex-shrink: 0; +} + +.c1 > input, +.c1 > select { + padding-left: 12px; + padding-right: 0; +} + .c2 { border: 0; font-size: inherit; @@ -2907,85 +2907,6 @@ exports[`TextInput renders trailingAction text button with a tooltip 1`] = ` color: inherit; } -.c0 { - font-size: 14px; - line-height: 20px; - color: var(--fgColor-default,var(--color-fg-default,#1F2328)); - vertical-align: middle; - background-color: var(--bgColor-default,var(--color-canvas-default,#ffffff)); - border: 1px solid var(--control-borderColor-rest,var(--borderColor-default,var(--color-border-default,#d0d7de))); - border-radius: 6px; - outline: none; - box-shadow: var(--shadow-inset,var(--color-primer-shadow-inset,inset 0 1px 0 rgba(208,215,222,0.2))); - display: -webkit-inline-box; - display: -webkit-inline-flex; - display: -ms-inline-flexbox; - display: inline-flex; - -webkit-align-items: stretch; - -webkit-box-align: stretch; - -ms-flex-align: stretch; - align-items: stretch; - min-height: 32px; - overflow: hidden; -} - -.c0 input, -.c0 textarea { - cursor: text; -} - -.c0 select { - cursor: pointer; -} - -.c0::-webkit-input-placeholder { - color: var(--fgColor-muted,var(--color-fg-subtle,#6e7781)); -} - -.c0::-moz-placeholder { - color: var(--fgColor-muted,var(--color-fg-subtle,#6e7781)); -} - -.c0:-ms-input-placeholder { - color: var(--fgColor-muted,var(--color-fg-subtle,#6e7781)); -} - -.c0::placeholder { - color: var(--fgColor-muted,var(--color-fg-subtle,#6e7781)); -} - -.c0 > textarea { - padding: 12px; -} - -.c1 { - background-repeat: no-repeat; - background-position: right 8px center; - padding-left: 0; - padding-right: 0; -} - -.c1 > :not(:last-child) { - margin-right: 8px; -} - -.c1 .TextInput-icon, -.c1 .TextInput-action { - -webkit-align-self: center; - -ms-flex-item-align: center; - align-self: center; - color: var(--fgColor-muted,var(--color-fg-muted,#656d76)); - -webkit-flex-shrink: 0; - -ms-flex-negative: 0; - flex-shrink: 0; -} - -.c1 > input, -.c1 > select { - padding-left: 12px; - padding-right: 0; -} - .c4 { position: relative; display: inline-block; @@ -3212,6 +3133,85 @@ exports[`TextInput renders trailingAction text button with a tooltip 1`] = ` left: 10px; } +.c0 { + font-size: 14px; + line-height: 20px; + color: var(--fgColor-default,var(--color-fg-default,#1F2328)); + vertical-align: middle; + background-color: var(--bgColor-default,var(--color-canvas-default,#ffffff)); + border: 1px solid var(--control-borderColor-rest,var(--borderColor-default,var(--color-border-default,#d0d7de))); + border-radius: 6px; + outline: none; + box-shadow: var(--shadow-inset,var(--color-primer-shadow-inset,inset 0 1px 0 rgba(208,215,222,0.2))); + display: -webkit-inline-box; + display: -webkit-inline-flex; + display: -ms-inline-flexbox; + display: inline-flex; + -webkit-align-items: stretch; + -webkit-box-align: stretch; + -ms-flex-align: stretch; + align-items: stretch; + min-height: 32px; + overflow: hidden; +} + +.c0 input, +.c0 textarea { + cursor: text; +} + +.c0 select { + cursor: pointer; +} + +.c0::-webkit-input-placeholder { + color: var(--fgColor-muted,var(--color-fg-subtle,#6e7781)); +} + +.c0::-moz-placeholder { + color: var(--fgColor-muted,var(--color-fg-subtle,#6e7781)); +} + +.c0:-ms-input-placeholder { + color: var(--fgColor-muted,var(--color-fg-subtle,#6e7781)); +} + +.c0::placeholder { + color: var(--fgColor-muted,var(--color-fg-subtle,#6e7781)); +} + +.c0 > textarea { + padding: 12px; +} + +.c1 { + background-repeat: no-repeat; + background-position: right 8px center; + padding-left: 0; + padding-right: 0; +} + +.c1 > :not(:last-child) { + margin-right: 8px; +} + +.c1 .TextInput-icon, +.c1 .TextInput-action { + -webkit-align-self: center; + -ms-flex-item-align: center; + align-self: center; + color: var(--fgColor-muted,var(--color-fg-muted,#656d76)); + -webkit-flex-shrink: 0; + -ms-flex-negative: 0; + flex-shrink: 0; +} + +.c1 > input, +.c1 > select { + padding-left: 12px; + padding-right: 0; +} + .c2 { border: 0; font-size: inherit; diff --git a/src/drafts/Tooltip/Tooltip.tsx b/src/drafts/Tooltip/Tooltip.tsx index 949ea46c34b..691db05d19b 100644 --- a/src/drafts/Tooltip/Tooltip.tsx +++ b/src/drafts/Tooltip/Tooltip.tsx @@ -2,7 +2,7 @@ import React, {Children, useEffect, useRef, useState} from 'react' import sx, {SxProp} from '../../sx' import {useId, useProvidedRefOrCreate} from '../../hooks' import {invariant} from '../../utils/invariant' -import {warning} from '../../utils/warning' +// import {warning} from '../../utils/warning' import styled from 'styled-components' import {get} from '../../constants' import {ComponentProps} from '../../utils/types' @@ -167,19 +167,16 @@ const positionToDirection: Record = { } // The list is from GitHub's custom-axe-rules https://github.com/github/github/blob/master/app/assets/modules/github/axe-custom-rules.ts#L3 -const interactiveElements = [ - 'a[href]', - 'button:not(:disabled)', - 'summary', - 'select', - 'input:not([type=hidden])', - 'textarea', -] +const interactiveElements = ['a[href]', 'button', 'summary', 'select', 'input:not([type=hidden])', 'textarea'] const isInteractive = (element: HTMLElement) => { return ( - interactiveElements.some(selector => element.matches(selector)) || - (element.hasAttribute('role') && element.getAttribute('role') === 'button') + interactiveElements.some(selector => { + if (element.tagName === 'BUTTON') { + return !element.hasAttribute('disabled') + } else return element.matches(selector) + }) || + (element.hasAttribute('role') && element.getAttribute('role') === 'button' && !element.hasAttribute('disabled')) ) } export const TooltipContext = React.createContext<{tooltipId?: string}>({}) @@ -189,6 +186,7 @@ export const Tooltip = React.forwardRef( const tooltipId = useId() const child = Children.only(children) const triggerRef = useProvidedRefOrCreate(forwardedRef as React.RefObject) + const [iconBtnAnchor, setIconBtnAnchor] = useState(false) const tooltipElRef = useRef(null) const [calculatedDirection, setCalculatedDirection] = useState(direction) @@ -220,16 +218,19 @@ export const Tooltip = React.forwardRef( 'The `Tooltip` component expects a single React element that contains interactive content. Consider using a ` + , + ) + }).toThrow( + 'The `Tooltip` component expects a single React element that contains interactive content. Consider using a `