Skip to content

Commit a38ae2d

Browse files
broccolinisoupjoshblack
broccolinisoup
and
joshblack
authored
Update Icons to use React.forwardRef & Remove support for Octicon (#943)
* Update peer dependecy react version and the octicon func to use React.forwardRef * snaps * add a changeset * update type check * Bye bye Octicons * update snapshots * Update lib/octicons_react/README.md Co-authored-by: Josh Black <joshblack@github.com> * update changeset --------- Co-authored-by: Josh Black <joshblack@github.com>
1 parent 6c94e0e commit a38ae2d

11 files changed

Lines changed: 56 additions & 127 deletions

File tree

‎.changeset/dry-buttons-leave.md‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
---
2+
'@primer/octicons': major
3+
---
4+
5+
Remove support for `Octicon`
6+
Update peer dependency React version to support >=16.3
7+
Update icons to use React.forwardRef

‎lib/octicons_react/README.md‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -121,9 +121,7 @@ export default () => (
121121
)
122122
```
123123

124-
### `Octicon` (DEPRECATED)
125124

126-
> ⚠️ The `Octicon` component is deprecated. Use icon components on their own instead:
127125
```diff
128126
- <Octicon icon={AlertIcon} />
129127
+ <AlertIcon />

‎lib/octicons_react/__tests__/__snapshots__/public-api.test.js.snap‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -312,6 +312,5 @@ exports[`@primer/octicons-react should not update exports without a semver chang
312312
"ZapIcon",
313313
"ZoomInIcon",
314314
"ZoomOutIcon",
315-
"default",
316315
]
317316
`;

‎lib/octicons_react/__tests__/tree-shaking.test.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,5 +50,5 @@ test('tree shaking single export', async () => {
5050
})
5151

5252
const bundleSize = Buffer.byteLength(output[0].code.trim()) / 1000
53-
expect(`${bundleSize}kB`).toMatchInlineSnapshot(`"3.168kB"`)
53+
expect(`${bundleSize}kB`).toMatchInlineSnapshot(`"3.244kB"`)
5454
})

