Merge pull request #73 from quid/fix/button-icon-spacing

fix: properly space Icon when inside Button
quid · Apr 11, 2019 · c3ebc9b · c3ebc9b
2 parents 6b02ae8 + b2bb849
commit c3ebc9b
Show file tree

Hide file tree

Showing 3 changed files with 71 additions and 41 deletions.
diff --git a/packages/react-core/src/Button/Button.md b/packages/react-core/src/Button/Button.md
@@ -82,6 +82,21 @@ import Icon from '../Icon';
         </Button>
       </td>
     </tr>
+    <tr>
+      <td>Primary with both (alt)</td>
+      <td>
+        <Button importance="primary">
+          Button
+          <Icon name="download" />
+        </Button>
+      </td>
+      <td>
+        <Button importance="primary" disabled>
+          Button
+          <Icon name="download" />
+        </Button>
+      </td>
+    </tr>
     <tr>
       <td>Primary Small with both</td>
       <td>

diff --git a/packages/react-core/src/Button/__snapshots__/index.test.js.snap b/packages/react-core/src/Button/__snapshots__/index.test.js.snap
@@ -63,12 +63,8 @@ exports[`renders a Button with small size 1`] = `
 .emotion-0 .e1t5eso00 {
   display: block;
   line-height: 1;
-  font-size: 1.42em;
-  margin-left: -6px;
-  margin-right: -6px;
-  margin-top: 5px;
-  margin-bottom: 5px;
   font-size: 1em;
+  margin: 5px -6px;
 }
 
 <Button
@@ -80,7 +76,11 @@ exports[`renders a Button with small size 1`] = `
     className="emotion-0 emotion-1"
     type="button"
   >
-    Hello, World!
+    <span
+      key=".0"
+    >
+      Hello, World!
+    </span>
   </button>
 </Button>
 `;
@@ -158,10 +158,7 @@ exports[`renders a disabled and transparent Button 1`] = `
   display: block;
   line-height: 1;
   font-size: 1.42em;
-  margin-left: -6px;
-  margin-right: -6px;
-  margin-top: 5px;
-  margin-bottom: 5px;
+  margin: 5px -6px;
 }
 
 <Button
@@ -176,7 +173,11 @@ exports[`renders a disabled and transparent Button 1`] = `
     disabled={true}
     type="button"
   >
-    Hello, World!
+    <span
+      key=".0"
+    >
+      Hello, World!
+    </span>
   </button>
 </Button>
 `;
@@ -245,10 +246,7 @@ exports[`renders the expected markup 1`] = `
   display: block;
   line-height: 1;
   font-size: 1.42em;
-  margin-left: -6px;
-  margin-right: -6px;
-  margin-top: 5px;
-  margin-bottom: 5px;
+  margin: 5px -6px;
 }
 
 <Button
@@ -262,7 +260,11 @@ exports[`renders the expected markup 1`] = `
     data-action="foo"
     type="button"
   >
-    Hello, World!
+    <span
+      key=".0"
+    >
+      Hello, World!
+    </span>
   </button>
 </Button>
 `;
diff --git a/packages/react-core/src/Button/index.js b/packages/react-core/src/Button/index.js
@@ -80,6 +80,7 @@ const Button = styled(
     importance,
     disabled,
     type,
+    children,
     ...props
   }: Props) => {
     let Tag, specificProps;
@@ -94,7 +95,17 @@ const Button = styled(
       specificProps = { disabled, type };
     }
 
-    return <Tag {...specificProps} {...props} />;
+    return (
+      <Tag {...specificProps} {...props}>
+        {React.Children.map(children, node =>
+          ['string', 'number'].includes(typeof node) ? (
+            <span>{node}</span>
+          ) : (
+            node
+          )
+        )}
+      </Tag>
+    );
   }
 )`
   ${reset};
@@ -162,31 +173,33 @@ const Button = styled(
     `}
 
   ${Icon} {
-      display: block;
-      line-height: 1;
-      font-size: 1.42em;
-      margin-left: -6px;
-      margin-right: -6px;
-      margin-top: 5px;
-      margin-bottom: 5px;
-
-      ${props =>
-        props.size === 'small' &&
-        css`
-          font-size: 1em;
-        `}
-
-      ${props =>
-        React.Children.count(props.children) > 1 &&
-        css`
-          display: inline-block;
-          position: relative;
-          font-size: 1em;
-          margin-left: 0;
-          bottom: -1px;
-          margin-right: 0.35em;
-        `}
+    display: block;
+    line-height: 1;
+    font-size: ${props => (props.size === 'small' ? 1 : 1.42)}em;
+    margin: 5px -6px;
   }
+
+  ${props =>
+    React.Children.count(props.children) > 1 &&
+    css`
+      ${Icon} {
+        display: inline-block;
+        position: relative;
+        font-size: 1em;
+        margin-left: 0;
+        bottom: -1px;
+        margin-left: 0.35em;
+        margin-right: 0;
+        &:first-child {
+          margin-right: 0.35em;
+          margin-left: 0;
+        }
+        &:last-child {
+          margin-left: 0.35em;
+          margin-right: 0;
+        }
+      }
+    `}
 `;
 
 Button.defaultProps = {