Icon button fixes: Removes iconLabel and adds aria-label to the type

.changeset/large-owls-dance.md

@@ -0,0 +1,5 @@
+---
+"@primer/react": patch
+---
+
+Icon button fixes: Removes iconLabel and adds aria-label to the type

docs/content/Button.mdx

@@ -2,58 +2,58 @@
 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
 <Button>Default</Button>
 ```
 
 ### Danger button
 
 The `danger` variant of `Button` is used to warn users about potentially destructive actions
 
-```jsx live drafts
+```jsx live
 <Button variant="danger">Danger</Button>
 ```
 
 ### Outline button
 
 The `outline` variant of `Button` is typically used as a secondary button
 
-```jsx live drafts
+```jsx live
 <Button variant="outline">Outline</Button>
 ```
 
 ### Invisible button
 
 The `invisible` variant of `Button` indicates that the action is a low priority one.
 
-```jsx live drafts
+```jsx live
 <Button variant="invisible">Invisible</Button>
 ```
 
 ### Different sized buttons
 
 `Button` component supports three different sizes. `small`, `medium`, `large`.
 
-```jsx live drafts
+```jsx live
 <>
  <Button size="small">Search</Button>
  <Button sx={{mt: 2}}>Search</Button>
@@ -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
 <>
  <Button leadingIcon={SearchIcon}>Search</Button>
  <Button trailingIcon={SearchIcon} sx={{mt: 2}}>
@@ -84,25 +84,19 @@ It is recommended to use an octicon here.
 
 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
-<IconButton icon={SearchIcon}>Search</IconButton>
+```jsx live
+<IconButton aria-label="Search" icon={SearchIcon} />
 ```
 
 ### Different sized icon buttons
 
 `IconButton` also supports the three different sizes. `small`, `medium`, `large`.
 
-```jsx live drafts
+```jsx live
 <>
- <IconButton icon={SearchIcon} size="small">
- Search
- </IconButton>
- <IconButton sx={{ml: 2}} icon={SearchIcon}>
- Search
- </IconButton>
- <IconButton sx={{ml: 2}} icon={SearchIcon} size="large">
- Search
- </IconButton>
+ <IconButton aria-label="Search" size="small" icon={SearchIcon} />
+ <IconButton aria-label="Search" icon={SearchIcon} sx={{ml: 2}} />
+ <IconButton aria-label="Search" size="large" icon={SearchIcon} sx={{ml: 2}} />
 </>
 ```
 
@@ -112,7 +106,7 @@ A common use case for primer is a button with a counter component which shows th
 We provide `Button.Counter` as a composite component which requires you to provide a number as child.
 The counter will match the `variant` styles of the parent button.
 
-```jsx live drafts
+```jsx live
 <Button>
  Watch
  <Button.Counter>1</Button.Counter>
@@ -124,7 +118,7 @@ The counter will match the `variant` styles of the parent button.
 A button can be styled in an appropriate manner using the `sx` prop. This may be to change width, or to add margins etc.
 Here's an example of a block button which takes 100% of available width. Checkout [styled-system](https://styled-system.com/) to see what you can send in an `sx` prop.
 
-```jsx live drafts
+```jsx live
 <Button sx={{width: '100%'}}>Block</Button>
 ```
 

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
-<IconButton icon={SearchIcon}>Search</IconButton>
+```jsx live
+<IconButton aria-label="Search" icon={SearchIcon} />
 ```
 
 ### Different sized icon buttons
 
 `IconButton` also supports the three different sizes. `small`, `medium`, `large`.
 
-```jsx live drafts
+```jsx live
 <>