‎lib/octicons_react/package.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@
6969
"typescript": "^4.8.3"
7070
},
7171
"peerDependencies": {
72-
"react": ">=15"
72+
"react": ">=16.3"
7373
},
7474
"engines": {
7575
"node": ">=8"

‎lib/octicons_react/pages/index.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ export default function App() {
4545
<td>
4646
<pre>
4747
{`
48-
import Octicon, {${Icon.name}} from '${pkg.name}'
48+
import {${Icon.name}} from '${pkg.name}'
4949
export default () => <${Icon.name} />
5050
`.trim()}
5151
</pre>

‎lib/octicons_react/src/__tests__/octicon.js‎

Lines changed: 1 addition & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -1,61 +1,7 @@
11
import '@testing-library/jest-dom'
22
import {render} from '@testing-library/react'
33
import React from 'react'
4-
import Octicon, {AlertIcon} from '../index'
5-
6-
describe('Octicon component', () => {
7-
let mock
8-
9-
beforeEach(() => {
10-
mock = jest.spyOn(console, 'warn').mockImplementation(() => jest.fn())
11-
})
12-
13-
afterEach(() => {
14-
expect(mock).toHaveBeenCalledWith(
15-
expect.stringContaining(
16-
`<Octicon> is deprecated. Use icon components on their own instead (e.g. <Octicon icon={AlertIcon} /> → <AlertIcon />)`
17-
)
18-
)
19-
mock.mockRestore()
20-
})
21-
22-
it('throws an error without a single child or icon prop', () => {
23-
const spy = jest.spyOn(console, 'error').mockImplementation(() => jest.fn())
24-
expect(() => render(<Octicon />)).toThrow()
25-
expect(() => render(<Octicon icon={null} />)).toThrow()
26-
expect(spy).toHaveBeenCalledWith(
27-
expect.objectContaining({
28-
message: expect.stringContaining('React.Children.only expected to receive a single React element child')
29-
})
30-
)
31-
32-
spy.mockRestore()
33-
})
34-
35-
it('passes props to icon prop', () => {
36-
const {container} = render(
37-
<Octicon aria-label="icon" className="foo" size={20} verticalAlign="middle" icon={AlertIcon} />
38-
)
39-
expect(container.querySelector('svg')).toHaveAttribute('aria-label', 'icon')
40-
expect(container.querySelector('svg')).toHaveAttribute('class', 'foo')
41-
expect(container.querySelector('svg')).toHaveAttribute('width', '20')
42-
expect(container.querySelector('svg')).toHaveAttribute('height', '20')
43-
expect(container.querySelector('svg')).toHaveStyle({verticalAlign: 'middle'})
44-
})
45-
46-
it('passes props to icon as child', () => {
47-
const {container} = render(
48-
<Octicon aria-label="icon" className="foo" size={20} verticalAlign="middle">
49-
<AlertIcon />
50-
</Octicon>
51-
)
52-
expect(container.querySelector('svg')).toHaveAttribute('aria-label', 'icon')
53-
expect(container.querySelector('svg')).toHaveAttribute('class', 'foo')
54-
expect(container.querySelector('svg')).toHaveAttribute('width', '20')
55-
expect(container.querySelector('svg')).toHaveAttribute('height', '20')
56-
expect(container.querySelector('svg')).toHaveStyle({verticalAlign: 'middle'})
57-
})
58-
})
4+
import {AlertIcon} from '../index'
595

606
describe('An icon component', () => {
617
it('matches snapshot', () => {

‎lib/octicons_react/src/createIconComponent.js‎

Lines changed: 42 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -10,43 +10,49 @@ export function createIconComponent(name, defaultClassName, getSVGData) {
1010
const svgDataByHeight = getSVGData()
1111
const heights = Object.keys(svgDataByHeight)
1212

13-
function Icon({
14-
'aria-label': ariaLabel,
15-
tabIndex,
16-
className = defaultClassName,
17-
fill = 'currentColor',
18-
size = 16,
19-
verticalAlign = 'text-bottom'
20-
}) {
21-
const height = sizeMap[size] || size
22-
const naturalHeight = closestNaturalHeight(heights, height)
23-
const naturalWidth = svgDataByHeight[naturalHeight].width
24-
const width = height * (naturalWidth / naturalHeight)
25-
const path = svgDataByHeight[naturalHeight].path
13+
const Icon = React.forwardRef(
14+
(
15+
{
16+
'aria-label': ariaLabel,
17+
tabIndex,
18+
className = defaultClassName,
19+
fill = 'currentColor',
20+
size = 16,
21+
verticalAlign = 'text-bottom'
22+
},
23+
forwardedRef
24+
) => {
25+
const height = sizeMap[size] || size
26+
const naturalHeight = closestNaturalHeight(heights, height)
27+
const naturalWidth = svgDataByHeight[naturalHeight].width
28+
const width = height * (naturalWidth / naturalHeight)
29+
const path = svgDataByHeight[naturalHeight].path
2630

27-
return (
28-
<svg
29-
aria-hidden={ariaLabel ? 'false' : 'true'}
30-
tabIndex={tabIndex}
31-
focusable={tabIndex >= 0 ? 'true' : 'false'}
32-
aria-label={ariaLabel}
33-
role="img"
34-
className={className}
35-
viewBox={`0 0 ${naturalWidth} ${naturalHeight}`}
36-
width={width}
37-
height={height}
38-
fill={fill}
39-
style={{
40-
display: 'inline-block',
41-
userSelect: 'none',
42-
verticalAlign,
43-
overflow: 'visible'
44-
}}
45-
>
46-
{path}
47-
</svg>
48-
)
49-
}
31+
return (
32+
<svg
33+
ref={forwardedRef}
34+
aria-hidden={ariaLabel ? 'false' : 'true'}
35+
tabIndex={tabIndex}
36+
focusable={tabIndex >= 0 ? 'true' : 'false'}
37+
aria-label={ariaLabel}
38+
role="img"
39+
className={className}
40+
viewBox={`0 0 ${naturalWidth} ${naturalHeight}`}
41+
width={width}
42+
height={height}
43+
fill={fill}
44+
style={{
45+
display: 'inline-block',
46+
userSelect: 'none',
47+
verticalAlign,
48+
overflow: 'visible'
49+
}}
50+
>
51+
{path}
52+
</svg>
53+
)
54+
}
55+
)
5056

5157
Icon.displayName = name
5258

‎lib/octicons_react/src/index.d.ts‎

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -15,11 +15,4 @@ export interface OcticonProps {
1515
verticalAlign?: 'middle' | 'text-bottom' | 'text-top' | 'top' | 'unset'
1616
}
1717

18-
/**
19-
* @deprecated Use icon components on their own instead (e.g. `<Octicon icon={AlertIcon} />` → `<AlertIcon />`)
20-
*/
21-
declare const Octicon: React.FC<OcticonProps>
22-
23-
export default Octicon
24-
2518
export * from './__generated__/icons'

‎lib/octicons_react/src/index.js‎

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1 @@
1-
import React from 'react'
2-
3-
export default function Octicon({icon: Icon, children, ...props}) {
4-
// eslint-disable-next-line no-console
5-
console.warn(
6-
// eslint-disable-next-line github/unescaped-html-literal
7-
'<Octicon> is deprecated. Use icon components on their own instead (e.g. <Octicon icon={AlertIcon} /> → <AlertIcon />)'
8-
)
9-
return typeof Icon === 'function' ? <Icon {...props} /> : React.cloneElement(React.Children.only(children), props)
10-
}
11-
121
export * from './__generated__/icons'

0 commit comments

Comments
 (0)
⚡