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

…1945)

* Icon button fixes removes iconLabel and adds aria-label to the type

* Update test

* Fix stories and tests. Also add tooltip story

* Fix aria label types

* Update docs from drafts to main

* Create large-owls-dance.md

* update icon only example in Button.mdx

* Update docs from drafts to main

Co-authored-by: Siddharth Kshetrapal <siddharthkp@github.com>

.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()
   })
 })