From d10a5c781dc210d563ea8577ff56d4e5501f79d4 Mon Sep 17 00:00:00 2001
From: Jon Q <hello@jonquach.com>
Date: Mon, 15 Apr 2019 11:54:44 -0400
Subject: [PATCH] Adjust Button + Loading UI. Add + fix tests

---
 src/components/Button/Button.css.js           | 27 ++++++++++--
 src/components/Button/ButtonV2.js             |  9 +++-
 .../Button/__tests__/ButtonV2.test.js         | 44 +++++++++++++++++++
 src/components/Button/docs/ButtonV2.md        |  1 +
 src/components/Spinner/Spinner.tsx            |  3 +-
 .../Spinner/__tests__/Spinner.test.tsx        | 33 ++++++++++++--
 stories/ButtonV2.stories.js                   | 16 ++++---
 7 files changed, 116 insertions(+), 17 deletions(-)

diff --git a/src/components/Button/Button.css.js b/src/components/Button/Button.css.js
index 31fde89bd..3489efe8c 100644
--- a/src/components/Button/Button.css.js
+++ b/src/components/Button/Button.css.js
@@ -299,6 +299,18 @@ export const makeButtonUI = (selector = 'button') => {
       width: 100%;
     }
 
+    &.is-loading {
+      &.is-spinButtonOnLoading {
+        animation: SpinButtonOnLoadAnimation 700ms linear infinite;
+        will-change: transform;
+        @keyframes SpinButtonOnLoadAnimation {
+          100% {
+            transform: rotate(360deg);
+          }
+        }
+      }
+    }
+
     ${makeButtonSizeStyles()}
 
     ${props => makePrimaryStyles('primary', props)}
@@ -472,6 +484,12 @@ export const ButtonContentUI = styled('span')`
       pointer-events: none;
     }
   `};
+
+  ${({ isLoading }) =>
+    isLoading &&
+    `
+    opacity: 0.35;
+  `};
 `
 
 export const FocusUI = styled('span')`
