From fe654ea82832e6f7ca5514e95cf536fd16db5928 Mon Sep 17 00:00:00 2001
From: Michele Azzolari <michele@azzolari.it>
Date: Wed, 29 Apr 2020 13:05:54 +0200
Subject: [PATCH] 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
---
 docs/content/packages/react.mdx                | 18 ++++++++++++++++++
 .../__tests__/__snapshots__/octicon.js.snap    |  2 +-
 lib/octicons_react/src/__tests__/octicon.js    |  4 ++++
 lib/octicons_react/src/index.d.ts              |  3 ++-
 lib/octicons_react/src/index.js                |  7 +++++--
 5 files changed, 30 insertions(+), 4 deletions(-)

diff --git a/docs/content/packages/react.mdx b/docs/content/packages/react.mdx
index 26887d13b..d55e442e9 100644
--- 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
index d2f4ec7d8..047e720c4 100644
--- 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
index faaee0c22..97a25a0de 100644
--- 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
index d5f70e63d..b29838a1c 100644
--- 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
index c9431e024..34d6be026 100644
--- 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,6 +52,7 @@ export default function Octicon(props) {
 
 Octicon.defaultProps = {
   className: 'octicon',
+  fill: 'currentColor',
   size: 16,
   verticalAlign: 'text-bottom'
 }
@@ -59,6 +60,8 @@ Octicon.defaultProps = {
 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))]),