feat(button): Removing un-needed outline prop for Button.

telus · Aug 3, 2017 · 36b2b65 · 36b2b65
1 parent 61fa039
commit 36b2b65
Show file tree

Hide file tree

Showing 4 changed files with 56 additions and 101 deletions.
diff --git a/src/components/Button/Button.jsx b/src/components/Button/Button.jsx
@@ -3,43 +3,33 @@ import PropTypes from 'prop-types';
 
 import styles from './Button.modules.scss';
 
-const getClassName = (variant, invert, outline) => {
-  if (invert && outline) {
-    return styles[`${variant}InvertedOutlined`];
-  }
-
+const getClassName = (variant, invert) => {
   if (invert) {
     return styles[`${variant}Inverted`];
   }
 
-  if (outline) {
-    return styles[`${variant}Outlined`];
-  }
-
   return styles[variant];
 };
 
 /**
  * <span class="docs--badge green">coming soon!</span>
  */
-const Button = ({ type, variant, invert, outline, children, ...rest }) => (
-  <button {...rest} type={type} className={getClassName(variant, invert, outline)}>
+const Button = ({ type, variant, invert, children, ...rest }) => (
+  <button {...rest} type={type} className={getClassName(variant, invert)}>
     {children}
   </button>
 );
 
 Button.propTypes = {
   type: PropTypes.oneOf(['button', 'submit', 'reset']),
-  variant: PropTypes.oneOf(['primary', 'secondary']),
+  variant: PropTypes.oneOf(['primary', 'secondary', 'outlined']),
   invert: PropTypes.bool,
-  outline: PropTypes.bool,
   children: PropTypes.string.isRequired
 };
 Button.defaultProps = {
   type: 'button',
   variant: 'primary',
-  invert: false,
-  outline: false
+  invert: false
 };
 
 export default Button;
diff --git a/src/components/Button/Button.md b/src/components/Button/Button.md
@@ -8,32 +8,27 @@ WIP
 
 ## Variants
 ```
-<div>
-  <Button variant="primary">Submit</Button>
-  <Button variant="secondary">Reset</Button>
+<div style={{backgroundColor: "blue", padding: "10px"}}>
+  <div>
+    <Button variant="primary">Submit</Button>
+    <Button variant="primary" invert>Submit</Button>
+  </div>
+  <div>
+    <Button variant="secondary">Reset</Button>
+    <Button variant="secondary" invert>Reset</Button>
+  </div>
+  <div>
+    <Button variant="outlined">Go back</Button>
+    <Button variant="outlined" invert>Go back</Button>
+  </div>
 </div>
 ```
 
 ## Inverted
 ```
-<div>
+<div style={{backgroundColor: "blue", padding: "10px"}}>
   <Button variant="primary" invert>Submit</Button>
   <Button variant="secondary" invert>Reset</Button>
-</div>
-```
-
-## Outlined
-```
-<div>
-  <Button variant="primary" outline>Submit</Button>
-  <Button variant="secondary" outline>Reset</Button>
-</div>
-```
-
-## Invert and Outlined
-```
-<div>
-  <Button variant="primary" invert outline>Submit</Button>
-  <Button variant="secondary" invert outline>Reset</Button>
+  <Button variant="outlined" invert>Go back</Button>
 </div>
 ```
diff --git a/src/components/Button/Button.modules.scss b/src/components/Button/Button.modules.scss
@@ -5,16 +5,15 @@
 
 $primary-bg-color: $color-primary;
 $secondary-bg-color: $color-secondary;
-$text-color: $color-white;
+$default-text-color: $color-white;
 
 
-@mixin text-color-outline($text-color) {
-  background-color: transparent;
+@mixin text-color-outline($text-color, $bg-color) {
+  background-color: $bg-color;
+  color: $text-color;
   // Don't add a border because it changes the size of the button.
   // Box-shadow gives a "free" border using the text color.
   box-shadow: 0 0 0 1px;
-
-  color: $text-color;
 }
 
 .button {
@@ -44,93 +43,71 @@ $text-color: $color-white;
   }
 }
 
-.inverted {
-  background-color: $text-color;
 
-  &:hover {
-    @include text-color-outline($text-color)
-  }
-}
+.primary {
+  composes: button;
 
-.invertedOutlined {
-  background-color: transparent;
-  color: $text-color;
+  background-color: $primary-bg-color;
+  color: $default-text-color;
 
   &:hover {
-    background-color: $text-color;
-    box-shadow: 0 0 0 1px;
+    @include text-color-outline($primary-bg-color, $default-text-color);
   }
 }
 
-
-.primary {
+.primaryInverted {
   composes: button;
 
-  background-color: $primary-bg-color;
-  color: $text-color;
+  background-color: $default-text-color;
+  color: $primary-bg-color;
 
   &:hover {
-    @include text-color-outline($primary-bg-color);
+    background-color: $primary-bg-color;
+    color: $default-text-color;
   }
 }
 
 .secondary {
   composes: button;
 
   background-color: $secondary-bg-color;
-  color: $text-color;
+  color: $default-text-color;
 
   &:hover {
-    @include text-color-outline($secondary-bg-color);
+    @include text-color-outline($secondary-bg-color, $default-text-color);
   }
 }
 
-.primaryOutlined {
+.secondaryInverted {
   composes: button;
 
-  @include text-color-outline($primary-bg-color);
+  background-color: $default-text-color;
+  color: $secondary-bg-color;
 
   &:hover {
-    background-color: $primary-bg-color;
-    color: $text-color;
+    background-color: $secondary-bg-color;
+    color: $default-text-color;
   }
 }
 
-.secondaryOutlined {
+.outlined {
   composes: button;
 
-  @include text-color-outline($secondary-bg-color);
+  @include text-color-outline($default-text-color, transparent);
 
   &:hover {
     background-color: $secondary-bg-color;
-    color: $text-color;
+    color: $default-text-color;
   }
 }
 
-.primaryInverted {
-  composes: inverted button;
-
-  color: $primary-bg-color;
-}
-
-.secondaryInverted {
-  composes: inverted button;
+.outlinedInverted {
+  composes: button;
 
+  background-color: $default-text-color;
   color: $secondary-bg-color;
-}
-
-.primaryInvertedOutlined {
-  composes: invertedOutlined button;
-
-  &:hover {
-    color: $primary-bg-color;
-  }
-}
-
-.secondaryInvertedOutlined {
-  composes: invertedOutlined button;
 
   &:hover {
-    color: $secondary-bg-color;
+    @include text-color-outline($default-text-color, transparent)
   }
 }
diff --git a/src/components/Button/__tests__/Button.spec.jsx b/src/components/Button/__tests__/Button.spec.jsx
@@ -30,27 +30,20 @@ describe('Button', () => {
 
     button = doShallow({ variant: 'secondary' });
     expect(button).toHaveClassName('secondary');
+
+    button = doShallow({ variant: 'outlined' });
+    expect(button).toHaveClassName('outlined');
   });
 
-  it('can be inverted, outlined, or both', () => {
-    let primaryButton = doShallow({ variant: 'primary', invert: true });
+  it('can be inverted', () => {
+    const primaryButton = doShallow({ variant: 'primary', invert: true });
     expect(primaryButton).toHaveClassName('primaryInverted');
 
-    primaryButton = doShallow({ variant: 'primary', outline: true });
-    expect(primaryButton).toHaveClassName('primaryOutlined');
-
-    primaryButton = doShallow({ variant: 'primary', invert: true, outline: true });
-    expect(primaryButton).toHaveClassName('primaryInvertedOutlined');
-
-
-    let secondaryButton = doShallow({ variant: 'secondary', invert: true });
+    const secondaryButton = doShallow({ variant: 'secondary', invert: true });
     expect(secondaryButton).toHaveClassName('secondaryInverted');
 
-    secondaryButton = doShallow({ variant: 'secondary', outline: true });
-    expect(secondaryButton).toHaveClassName('secondaryOutlined');
-
-    secondaryButton = doShallow({ variant: 'secondary', invert: true, outline: true });
-    expect(secondaryButton).toHaveClassName('secondaryInvertedOutlined');
+    const outlinedButton = doShallow({ variant: 'outlined', invert: true });
+    expect(outlinedButton).toHaveClassName('outlinedInverted');
   });
 
   it('passes additional attributes to button element', () => {