[WNMGDS-326] Fix react button styling (#572)

* Add combined ds-c-button--transparent ds-c-button--inverse styles as equivalent to --transparent-inverse

* Update Button.jsx, example and tests

* Update Badge example to not use <code>

packages/core/src/components/Badge/Badge.example.jsx

@@ -5,12 +5,8 @@ import ReactDOM from 'react-dom';
 ReactDOM.render(
   <Fragment>
     <Badge>Default badge</Badge>
-    <Badge variation="info">
-      Badge with <code>variation</code> prop
-    </Badge>
-    <Badge size="big">
-      Badge with <code>size</code> prop
-    </Badge>
+    <Badge variation="info">Badge with `variation` prop</Badge>
+    <Badge size="big">Badge with `size` prop</Badge>
   </Fragment>,
   document.getElementById('js-example')
 );

packages/core/src/components/Button/Button.example.jsx

@@ -1,18 +1,19 @@
+import React, { Fragment } from 'react';
 import Button from './Button';
-import React from 'react';
 import ReactDOM from 'react-dom';
 
 ReactDOM.render(
-  <div>
-    <Button>Button</Button>
-
-    <Button
-      className="ds-u-margin-left--1"
-      href="javascript:void(0);"
-      variation="primary"
-    >
-      Anchor button
+  <Fragment>
+    <Button className="ds-u-margin-right--1">Button</Button>
+    <Button className="ds-u-margin-right--1" variation="primary">
+      Button with `variation` prop
+    </Button>
+    <Button className="ds-u-margin-right--1" disabled>
+      Button with `disabled` prop
+    </Button>
+    <Button className="ds-u-margin-right--1" href="javascript:void(0);">
+      Button with `href` prop
     </Button>
-  </div>,
+  </Fragment>,
   document.getElementById('js-example')
 );

packages/core/src/components/Button/Button.jsx

@@ -57,24 +57,26 @@ export class Button extends React.PureComponent {
   }
 
   classNames() {
-    let variationClass = this.props.variation && `ds-c-button--${this.props.variation}`;
-    let disabledClass = this.props.disabled && 'ds-c-button--disabled';
+    const variationClass = this.props.variation && `ds-c-button--${this.props.variation}`;
+    const disabledClass = this.props.disabled && 'ds-c-button--disabled';
+    const sizeClass = this.props.size && `ds-c-button--${this.props.size}`;
+    let inverseClass = this.props.inverse && 'ds-c-button--inverse';
 
-    if (this.props.inverse) {
-      if (disabledClass) {
-        disabledClass += '-inverse';
-      } else if (variationClass) {
-        variationClass += '-inverse';
-      } else {
-        variationClass = 'ds-c-button--inverse';
-      }
+    // primary/danger/success variations don't need the inverse class
+    if (
+      this.props.variation === 'primary' ||
+      this.props.variation === 'danger' ||
+      this.props.variation === 'success'
+    ) {
+      inverseClass = '';
     }
 
     return classNames(
       'ds-c-button',
       disabledClass,
-      !disabledClass && variationClass,
-      this.props.size && `ds-c-button--${this.props.size}`,
+      variationClass,
+      inverseClass,
+      sizeClass,
       this.props.className
     );
   }

packages/core/src/components/Button/Button.scss

@@ -40,6 +40,10 @@
   }
 }
 
