From 21d88658a60518aa264a2f34e2ba6fd025a42a11 Mon Sep 17 00:00:00 2001 From: Jon Q Date: Thu, 4 Apr 2019 11:37:38 -0400 Subject: [PATCH 1/2] Spinner: Refactor + Integrate into Button This update refactors the Spinner component to be an animated SVG, rather than a spinner Icon. It also refactors Spinner with `css.js` and all of the latest HSDS: React conventions. Spinner has also been integrated into components like `Button` under a new `isLoading` prop. --- src/components/Button/Button.css.js | 11 +++ src/components/Button/ButtonV2.js | 19 ++++- src/components/Button/docs/ButtonV2.md | 12 +-- src/components/Spinner/README.md | 25 ++---- src/components/Spinner/Spinner.css.ts | 85 +++++++++++++++++++ src/components/Spinner/Spinner.tsx | 82 ++++++++++++++++++ src/components/Spinner/Spinner.types.ts | 0 src/components/Spinner/Spinner.utils.ts | 7 ++ .../Spinner/__tests__/Spinner.test.tsx | 46 ++++++++++ src/components/Spinner/index.js | 40 --------- src/components/Spinner/index.ts | 3 + src/components/Spinner/propTypes.js | 3 - src/components/Spinner/tests/Spinner.test.js | 65 -------------- src/styles/components/Spinner.scss | 25 +----- stories/ButtonV2.stories.js | 42 +++++++-- 15 files changed, 304 insertions(+), 161 deletions(-) create mode 100644 src/components/Spinner/Spinner.css.ts create mode 100644 src/components/Spinner/Spinner.tsx create mode 100644 src/components/Spinner/Spinner.types.ts create mode 100644 src/components/Spinner/Spinner.utils.ts create mode 100644 src/components/Spinner/__tests__/Spinner.test.tsx delete mode 100644 src/components/Spinner/index.js create mode 100644 src/components/Spinner/index.ts delete mode 100644 src/components/Spinner/propTypes.js delete mode 100644 src/components/Spinner/tests/Spinner.test.js diff --git a/src/components/Button/Button.css.js b/src/components/Button/Button.css.js index d391e5fa6..a630e3ad2 100644 --- a/src/components/Button/Button.css.js +++ b/src/components/Button/Button.css.js @@ -1,6 +1,7 @@ // @flow import get from 'dash-get' import baseStyles from '../../styles/resets/baseStyles.css.js' +import Spinner from '../Spinner' import styled from '../styled' import { getColor } from '../../styles/utilities/color' import forEach from '../../styles/utilities/forEach' @@ -506,3 +507,13 @@ export const FocusUI = styled('span')` } } ` + +export const SpinnerUI = styled(Spinner)` + margin-left: 8px; + position: relative; + top: -0.5px; +` + +SpinnerUI.defaultProps = { + size: 12, +} diff --git a/src/components/Button/ButtonV2.js b/src/components/Button/ButtonV2.js index 731ded1a6..6585de0a6 100644 --- a/src/components/Button/ButtonV2.js +++ b/src/components/Button/ButtonV2.js @@ -9,7 +9,12 @@ import { includes } from '../../utilities/arrays' import { noop } from '../../utilities/other' import { memoize } from '../../utilities/memoize' import RouteWrapper from '../RouteWrapper' -import { makeButtonUI, ButtonContentUI, FocusUI } from './Button.css.js' +import { + makeButtonUI, + ButtonContentUI, + FocusUI, + SpinnerUI, +} from './Button.css.js' import { COMPONENT_KEY } from './utils' import { COMPONENT_KEY as ICON_KEY } from '../Icon/utils' @@ -20,6 +25,7 @@ type Props = { children?: any, className?: string, disabled: boolean, + disableOnLoading: boolean, kind: ButtonKind, innerRef: (ref: any) => void, isActive: boolean, @@ -27,6 +33,7 @@ type Props = { isFirst: boolean, isNotOnly: boolean, isLast: boolean, + isLoading: boolean, isSuffix: boolean, size: ButtonSize, state?: UIState, @@ -40,6 +47,7 @@ class Button extends Component { buttonRef: noop, canRenderFocus: true, disable: false, + disableOnLoading: true, kind: 'default', innerRef: noop, isActive: false, @@ -132,6 +140,8 @@ class Button extends Component { allowContentEventPropogation, children, className, + disabled, + disableOnLoading, kind, innerRef, isActive, @@ -139,6 +149,7 @@ class Button extends Component { isFirst, isNotOnly, isLast, + isLoading, isSuffix, size, state, @@ -149,13 +160,17 @@ class Button extends Component { ...rest } = this.props + const isDisabled = disabled || (isLoading && disableOnLoading) + const componentClassName = classNames( 'c-ButtonV2', isActive && 'is-active', isBlock && 'is-block', + isDisabled && 'is-disabled', isFirst && 'is-first', isNotOnly && 'is-notOnly', isLast && 'is-last', + isLoading && 'is-loading', isSuffix && 'is-suffix', kind && `is-${kind}`, size && `is-${size}`, @@ -172,6 +187,7 @@ class Button extends Component { @@ -180,6 +196,7 @@ class Button extends Component { allowContentEventPropogation={allowContentEventPropogation} > {this.getChildrenMarkup()} + {isLoading ? : null} {this.getFocusMarkup()} diff --git a/src/components/Button/docs/ButtonV2.md b/src/components/Button/docs/ButtonV2.md index d7978db69..58ac207b2 100644 --- a/src/components/Button/docs/ButtonV2.md +++ b/src/components/Button/docs/ButtonV2.md @@ -26,21 +26,23 @@ Alternatively, [PropProvider](../../PropProvider) can be used to set this prop a | Prop | Type | Description | | ---------------------------- | ---------- | ------------------------------------------------------------------------------- | -| allowContentEventPropogation | `bool` | Enables child events to pass through to Button. Default `true`. | +| allowContentEventPropogation | `boolean` | Enables child events to pass through to Button. Default `true`. | | className | `string` | Custom class names to be added to the component. | -| disabled | `bool` | Disable the button so it can't be clicked. | +| disabled | `boolean` | Disable the button so it can't be clicked. | +| disabledOnLoading | `boolean` | Disables the button when `isLoading` is true. Default `true`. | | fetch | `function` | function which returns a promise, will be invoked before routing the `to` route | | href | `string` | Hyperlink for the button. This transforms the button to a `` selector. | | innerRef | `function` | Retrieves the `button` DOM node. | -| isFocused | `bool` | Renders the focused style. | -| isSuffix | `bool` | Renders suffix styles. | +| isFocused | `boolean` | Renders the focused style. | +| isLoading | `boolean` | Renders a loading [Spinner](../../Spinner). | +| isSuffix | `boolean` | Renders suffix styles. | | onBlur | `function` | `onBlur` event handler. | | onClick | `function` | `onClick` event handler. | | 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"`. | | state | `string` | Applies state styles to the button. | -| submit | `bool` | Sets the `type` of the button to `"submit"`. | +| submit | `boolean` | Sets the `type` of the button to `"submit"`. | | version | `number` | Applies the version `2` variant of the button. | | theme | `string` | Applies a theme based style to the button. | | to | `string` | React Router path to navigate on click. | diff --git a/src/components/Spinner/README.md b/src/components/Spinner/README.md index 13afe17d9..cff7b6f90 100644 --- a/src/components/Spinner/README.md +++ b/src/components/Spinner/README.md @@ -2,7 +2,6 @@ A Spinner component provides the UI to indicate a loading state. - ## Example ```jsx @@ -12,21 +11,13 @@ A Spinner component provides the UI to indicate a loading state. ``` - ## Props -| Prop | Type | Description | -| --- | --- | --- | -| className | `string` | Custom class names to be added to the component. | -| size | `string` | Determines the size of the spinner [Icon](../Icon). Default `md`. | - - -### Sizes - -| Prop | Description | -| --- | --- | -| `xl` | Renders a 32x32 (px) spinner. | -| `lg` | Renders a 24x24 (px) spinner. | -| `md` | Renders a 20x20 (px) spinner. | -| `sm` | Renders a 14x14 (px) spinner. | -| `xs` | Renders a 10x10 (px) spinner. | +| Prop | Type | Description | +| --------- | ----------------- | ---------------------------------------------------------- | +| className | `string` | Custom class names to be added to the component. | +| color | `string` | Color of the spinner. Default `currentColor`. | +| isRounded | `boolean` | Rounds the stroke ends of the spinner SVG. Default `true`. | +| shade | `string` | Determines the opacity of the spinner. | +| size | `number`/`string` | Determines the size of the spinner. Default `16`. | +| speed | `number` | Speed of the spinning animation (in `ms`). Default `1400`. | diff --git a/src/components/Spinner/Spinner.css.ts b/src/components/Spinner/Spinner.css.ts new file mode 100644 index 000000000..837d9375b --- /dev/null +++ b/src/components/Spinner/Spinner.css.ts @@ -0,0 +1,85 @@ +import baseStyles from '../../styles/resets/base.css.js' +import styled from '../styled' + +export const config = { + defaultSize: 16, + shades: { + extraMuted: '0.3', + faint: '0.4', + muted: '0.5', + slightlyMuted: '0.6', + subtle: '0.7', + }, +} + +export const SpinnerUI = styled('div')(props => { + const { spinnerSize } = props + + return ` + ${baseStyles}; + display: block; + width: ${spinnerSize}px; + height: ${spinnerSize}px; + ` +}) + +export const SpinnerSVGUI = styled('svg')(props => { + const { speed, spinnerSize } = props + + return ` + ${baseStyles}; + animation: SpinnerUIAnimation ${speed}ms linear infinite; + display: block; + width: ${spinnerSize}px; + height: ${spinnerSize}px; + x: 0px; + y: 0px; + will-change: transform; + + @keyframes SpinnerUIAnimation { + 100% { + transform: rotate(360deg); + } + } + ` +}) + +export const SpinnerCircleUI = styled('circle')(props => { + const { color, shade, isRounded, speed } = props + + const lineCap = isRounded ? 'round' : 'square' + const opacity = config.shades[shade] + + return ` + ${baseStyles}; + animation: SpinnerCircleUIAnimation ${speed}ms ease-in-out infinite; + display: block; + fill: transparent; + opacity: ${opacity}; + stroke: ${color}; + stroke-width: 3.6; + stroke-linecap: ${lineCap}; + stroke-dasharray: 80px, 200px; + stroke-dashoffset: 0px; + will-change: transform, stroke-dashoffset; + + @keyframes SpinnerCircleUIAnimation { + 0% { + stroke-dasharray: 1px, 200px; + stroke-dashoffset: 0px; + opacity: 0; + } + 0.1% { + opacity: 1; + } + 50% { + stroke-dasharray: 100px, 200px; + stroke-dashoffset: -15px; + } + 100% { + stroke-dasharray: 100px, 200px; + stroke-dashoffset: -125px; + } + } + ` +}) diff --git a/src/components/Spinner/Spinner.tsx b/src/components/Spinner/Spinner.tsx new file mode 100644 index 000000000..22eaead65 --- /dev/null +++ b/src/components/Spinner/Spinner.tsx @@ -0,0 +1,82 @@ +import * as React from 'react' +import VisuallyHidden from '../VisuallyHidden' +import { classNames } from '../../utilities/classNames' +import { isString } from '../../utilities/is' +import { legacySizes } from './Spinner.utils' +import { SpinnerUI, SpinnerSVGUI, SpinnerCircleUI } from './Spinner.css' + +export interface Props { + className?: string + color: string + shade: string + isRounded: boolean + speed: number + size: string | number +} + +export class Spinner extends React.PureComponent { + static className = 'c-Spinner' + static defaultProps = { + color: 'currentColor', + isRounded: true, + shade: 'default', + size: 16, + speed: 1400, + } + + getClassName() { + const { className } = this.props + + return classNames(Spinner.className, className) + } + + getSize(): number { + const { size } = this.props + const defaultSize = 16 + + if (isString(size)) { + return legacySizes[size] || defaultSize + } + + return size + } + + render() { + const { color, shade, isRounded, speed, ...rest } = this.props + const size = this.getSize() + const spinnerSize = size + + return ( + + + + + Loading + + ) + } +} + +export default Spinner diff --git a/src/components/Spinner/Spinner.types.ts b/src/components/Spinner/Spinner.types.ts new file mode 100644 index 000000000..e69de29bb diff --git a/src/components/Spinner/Spinner.utils.ts b/src/components/Spinner/Spinner.utils.ts new file mode 100644 index 000000000..f78766826 --- /dev/null +++ b/src/components/Spinner/Spinner.utils.ts @@ -0,0 +1,7 @@ +export const legacySizes = { + xl: 32, + lg: 24, + md: 20, + sm: 14, + xs: 10, +} diff --git a/src/components/Spinner/__tests__/Spinner.test.tsx b/src/components/Spinner/__tests__/Spinner.test.tsx new file mode 100644 index 000000000..cc4b3bccf --- /dev/null +++ b/src/components/Spinner/__tests__/Spinner.test.tsx @@ -0,0 +1,46 @@ +import * as React from 'react' +import { mount } from 'enzyme' +import { Spinner } from '../Spinner' + +describe('ClassName', () => { + test('Has default className', () => { + const wrapper = mount() + + expect(wrapper.hasClass('c-Spinner')).toBeTruthy() + }) + + test('Applies custom className if specified', () => { + const customClass = 'piano-key-neck-tie' + const wrapper = mount() + + expect(wrapper.prop('className')).toContain(customClass) + }) +}) + +describe('Accessibility', () => { + test('Contains aria-role', () => { + const wrapper = mount() + + expect(wrapper.props()['aria-busy']).toBeTruthy() + }) + + test('Contains hidden loading text', () => { + const wrapper = mount() + const o = wrapper.find('VisuallyHidden') + + expect(o.length).toBeGreaterThan(0) + }) +}) + +describe('Children', () => { + test('Does not render children', () => { + const wrapper = mount( + +
Hello
+ + ) + const el = wrapper.find('div.child') + + expect(el.length).toBe(0) + }) +}) diff --git a/src/components/Spinner/index.js b/src/components/Spinner/index.js deleted file mode 100644 index 035f1bd62..000000000 --- a/src/components/Spinner/index.js +++ /dev/null @@ -1,40 +0,0 @@ -import React from 'react' -import Icon from '../Icon' -import VisuallyHidden from '../VisuallyHidden' -import { classNames } from '../../utilities/classNames' -import { sizeTypes } from './propTypes' - -export const propTypes = { - size: sizeTypes, -} - -const defaultProps = { - size: 'md', -} - -export const iconSizes = { - xl: '32', - lg: '24', - md: '20', - sm: '14', - xs: '10', -} - -const Spinner = props => { - const { className, children, size, ...rest } = props - - const componentClassName = classNames('c-Spinner', className) - - return ( -
- - Loading -
- ) -} - -Spinner.propTypes = propTypes -Spinner.defaultProps = defaultProps -Spinner.displayName = 'Spinner' - -export default Spinner diff --git a/src/components/Spinner/index.ts b/src/components/Spinner/index.ts new file mode 100644 index 000000000..52d2a503a --- /dev/null +++ b/src/components/Spinner/index.ts @@ -0,0 +1,3 @@ +import Spinner from './Spinner' + +export default Spinner diff --git a/src/components/Spinner/propTypes.js b/src/components/Spinner/propTypes.js deleted file mode 100644 index f513d798d..000000000 --- a/src/components/Spinner/propTypes.js +++ /dev/null @@ -1,3 +0,0 @@ -import PropTypes from 'prop-types' - -export const sizeTypes = PropTypes.oneOf(['xl', 'lg', 'md', 'sm', 'xs']) diff --git a/src/components/Spinner/tests/Spinner.test.js b/src/components/Spinner/tests/Spinner.test.js deleted file mode 100644 index 551201d83..000000000 --- a/src/components/Spinner/tests/Spinner.test.js +++ /dev/null @@ -1,65 +0,0 @@ -import React from 'react' -import { shallow } from 'enzyme' -import { default as Spinner, iconSizes } from '../index' -import { Icon, VisuallyHidden } from '../../index' - -describe('ClassName', () => { - test('Has default className', () => { - const wrapper = shallow() - - expect(wrapper.hasClass('c-Spinner')).toBeTruthy() - }) - - test('Applies custom className if specified', () => { - const customClass = 'piano-key-neck-tie' - const wrapper = shallow() - - expect(wrapper.prop('className')).toContain(customClass) - }) -}) - -describe('Accessibility', () => { - test('Contains aria-role', () => { - const wrapper = shallow() - - expect(wrapper.props()['aria-busy']).toBeTruthy() - }) - - test('Contains hidden loading text', () => { - const wrapper = shallow() - const o = wrapper.find(VisuallyHidden) - - expect(o.length).toBeGreaterThan(0) - }) -}) - -describe('Children', () => { - test('Does not render children', () => { - const wrapper = shallow( - -
Hello
- - ) - const el = wrapper.find('div.child') - - expect(el.length).toBe(0) - }) -}) - -describe('Icon', () => { - test('Renders a spinner Icon', () => { - const wrapper = shallow() - const o = wrapper.find(Icon) - - expect(o.length).toBe(1) - expect(o.prop('name')).toBe('spinner') - }) - - test('Can render a different size', () => { - const wrapper = shallow() - const o = wrapper.find(Icon) - - expect(o.length).toBe(1) - expect(o.prop('size')).toBe(iconSizes['xs']) - }) -}) diff --git a/src/styles/components/Spinner.scss b/src/styles/components/Spinner.scss index 4df5a5a3d..1e2667274 100644 --- a/src/styles/components/Spinner.scss +++ b/src/styles/components/Spinner.scss @@ -1,23 +1,2 @@ -.c-Spinner { - @import "../resets/base"; - @import "../configs/color"; - color: _color(grey, 700); - display: inline-block; - - &__icon { - animation-name: BlueSpinnerSpin; - animation-duration: 700ms; - animation-iteration-count: infinite; - animation-timing-function: linear; - transform-origin: center center; - } - - @keyframes BlueSpinnerSpin { - from { - transform: rotate(0deg); - } - to { - transform: rotate(360deg); - } - } -} +// Is now being rendered by Fancy :) +// File exists to preserve @import implementation. diff --git a/stories/ButtonV2.stories.js b/stories/ButtonV2.stories.js index 4c1acb183..c8cae30a6 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 { withKnobs, select } from '@storybook/addon-knobs' +import { boolean, select } from '@storybook/addon-knobs' import { Button, ControlGroup, @@ -12,7 +12,6 @@ import { import styled from '../src/components/styled' const stories = storiesOf('Button/V2', module) -stories.addDecorator(withKnobs) const ContainerUI = styled('div')` background: #f1f3f5; @@ -62,11 +61,40 @@ const makeButtonVariations = (props = {}) => { ) } -stories.add('default', () => ( - -)) +stories.add('Default', () => { + const props = { + isLoading: boolean('isLoading', false), + kind: select( + 'selector', + { + primary: 'primary', + primaryAlt: 'primaryAlt', + secondary: 'secondary', + secondaryAlt: 'secondaryAlt', + default: 'default', + link: 'link', + }, + 'secondary' + ), + size: select( + 'size', + { + xl: 'xl', + lgxl: 'lgxl', + lg: 'lg', + md: 'md', + sm: 'sm', + xs: 'xs', + }, + 'lg' + ), + } + return ( + + ) +}) stories.add('everything', () => ( From d10a5c781dc210d563ea8577ff56d4e5501f79d4 Mon Sep 17 00:00:00 2001 From: Jon Q Date: Mon, 15 Apr 2019 11:54:44 -0400 Subject: [PATCH 2/2] 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 { isLast: false, isSuffix: false, size: 'md', + spinButtonOnLoading: false, submit: false, } @@ -152,6 +154,7 @@ class Button extends Component { isLoading, isSuffix, size, + spinButtonOnLoading, state, submit, theme, @@ -174,6 +177,7 @@ class Button extends Component { 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 { innerRef={this.setInnerRef} type={type} > + {isLoading ? : null} {this.getChildrenMarkup()} - {isLoading ? : null} {this.getFocusMarkup()} 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( - ) + return