Skip to content

Commit

Permalink
fix(octicons_react): remove role if aria-hidden (#1007)
Browse files Browse the repository at this point in the history
* chore(project): set yarn version

* fix(octicons_react): remove role if aria-hidden

* chore: add changeset

* test: update snapshots
  • Loading branch information
joshblack authored Mar 13, 2024
1 parent dc333cc commit 79b9395
Show file tree
Hide file tree
Showing 6 changed files with 34 additions and 7 deletions.
5 changes: 5 additions & 0 deletions .changeset/spicy-actors-act.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@primer/octicons-react': patch
---

Update octicons in React to no longer set role="img" if the icon is aria-hidden
2 changes: 1 addition & 1 deletion lib/octicons_react/__tests__/tree-shaking.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,5 +50,5 @@ test('tree shaking single export', async () => {
})

const bundleSize = Buffer.byteLength(output[0].code.trim()) / 1000
expect(`${bundleSize}kB`).toMatchInlineSnapshot(`"3.47kB"`)
expect(`${bundleSize}kB`).toMatchInlineSnapshot(`"3.563kB"`)
})
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ exports[`An icon component matches snapshot 1`] = `
fill="currentColor"
focusable="false"
height="16"
role="img"
style="display: inline-block; user-select: none; vertical-align: text-bottom; overflow: visible;"
viewBox="0 0 16 16"
width="16"
Expand Down
24 changes: 22 additions & 2 deletions lib/octicons_react/src/__tests__/octicon.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import '@testing-library/jest-dom'
import {render} from '@testing-library/react'
import {render, screen} from '@testing-library/react'
import React from 'react'
import {AlertIcon} from '../index'

Expand All @@ -9,9 +9,29 @@ describe('An icon component', () => {
expect(container.querySelector('svg')).toMatchSnapshot()
})

it('defaults to `aria-hidden="true"` if no label is present', () => {
const {container} = render(<AlertIcon />)
expect(container.querySelector('svg')).toHaveAttribute('aria-hidden', 'true')
})

it('sets `role="img"` if `aria-label` is provided', () => {
render(<AlertIcon aria-label="Alert" />)
expect(screen.getByLabelText('Alert')).toHaveAttribute('role', 'img')
})

it('sets `role="img"` if `aria-labelledby` is provided', () => {
render(
<>
<span id="label">Alert</span>
<AlertIcon aria-labelledby="label" />
</>
)
expect(screen.getByLabelText('Alert')).toHaveAttribute('role', 'img')
})

it('sets aria-hidden="false" if ariaLabel prop is present', () => {
const {container} = render(<AlertIcon aria-label="icon" />)
expect(container.querySelector('svg')).toHaveAttribute('aria-hidden', 'false')
expect(container.querySelector('svg')).not.toHaveAttribute('aria-hidden')
expect(container.querySelector('svg')).toHaveAttribute('aria-label', 'icon')
})

Expand Down
6 changes: 4 additions & 2 deletions lib/octicons_react/src/createIconComponent.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,17 +30,19 @@ export function createIconComponent(name, defaultClassName, getSVGData) {
const naturalWidth = svgDataByHeight[naturalHeight].width
const width = height * (naturalWidth / naturalHeight)
const path = svgDataByHeight[naturalHeight].path
const labelled = ariaLabel || arialabelledby
const role = labelled ? 'img' : undefined

return (
<svg
ref={forwardedRef}
aria-hidden={ariaLabel ? 'false' : 'true'}
aria-hidden={labelled ? undefined : 'true'}
tabIndex={tabIndex}
focusable={tabIndex >= 0 ? 'true' : 'false'}
aria-label={ariaLabel}
aria-labelledby={arialabelledby}
role="img"
className={className}
role={role}
viewBox={`0 0 ${naturalWidth} ${naturalHeight}`}
width={width}
height={height}
Expand Down
3 changes: 2 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -49,5 +49,6 @@
"github/no-then": 0,
"eslint-comments/no-use": 0
}
}
},
"packageManager": "[email protected]"
}

0 comments on commit 79b9395

Please sign in to comment.