- <IconButton icon={SearchIcon} size="small">
- Search
- </IconButton>
- <IconButton sx={{ml: 2}} icon={SearchIcon}>
- Search
- </IconButton>
- <IconButton sx={{ml: 2}} icon={SearchIcon} size="large">
- Search
- </IconButton>
+ <IconButton aria-label="Search" icon={SearchIcon} size="small" />
+ <IconButton aria-label="Search" sx={{ml: 2}} icon={SearchIcon} />
+ <IconButton aria-label="Search" sx={{ml: 2}} icon={SearchIcon} size="large" />
 </>
 ```
 

src/Button/Button2.stories.tsx → 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 (
  <>
  <Box mb={2}>
- <IconButton icon={XIcon} iconLabel="Close" {...args} />
+ <IconButton icon={XIcon} aria-label="Close" {...args} />
  </Box>
  <Box mb={2}>
- <IconButton icon={XIcon} iconLabel="Close" {...args} variant="invisible" sx={{mt: 2}} />
+ <IconButton icon={XIcon} aria-label="Close" {...args} variant="invisible" sx={{mt: 2}} />
  </Box>
  <Box mb={2}>
- <IconButton icon={XIcon} iconLabel="Close" {...args} variant="danger" />
+ <IconButton icon={XIcon} aria-label="Close" {...args} variant="danger" />
  </Box>
  <Box mb={2}>
- <IconButton icon={XIcon} iconLabel="Close" {...args} variant="primary" />
+ <IconButton icon={XIcon} aria-label="Close" {...args} variant="primary" />
  </Box>
  <Box mb={2}>
- <IconButton icon={XIcon} iconLabel="Close" {...args} variant="outline" />
+ <IconButton icon={XIcon} aria-label="Close" {...args} variant="outline" />
  </Box>
  </>
  )
@@ -193,7 +193,7 @@ export const DisabledButton = ({...args}: ButtonProps) => {
  </Button>
  </Box>
  <Box mb={2}>
- <IconButton disabled icon={() => <XIcon />} iconLabel="Close" {...args} />
+ <IconButton disabled icon={() => <XIcon />} aria-label="Close" {...args} />
  </Box>
  </>
  )

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<HTMLButtonElement, IconButtonProps>((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<HTMLButtonElement, IconButtonProps>((props, forwar
  sxProp as SxProp
  ])
  return (
- <StyledButton aria-labelledby={iconLabelId} sx={sxStyles} {...rest} ref={forwardedRef}>
- {iconLabel && (
- <span id={iconLabelId} hidden={true}>
- {iconLabel}
- </span>
- )}
+ <StyledButton sx={sxStyles} {...rest} ref={forwardedRef}>
  <Box as="span" sx={{display: 'inline-block'}}>
  <Icon />
  </Box>

src/Button/types.ts

@@ -11,6 +11,8 @@ export type Size = 'small' | 'medium' | 'large'
 
 type StyledButtonProps = ComponentPropsWithRef<typeof StyledButton>
 
+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<IconProps>
- iconLabel: string
 } & ButtonBaseProps
 
 // adopted from React.AnchorHTMLAttributes

src/__tests__/Button.test.tsx

@@ -10,9 +10,9 @@ expect.extend(toHaveNoViolations)
 describe('Button', () => {
  behavesAsComponent({Component: Button, options: {skipAs: true}})
 
- it('renders a <button>', async () => {
+ it('renders a <button>', () => {
  const container = render(<Button>Default</Button>)
- 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(<Button onClick={onClick}>Noop</Button>)
- 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(<Button sx={{width: 200}}>Block</Button>)
- 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(
  <Button onClick={onClick} disabled>
  Disabled
  </Button>
  )
- 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(<Button size="small">Smol</Button>)
- 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(
  <Button size="small" sx={{fontSize: 20}}>
  Big Smol
  </Button>
  )
- 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(<Button variant="primary">Primary</Button>)
- 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(<Button variant="invisible">Invisible</Button>)
- 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(<Button variant="danger">Danger</Button>)
- 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(<Button variant="outline">Outline</Button>)
- 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(<IconButton icon={SearchIcon} iconLabel="Search icon only button" />)
- const IconOnlyButton = await container.findByRole('button')
+ it('styles icon only button to make it a square', () => {
+ const container = render(<IconButton icon={SearchIcon} aria-label="Search button" />)
+ 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(<IconButton icon={SearchIcon} aria-label="Search button" />)
+ const IconOnlyButton = container.getByLabelText('Search button')
+ expect(IconOnlyButton).toBeTruthy()
  })
 })