diff --git a/.changeset/large-owls-dance.md b/.changeset/large-owls-dance.md new file mode 100644 index 00000000000..0c218546dc7 --- /dev/null +++ b/.changeset/large-owls-dance.md @@ -0,0 +1,5 @@ +--- +"@primer/react": patch +--- + +Icon button fixes: Removes iconLabel and adds aria-label to the type diff --git a/docs/content/Button.mdx b/docs/content/Button.mdx index b31522104a6..eeb8c234b7b 100644 --- a/docs/content/Button.mdx +++ b/docs/content/Button.mdx @@ -2,26 +2,26 @@ componentId: button title: Button status: Alpha -source: https://github.com/primer/react/tree/main/src/Button2 -storybook: '/react/storybook?path=/story/composite-components-button2' +source: https://github.com/primer/react/tree/main/src/Button +storybook: '/react/storybook?path=/story/composite-components-button' description: Use button for the main actions on a page or form. --- -import {Button, IconButton, LinkButton} from '@primer/react/drafts' +import {Button, IconButton, LinkButton} from '@primer/react' ## Usage ### Installation ```js -import {Button} from '@primer/react/drafts' +import {Button} from '@primer/react' ``` ### Default button This is the default variant for the `Button` component. -```jsx live drafts +```jsx live ``` @@ -29,7 +29,7 @@ This is the default variant for the `Button` component. The `danger` variant of `Button` is used to warn users about potentially destructive actions -```jsx live drafts +```jsx live ``` @@ -37,7 +37,7 @@ The `danger` variant of `Button` is used to warn users about potentially destruc The `outline` variant of `Button` is typically used as a secondary button -```jsx live drafts +```jsx live ``` @@ -45,7 +45,7 @@ The `outline` variant of `Button` is typically used as a secondary button The `invisible` variant of `Button` indicates that the action is a low priority one. -```jsx live drafts +```jsx live ``` @@ -53,7 +53,7 @@ The `invisible` variant of `Button` indicates that the action is a low priority `Button` component supports three different sizes. `small`, `medium`, `large`. -```jsx live drafts +```jsx live <> @@ -68,7 +68,7 @@ The `invisible` variant of `Button` indicates that the action is a low priority We can place an icon inside the `Button` in either the leading or the trailing position to enhance the visual context. It is recommended to use an octicon here. -```jsx live drafts +```jsx live <> ``` diff --git a/docs/content/IconButton.mdx b/docs/content/IconButton.mdx index 0edea279283..2af6b14024b 100644 --- a/docs/content/IconButton.mdx +++ b/docs/content/IconButton.mdx @@ -2,8 +2,8 @@ title: IconButton componentId: icon_button status: Alpha -source: https://github.com/primer/react/tree/main/src/Button2 -storybook: '/react/storybook?path=/story/composite-components-button2' +source: https://github.com/primer/react/tree/main/src/Button +storybook: '/react/storybook?path=/story/composite-components-button' description: An accessible button component with no text and only icon. --- @@ -12,32 +12,26 @@ description: An accessible button component with no text and only icon. ### Installation ```js -import {IconButton} from '@primer/react/drafts' +import {IconButton} from '@primer/react' ``` ### Icon only button A separate component called `IconButton` is used if the action shows only an icon with no text. This button will remain square in shape. -```jsx live drafts -Search +```jsx live + ``` ### Different sized icon buttons `IconButton` also supports the three different sizes. `small`, `medium`, `large`. -```jsx live drafts +```jsx live <> - - Search - - - Search - - - Search - + + + ``` diff --git a/src/Button/Button2.stories.tsx b/src/Button/Button.stories.tsx similarity index 92% rename from src/Button/Button2.stories.tsx rename to src/Button/Button.stories.tsx index a4ae611a3ba..800915dc3a9 100644 --- a/src/Button/Button2.stories.tsx +++ b/src/Button/Button.stories.tsx @@ -6,7 +6,7 @@ import {BaseStyles, ThemeProvider} from '..' import Box from '../Box' export default { - title: 'Composite components/Button2', + title: 'Composite components/Button', decorators: [ Story => { @@ -75,19 +75,19 @@ export const iconButton = ({...args}: ButtonProps) => { return ( <> - + - + - + - + - + ) @@ -193,7 +193,7 @@ export const DisabledButton = ({...args}: ButtonProps) => { - } iconLabel="Close" {...args} /> + } aria-label="Close" {...args} /> ) diff --git a/src/Button/IconButton.tsx b/src/Button/IconButton.tsx index 2b7ad955e48..401650829fc 100644 --- a/src/Button/IconButton.tsx +++ b/src/Button/IconButton.tsx @@ -4,11 +4,9 @@ import {useTheme} from '../ThemeProvider' import Box from '../Box' import {IconButtonProps, StyledButton} from './types' import {getBaseStyles, getSizeStyles, getVariantStyles} from './styles' -import {useSSRSafeId} from '@react-aria/ssr' const IconButton = forwardRef((props, forwardedRef): JSX.Element => { - const {variant = 'default', size = 'medium', sx: sxProp = {}, icon: Icon, iconLabel, ...rest} = props - const iconLabelId = useSSRSafeId() + const {variant = 'default', size = 'medium', sx: sxProp = {}, icon: Icon, ...rest} = props const {theme} = useTheme() const sxStyles = merge.all([ getBaseStyles(theme), @@ -17,12 +15,7 @@ const IconButton = forwardRef((props, forwar sxProp as SxProp ]) return ( - - {iconLabel && ( - - )} + diff --git a/src/Button/types.ts b/src/Button/types.ts index 9f391f7b122..10c7188bc1a 100644 --- a/src/Button/types.ts +++ b/src/Button/types.ts @@ -11,6 +11,8 @@ export type Size = 'small' | 'medium' | 'large' type StyledButtonProps = ComponentPropsWithRef +type ButtonA11yProps = {'aria-label': string; 'aria-labelby'?: never} | {'aria-label'?: never; 'aria-labelby': string} + export type ButtonBaseProps = { /** * Determine's the styles on a button one of 'default' | 'primary' | 'invisible' | 'danger' @@ -40,12 +42,8 @@ export type ButtonProps = { children: React.ReactNode } & ButtonBaseProps -export type IconButtonProps = { - /** - * This is to be used if it is an icon-only button. Will make text visually hidden - */ +export type IconButtonProps = ButtonA11yProps & { icon: React.FunctionComponent - iconLabel: string } & ButtonBaseProps // adopted from React.AnchorHTMLAttributes diff --git a/src/__tests__/Button.test.tsx b/src/__tests__/Button.test.tsx index 4da60203806..7e917445f9e 100644 --- a/src/__tests__/Button.test.tsx +++ b/src/__tests__/Button.test.tsx @@ -10,9 +10,9 @@ expect.extend(toHaveNoViolations) describe('Button', () => { behavesAsComponent({Component: Button, options: {skipAs: true}}) - it('renders a ) - const button = await container.findByRole('button') + const button = container.getByRole('button') expect(button.textContent).toEqual('Default') }) @@ -23,76 +23,82 @@ describe('Button', () => { cleanup() }) - it('preserves "onClick" prop', async () => { + it('preserves "onClick" prop', () => { const onClick = jest.fn() const container = render() - const button = await container.findByRole('button') + const button = container.getByRole('button') fireEvent.click(button) expect(onClick).toHaveBeenCalledTimes(1) }) - it('respects width props', async () => { + it('respects width props', () => { const container = render() - const button = await container.findByRole('button') + const button = container.getByRole('button') expect(button).toHaveStyleRule('width', '200px') }) - it('respects the "disabled" prop', async () => { + it('respects the "disabled" prop', () => { const onClick = jest.fn() const container = render( ) - const button = await container.findByRole('button') + const button = container.getByRole('button') expect(button.hasAttribute('disabled')).toEqual(true) fireEvent.click(button) expect(onClick).toHaveBeenCalledTimes(0) }) - it('respects the "variant" prop', async () => { + it('respects the "variant" prop', () => { const container = render() - const button = await container.findByRole('button') + const button = container.getByRole('button') expect(button).toHaveStyleRule('font-size', '12px') }) - it('respects the "fontSize" prop over the "variant" prop', async () => { + it('respects the "fontSize" prop over the "variant" prop', () => { const container = render( ) - const button = await container.findByRole('button') + const button = container.getByRole('button') expect(button).toHaveStyleRule('font-size', '20px') }) - it('styles primary button appropriately', async () => { + it('styles primary button appropriately', () => { const container = render() - const button = await container.findByRole('button') + const button = container.getByRole('button') expect(button).toMatchSnapshot() }) - it('styles invisible button appropriately', async () => { + it('styles invisible button appropriately', () => { const container = render() - const button = await container.findByRole('button') + const button = container.getByRole('button') expect(button).toMatchSnapshot() }) - it('styles danger button appropriately', async () => { + it('styles danger button appropriately', () => { const container = render() - const button = await container.findByRole('button') + const button = container.getByRole('button') expect(button).toMatchSnapshot() }) - it('styles outline button appropriately', async () => { + it('styles outline button appropriately', () => { const container = render() - const button = await container.findByRole('button') + const button = container.getByRole('button') expect(button).toMatchSnapshot() }) - it('styles icon only button to make it a square', async () => { - const container = render() - const IconOnlyButton = await container.findByRole('button') + it('styles icon only button to make it a square', () => { + const container = render() + const IconOnlyButton = container.getByRole('button') expect(IconOnlyButton).toHaveStyleRule('padding-right', '8px') + expect(IconOnlyButton).toMatchSnapshot() + }) + it('makes sure icon button has an aria-label', () => { + const container = render() + const IconOnlyButton = container.getByLabelText('Search button') + expect(IconOnlyButton).toBeTruthy() }) }) diff --git a/src/__tests__/__snapshots__/Button.test.tsx.snap b/src/__tests__/__snapshots__/Button.test.tsx.snap index 6055d1e1a3d..48a7b73120c 100644 --- a/src/__tests__/__snapshots__/Button.test.tsx.snap +++ b/src/__tests__/__snapshots__/Button.test.tsx.snap @@ -221,6 +221,111 @@ exports[`Button styles danger button appropriately 1`] = ` `; +exports[`Button styles icon only button to make it a square 1`] = ` +.c1 { + display: inline-block; +} + +.c0 { + border-radius: 2; + border: 1px solid; + font-family: inherit; + font-weight: bold; + line-height: 20px; + white-space: nowrap; + vertical-align: middle; + cursor: pointer; + -webkit-appearance: none; + -moz-appearance: none; + appearance: none; + -webkit-user-select: none; + -moz-user-select: none; + -ms-user-select: none; + user-select: none; + -webkit-text-decoration: none; + text-decoration: none; + text-align: center; + padding-top: 5px; + padding-bottom: 5px; + padding-left: 8px; + padding-right: 8px; + font-size: 14px; + color: btn.text; + background-color: btn.bg; + box-shadow: undefined,undefined; +} + +.c0:focus { + outline: none; +} + +.c0:disabled { + cursor: default; + color: primer.fg.disabled; +} + +.c0:disabled [data-component=ButtonCounter] { + color: inherit; +} + +.c0:disabled svg { + opacity: 0.6; +} + +.c0 [data-component=ButtonCounter] { + font-size: 14px; +} + +.c0:hover:not([disabled]) { + background-color: btn.hoverBg; +} + +.c0:focus:not([disabled]) { + box-shadow: undefined; +} + +.c0:active:not([disabled]) { + background-color: btn.activeBg; + border-color: btn.activeBorder; +} + +.c0[aria-expanded=true] { + background-color: btn.activeBg; + border-color: btn.activeBorder; +} + +@media (forced-colors:active) { + .c0:focus { + outline: solid 1px transparent; + } +} + + +`; + exports[`Button styles invisible button appropriately 1`] = ` .c0 { border-radius: 2; diff --git a/src/stories/Tooltip.stories.tsx b/src/stories/Tooltip.stories.tsx new file mode 100644 index 00000000000..59d034282f1 --- /dev/null +++ b/src/stories/Tooltip.stories.tsx @@ -0,0 +1,37 @@ +import React from 'react' +import {Meta} from '@storybook/react' +import {BaseStyles, ThemeProvider, IconButton} from '..' +import Box from '../Box' +import Tooltip from '../Tooltip' +import {SearchIcon} from '@primer/octicons-react' + +export default { + title: 'Tooltip/Default', + component: Tooltip, + + decorators: [ + Story => { + return ( + + + + + + ) + } + ] +} as Meta + +export const TextTooltip = () => ( + + Text with a tooltip + +) + +export const IconButtonTooltip = () => ( + + + + + +)