React: fill is not a style property, it's a svg attribute.

Added a default to currentColor but it's possibile to overwrite it via 'fill' prop Updated react doc Updates react test
primer · Apr 30, 2020 · fe654ea · fe654ea
1 parent c54859b
commit fe654ea
Show file tree

Hide file tree

Showing 5 changed files with 30 additions and 4 deletions.
diff --git a/docs/content/packages/react.mdx b/docs/content/packages/react.mdx
@@ -154,6 +154,24 @@ export default () => (
 ```
 
 
+### Fill
+The `fill` prop takes a string value that can be used to color the icon.
+Default value is `currentColor` which means it inherits parent's text color.
+
+```js
+// Example usage
+import Octicon, {LogoGithub} from '@primer/octicons-react'
+
+export default () => (
+  <h1>
+    <a href='https://github.com'>
+      <Octicon icon={LogoGithub} fill='#28284E' size='large' ariaLabel='GitHub'/>
+    </a>
+  </h1>
+)
+```
+
+
 ## Custom icons
 Each of our icon components is really just a function that renders its SVG
 `<path>`. To accommodate icons varying aspect ratios, the `Octicon` component

diff --git a/lib/octicons_react/src/__tests__/__snapshots__/octicon.js.snap b/lib/octicons_react/src/__tests__/__snapshots__/octicon.js.snap
@@ -4,12 +4,12 @@ exports[`<Octicon> outputs <svg> 1`] = `
 <svg
   aria-hidden="true"
   className="octicon"
+  fill="currentColor"
   height={16}
   role="img"
   style={
     Object {
       "display": "inline-block",
-      "fill": "currentColor",
       "userSelect": "none",
       "verticalAlign": "text-bottom",
     }

diff --git a/lib/octicons_react/src/__tests__/octicon.js b/lib/octicons_react/src/__tests__/octicon.js
@@ -35,6 +35,10 @@ describe('<Octicon>', () => {
     expect(render(<TestOcticon verticalAlign="top" />).props.style.verticalAlign).toEqual('text-top')
   })
 
+  it('respects the fill prop', () => {
+    expect(render(<TestOcticon fill="yellow" />).props.fill).toEqual('yellow')
+  })
+
   describe('size props', () => {
     it('respects size="small"', () => {
       const rendered = render(<TestOcticon size="small" />)

diff --git a/lib/octicons_react/src/index.d.ts b/lib/octicons_react/src/index.d.ts
@@ -5,8 +5,9 @@ import {Icon} from './__generated__/icons'
 type Size = 'small' | 'medium' | 'large'
 export interface OcticonProps {
   ariaLabel?: string
-  children?: React.ReactElement<any>
   className?: string
+  children?: React.ReactElement<any>
+  fill?: string
   height?: number
   icon: Icon
   size?: number | Size

diff --git a/lib/octicons_react/src/index.js b/lib/octicons_react/src/index.js
@@ -15,7 +15,7 @@ const alignMap = {
 const defaultSize = [16, 16]
 
 export default function Octicon(props) {
-  const {ariaLabel, children, className, height, icon: Icon, size, verticalAlign, width} = props
+  const {ariaLabel, children, className, height, icon: Icon, size, verticalAlign, width, fill} = props
 
   const child = typeof Icon === 'function' ? <Icon /> : React.Children.only(children)
 
@@ -26,6 +26,7 @@ export default function Octicon(props) {
     'aria-label': ariaLabel,
     className,
     height,
+    fill,
     role: 'img',
     viewBox: [0, 0, ...widthHeight].join(' ')
   }
@@ -42,7 +43,6 @@ export default function Octicon(props) {
 
   attrs.style = {
     display: 'inline-block',
-    fill: 'currentColor',
     userSelect: 'none',
     verticalAlign: alignMap[verticalAlign] || verticalAlign
   }
@@ -52,13 +52,16 @@ export default function Octicon(props) {
 
 Octicon.defaultProps = {
   className: 'octicon',
+  fill: 'currentColor',
   size: 16,
   verticalAlign: 'text-bottom'
 }
 
 Octicon.propTypes = {
   ariaLabel: PropTypes.string,
   children: PropTypes.element,
+  className: PropTypes.string,
+  fill: PropTypes.string,
   height: PropTypes.number,
   icon: PropTypes.func,
   size: PropTypes.oneOfType([PropTypes.number, PropTypes.oneOf(Object.keys(sizeMap))]),