+.ds-c-button > svg {
+  @include inline-icon;
+}
+
 .ds-c-button--big {
   font-size: $h3-font-size;
   padding-bottom: $spacer-2;
@@ -183,9 +187,9 @@
 Inverse buttons
 */
 .ds-c-button--inverse,
-.ds-c-button--inverse:visited,
-.ds-c-button--transparent-inverse,
-.ds-c-button--transparent-inverse:visited {
+.ds-c-button--inverse:visited {
+  background-color: $color-background-inverse;
+  border-color: $border-color-inverse;
   color: $color-base-inverse;
 
   &:focus,
@@ -204,47 +208,61 @@ Inverse buttons
   }
 }
 
-.ds-c-button--inverse,
-.ds-c-button--inverse:visited {
-  background-color: $color-background-inverse;
-  border-color: $border-color-inverse;
+/*
+Equivalent to ".ds-c-button--disabled-inverse"
+*/
+.ds-c-button--inverse:disabled,
+.ds-c-button--inverse.ds-c-button--disabled {
+  background-color: darken($color-background-inverse, 10%);
+  border-color: darken($color-background-inverse, 10%);
+  color: $color-muted-inverse;
+  pointer-events: none;
+}
 
+/*
+Equivalent to ".ds-c-button--transparent-inverse"
+*/
+.ds-c-button--inverse.ds-c-button--transparent,
+.ds-c-button--inverse.ds-c-button--transparent:visited {
+  border-color: $color-background-inverse; 
+
   &:disabled,
-  &.ds-c-button--disabled {
-    background-color: darken($color-background-inverse, 10%);
-    border-color: darken($color-background-inverse, 10%);
-    color: $color-muted-inverse;
-    pointer-events: none;
-  }
-
   &:focus,
   &:hover,
   &:active,
+  &.ds-c-button--disabled,
   &.ds-c-button--focus,
   &.ds-c-button--hover,
   &.ds-c-button--active {
-    border-color: $color-base-inverse;
+    border-color: $color-background-inverse;
   }
 }
 
+/* stylelint-disable */
 .ds-c-button--transparent-inverse,
 .ds-c-button--transparent-inverse:visited {
+  @extend .ds-c-button--inverse;
+  border-color: $color-background-inverse;
+
   &:disabled,
-  &.ds-c-button--disabled {
-    color: $color-muted-inverse;
+  &:focus,
+  &:hover,
+  &:active,
+  &.ds-c-button--disabled,
+  &.ds-c-button--focus,
+  &.ds-c-button--hover,
+  &.ds-c-button--active {
+    border-color: $color-background-inverse;
   }
 }
 
-/* stylelint-disable */
-.ds-c-button--disabled-inverse {
+.ds-c-button--disabled-inverse,
+.ds-c-button--disabled-inverse:visited {
   @extend .ds-c-button--inverse;
   background-color: darken($color-background-inverse, 10%);
   border-color: darken($color-background-inverse, 10%);
   color: $color-muted-inverse;
   pointer-events: none;
 }
-/* stylelint-enable */
 
-.ds-c-button > svg {
-  @include inline-icon;
-}
+/* stylelint-enable */

packages/core/src/components/Button/Button.test.jsx

@@ -71,70 +71,70 @@ describe('Button', () => {
     expect(wrapper.render().text()).toBe(buttonText);
   });
 
-  it('appends additional class names', () => {
+  it('applies additional classes', () => {
     const props = { className: 'foobar' };
     const wrapper = shallow(<Button {...props}>{buttonText}</Button>);
     expect(wrapper.hasClass('foobar')).toBe(true);
     expect(wrapper.hasClass('ds-c-button')).toBe(true);
   });
 
-  it('renders as a success button', () => {
-    const props = { variation: 'success' };
+  it('applies variation classes', () => {
+    const props = { variation: 'danger' };
     const wrapper = shallow(<Button {...props}>{buttonText}</Button>);
 
     expect(wrapper.hasClass('ds-c-button')).toBe(true);
-    expect(wrapper.hasClass('ds-c-button--success')).toBe(true);
-    expect(wrapper.hasClass('ds-c-button--primary')).toBe(false);
+    expect(wrapper.hasClass('ds-c-button--danger')).toBe(true);
   });
 
-  it('renders as a small button', () => {
+  it('applies size classes', () => {
     const props = { size: 'small' };
     const wrapper = shallow(<Button {...props}>{buttonText}</Button>);
 
     expect(wrapper.hasClass('ds-c-button')).toBe(true);
     expect(wrapper.hasClass('ds-c-button--small')).toBe(true);
   });
 
-  it('overrides variation class when disabled', () => {
-    const props = {
-      disabled: true,
-      variation: 'primary'
-    };
+  it('applies disabled class', () => {
+    const props = { disabled: true };
     const wrapper = shallow(<Button {...props}>{buttonText}</Button>);
 
+    expect(wrapper.hasClass('ds-c-button')).toBe(true);
     expect(wrapper.hasClass('ds-c-button--disabled')).toBe(true);
-    expect(wrapper.hasClass('ds-c-button--primary')).toBe(false);
   });
 
-  it('applies inverse suffix to variation class', () => {
+  it('applies disabled, inverse, and variation classes together', () => {
     const props = {
+      disabled: true,
       inverse: true,
       variation: 'transparent'
     };
     const wrapper = shallow(<Button {...props}>{buttonText}</Button>);
 
-    expect(wrapper.hasClass('ds-c-button--transparent-inverse')).toBe(true);
-    expect(wrapper.hasClass('ds-c-button--transparent')).toBe(false);
+    expect(wrapper.hasClass('ds-c-button--transparent')).toBe(true);
+    expect(wrapper.hasClass('ds-c-button--inverse')).toBe(true);
+    expect(wrapper.hasClass('ds-c-button--disabled')).toBe(true);
     expect(wrapper.hasClass('ds-c-button')).toBe(true);
-    expect(wrapper.hasClass('ds-c-button--inverse')).toBe(false);
   });
 
-  it('applies inverse suffix to disabled class', () => {
+  it('doesnt apply inverse to primary/danger/success variations', () => {
     const props = {
       inverse: true,
-      disabled: true
+      variation: 'primary'
     };
     const wrapper = shallow(<Button {...props}>{buttonText}</Button>);
 
-    expect(wrapper.hasClass('ds-c-button--disabled-inverse')).toBe(true);
-    expect(wrapper.hasClass('ds-c-button')).toBe(true);
+    expect(wrapper.hasClass('ds-c-button--inverse')).toBe(false);
+    expect(wrapper.hasClass('ds-c-button--primary')).toBe(true);
   });
 
-  it('applies inverse suffix to default button class', () => {
-    const props = { inverse: true };
+  it('applies inverse to default/transparent variations', () => {
+    const props = {
+      inverse: true,
+      variation: 'transparent'
+    };
     const wrapper = shallow(<Button {...props}>{buttonText}</Button>);
 
     expect(wrapper.hasClass('ds-c-button--inverse')).toBe(true);
-    expect(wrapper.hasClass('ds-c-button')).toBe(true);
+    expect(wrapper.hasClass('ds-c-button--transparent')).toBe(true);
   });
 });