@@ -509,9 +527,12 @@ export const FocusUI = styled('span')`
 `
 
 export const SpinnerUI = styled(Spinner)`
-  margin-left: 8px;
-  position: relative;
-  top: -0.5px;
+  color: ${getColor('charcoal.500')};
+  margin: -6px 0 0 -6px;
+  position: absolute;
+  z-index: 1;
+  top: 50%;
+  left: 50%;
 `
 
 SpinnerUI.defaultProps = {
diff --git a/src/components/Button/ButtonV2.js b/src/components/Button/ButtonV2.js
index 6585de0a6..47bf16a34 100644
--- a/src/components/Button/ButtonV2.js
+++ b/src/components/Button/ButtonV2.js
@@ -36,6 +36,7 @@ type Props = {
   isLoading: boolean,
   isSuffix: boolean,
   size: ButtonSize,
+  spinButtonOnLoading: boolean,
   state?: UIState,
   submit: boolean,
   theme?: string,
@@ -57,6 +58,7 @@ class Button extends Component<Props> {
     isLast: false,
     isSuffix: false,
     size: 'md',
+    spinButtonOnLoading: false,
     submit: false,
   }
 
@@ -152,6 +154,7 @@ class Button extends Component<Props> {
       isLoading,
       isSuffix,
       size,
+      spinButtonOnLoading,
       state,
       submit,
       theme,
@@ -174,6 +177,7 @@ class Button extends Component<Props> {
       isSuffix && 'is-suffix',
       kind && `is-${kind}`,
       size && `is-${size}`,
+      spinButtonOnLoading && 'is-spinButtonOnLoading',
       state && `is-${state}`,
       theme && `is-${theme}`,
       className
@@ -191,12 +195,13 @@ class Button extends Component<Props> {
         innerRef={this.setInnerRef}
         type={type}
       >
+        {isLoading ? <SpinnerUI /> : null}
         <ButtonContentUI
-          className="c-ButtonV2__content"
           allowContentEventPropogation={allowContentEventPropogation}
+          className="c-ButtonV2__content"
+          isLoading={isLoading}
         >
           {this.getChildrenMarkup()}
-          {isLoading ? <SpinnerUI /> : null}
         </ButtonContentUI>
         {this.getFocusMarkup()}
       </ButtonUI>
diff --git a/src/components/Button/__tests__/ButtonV2.test.js b/src/components/Button/__tests__/ButtonV2.test.js
index 040617951..41510d865 100644
--- a/src/components/Button/__tests__/ButtonV2.test.js
+++ b/src/components/Button/__tests__/ButtonV2.test.js
@@ -323,3 +323,47 @@ describe('Link', () => {
     expect(wrapper.find('button').length).toBeTruthy()
   })
 })
+
+describe('Loading', () => {
+  test('Add loading className, if isLoading', () => {
+    const wrapper = mount(<Button isLoading />)
+    const el = wrapper.find('button')
+
+    expect(el.hasClass('is-loading')).toBeTruthy()
+  })
+
+  test('Renders a spinner if isLoading', () => {
+    const wrapper = mount(<Button isLoading />)
+    const el = wrapper.find('div.c-Spinner')
+
+    expect(el.length).toBeTruthy()
+  })
+
+  test('Does not renders a spinner if not isLoading', () => {
+    const wrapper = mount(<Button isLoading={false} />)
+    const el = wrapper.find('div.c-Spinner')
+
+    expect(el.length).toBeFalsy()
+  })
+
+  test('Becomes disabled if isLoading, by default', () => {
+    const wrapper = mount(<Button isLoading />)
+    const el = wrapper.find('button')
+
+    expect(el.prop('disabled')).toBe(true)
+  })
+
+  test('Does not become disabled, if specified', () => {
+    const wrapper = mount(<Button isLoading disableOnLoading={false} />)
+    const el = wrapper.find('button')
+
+    expect(el.prop('disabled')).toBe(false)
+  })
+
+  test('Add special spinButtonOnLoading, if isLoading and enabled', () => {
+    const wrapper = mount(<Button isLoading spinButtonOnLoading />)
+    const el = wrapper.find('button')
+
+    expect(el.hasClass('is-spinButtonOnLoading')).toBeTruthy()
+  })
+})
diff --git a/src/components/Button/docs/ButtonV2.md b/src/components/Button/docs/ButtonV2.md
index 58ac207b2..42c7a075f 100644
--- a/src/components/Button/docs/ButtonV2.md
+++ b/src/components/Button/docs/ButtonV2.md
@@ -41,6 +41,7 @@ Alternatively, [PropProvider](../../PropProvider) can be used to set this prop a
 | onFocus                      | `function` | `onFocus` event handler.                                                        |
 | kind                         | `string`   | Applies the specified style to the button.                                      |
 | size                         | `string`   | Sets the size of the button. Can be one of `"sm"`, `"md"` or `"lg"`.            |
+| spinButtonOnLoading          | `boolean`  | A special property that... spins the button if `isLoading`.                     |
 | state                        | `string`   | Applies state styles to the button.                                             |
 | submit                       | `boolean`  | Sets the `type` of the button to `"submit"`.                                    |
 | version                      | `number`   | Applies the version `2` variant of the button.                                  |
diff --git a/src/components/Spinner/Spinner.tsx b/src/components/Spinner/Spinner.tsx
index 22eaead65..79d432a1e 100644
--- a/src/components/Spinner/Spinner.tsx
+++ b/src/components/Spinner/Spinner.tsx
@@ -43,8 +43,7 @@ export class Spinner extends React.PureComponent<Props> {
 
   render() {
     const { color, shade, isRounded, speed, ...rest } = this.props
-    const size = this.getSize()
-    const spinnerSize = size
+    const spinnerSize = this.getSize()
 
     return (
       <SpinnerUI
diff --git a/src/components/Spinner/__tests__/Spinner.test.tsx b/src/components/Spinner/__tests__/Spinner.test.tsx
index cc4b3bccf..0bf779e89 100644
--- a/src/components/Spinner/__tests__/Spinner.test.tsx
+++ b/src/components/Spinner/__tests__/Spinner.test.tsx
@@ -5,23 +5,26 @@ import { Spinner } from '../Spinner'
 describe('ClassName', () => {
   test('Has default className', () => {
     const wrapper = mount(<Spinner />)
+    const el = wrapper.find('div.c-Spinner')
 
-    expect(wrapper.hasClass('c-Spinner')).toBeTruthy()
+    expect(el.hasClass('c-Spinner')).toBeTruthy()
   })
 
   test('Applies custom className if specified', () => {
     const customClass = 'piano-key-neck-tie'
     const wrapper = mount(<Spinner className={customClass} />)
+    const el = wrapper.find('div.c-Spinner')
 
-    expect(wrapper.prop('className')).toContain(customClass)
+    expect(el.prop('className')).toContain(customClass)
   })
 })
 
 describe('Accessibility', () => {
   test('Contains aria-role', () => {
     const wrapper = mount(<Spinner />)
+    const el = wrapper.find('div.c-Spinner')
 
-    expect(wrapper.props()['aria-busy']).toBeTruthy()
+    expect(el.props()['aria-busy']).toBeTruthy()
   })
 
   test('Contains hidden loading text', () => {
@@ -44,3 +47,27 @@ describe('Children', () => {
     expect(el.length).toBe(0)
   })
 })
+
+describe('Size', () => {
+  test('Uses legacy size (string -> number) if defined', () => {
+    const wrapper = mount(<Spinner size="lg" />)
+    const el = wrapper.find('.c-SpinnerSVG').first()
+
+    expect(el.prop('spinnerSize')).toBe(24)
+  })
+
+  test('Uses direct number size, if defined', () => {
+    const wrapper = mount(<Spinner size={123} />)
+    const el = wrapper.find('.c-SpinnerSVG').first()
+
+    expect(el.prop('spinnerSize')).toBe(123)
+  })
+
+  test('Falls back to defaultSize if legacy size is invalid', () => {
+    const defaultSize = 16
+    const wrapper = mount(<Spinner size="omg" />)
+    const el = wrapper.find('.c-SpinnerSVG').first()
+
+    expect(el.prop('spinnerSize')).toBe(defaultSize)
+  })
+})
diff --git a/stories/ButtonV2.stories.js b/stories/ButtonV2.stories.js
index c8cae30a6..4a991ddbb 100644
--- a/stories/ButtonV2.stories.js
+++ b/stories/ButtonV2.stories.js
@@ -1,6 +1,6 @@
 import React from 'react'
 import { storiesOf } from '@storybook/react'
-import { boolean, select } from '@storybook/addon-knobs'
+import { boolean, select, text } from '@storybook/addon-knobs'
 import {
   Button,
   ControlGroup,
@@ -63,9 +63,14 @@ const makeButtonVariations = (props = {}) => {
 
 stories.add('Default', () => {
   const props = {
+    children: text('children', 'Button'),
+    disabled: boolean('disabled', false),
+    disableOnLoading: boolean('disableOnLoading', true),
+    isActive: boolean('isActive', false),
+    isBlock: boolean('isBlock', false),
     isLoading: boolean('isLoading', false),
     kind: select(
-      'selector',
+      'kind',
       {
         primary: 'primary',
         primaryAlt: 'primaryAlt',
@@ -88,12 +93,9 @@ stories.add('Default', () => {
       },
       'lg'
     ),
+    spinButtonOnLoading: boolean('spinButtonOnLoading', false),
   }
-  return (
-    <Button {...props} version={2}>
-      Click Me
-    </Button>
-  )
+  return <Button {...props} version={2} />
 })
 
 stories.add('everything', () => (