From 1ea848d5f24aa02fa3cc31b18730ab6a51aed947 Mon Sep 17 00:00:00 2001 From: Adam Hooper Date: Thu, 11 Oct 2018 17:31:13 -0400 Subject: [PATCH] Replace react-color [finishes #160456513] and reduces dev JS blob by 200kb --- __mocks__/reactstrap/lib/Popover.js | 23 +++ assets/css/components/Input_components.scss | 60 +++---- assets/js/ColorPicker.js | 163 ++++++++++++++++++ .../WfHamburgerMenu.test.js.snap | 1 + .../__snapshots__/WorkflowNavBar.test.js.snap | 1 + .../js/__snapshots__/workflows.test.js.snap | 5 + .../WfModuleContextMenu.test.js.snap | 1 + assets/js/wfparameters/ChartSeriesSelect.js | 62 +++---- .../js/wfparameters/ChartSeriesSelect.test.js | 29 +--- .../ChartSeriesMultiSelect.test.js.snap | 144 ++++++++++------ .../ChartSeriesSelect.test.js.snap | 72 +++++--- package-lock.json | 40 +---- package.json | 3 +- 13 files changed, 390 insertions(+), 214 deletions(-) create mode 100644 __mocks__/reactstrap/lib/Popover.js create mode 100644 assets/js/ColorPicker.js diff --git a/__mocks__/reactstrap/lib/Popover.js b/__mocks__/reactstrap/lib/Popover.js new file mode 100644 index 000000000..646197acc --- /dev/null +++ b/__mocks__/reactstrap/lib/Popover.js @@ -0,0 +1,23 @@ +import React from 'react' + +export default class Popover extends React.PureComponent { + render () { + const { innerClassName, isOpen } = this.props + if (!isOpen) return null + + const props = { ...this.props } + delete props.isOpen + delete props.innerClassName + delete props.toggle + delete props.target + delete props.boundariesElement + + props.className = innerClassName || null + + return ( +
+
+
+ ) + } +} diff --git a/assets/css/components/Input_components.scss b/assets/css/components/Input_components.scss index 2d88cabf7..9574a9aba 100644 --- a/assets/css/components/Input_components.scss +++ b/assets/css/components/Input_components.scss @@ -314,42 +314,32 @@ textarea[name="notes"], .editable-notes-read-only { margin:auto; } -.twitter-picker { - // Override react-color's style. We can't set `width` because react-color - // uses JS to set styles. But we can set `max-width`. - max-width: 24rem; - - // Center horizontally - // (it's already position:relative) - left: -.6rem; - - // Place vertically so triangle is just next to button - top: .3rem; - - // Can't adjust 'border' or 'box-shadow'. TwitterPicker does, but far too - // pale. Instead, use '::before' and '::after' to create a triangle pointer - // ... can we all agree that reactcss is terrible for customization? - &::before { - content: ''; - position: absolute; - top: 0; - left: 0; - width: 100%; - height: 100%; - border: 1px solid $light-gray; +.color-picker-popover { + button.color-choice { + height: 3rem; + width: 3rem; + cursor: pointer; + border-radius: .4rem; + margin: 0 .6rem .6rem 0; + border: none; } - &::after { - content: ''; - position: absolute; - top: -.5rem; - left: 1.1rem; // centered on button - width: 1rem; - height: 1rem; - background: white; - transform: rotate(-45deg); - border: 1px solid transparent; - border-right-color: $light-gray; - border-top-color: $light-gray; + button.choose-custom-color { + // These override Bootstrap's styles + transition: none; // animating color changes is so annoying + border: none; + width: 3rem; + } + + input { + width: 6em; + font-size: 1.4rem; + height: 3rem; + border: 1px solid $light-gray; + border-radius: .4rem; + + &:invalid { + border-color: $error-prompt; + } } } diff --git a/assets/js/ColorPicker.js b/assets/js/ColorPicker.js new file mode 100644 index 000000000..82b374bc1 --- /dev/null +++ b/assets/js/ColorPicker.js @@ -0,0 +1,163 @@ +import React from 'react' +import PropTypes from 'prop-types' +import Button from 'reactstrap/lib/Button' +import InputGroup from 'reactstrap/lib/InputGroup' +import InputGroupAddon from 'reactstrap/lib/InputGroupAddon' +import Input from 'reactstrap/lib/Input' +import Popover from 'reactstrap/lib/Popover' +import PopoverBody from 'reactstrap/lib/PopoverBody' + +class ColorChoice extends React.PureComponent { + static propTypes = { + color: PropTypes.string.isRequired, // like '#abcdef' + onClick: PropTypes.func.isRequired, // onClick('#abcdef') => undefined + } + + onClick = () => { + this.props.onClick(this.props.color) + } + + render () { + const { color } = this.props + const name = 'color-' + color.slice(1) + + return ( + + ) + } +} + + +/** + * a + + + + ) + } +} + + +/** + * A simulation for ` undefined + } + + state = { + isOpen: false + } + + buttonRef = React.createRef() + + toggleOpen = () => { + this.setState({ isOpen: !this.state.isOpen }) + } + + close = () => { + this.setState({ isOpen: false }) + } + + onChange = (color) => { + this.props.onChange(color) + this.setState({ isOpen: false }) + } + + render () { + const { name, value, choices } = this.props + const { isOpen } = this.state + const safeValue = value || '#000000' + + return ( +
+ + { isOpen ? ( + + + {choices.map(color => ( + + ))} + + + + ) : null } +
+ ) + } +} diff --git a/assets/js/__snapshots__/WfHamburgerMenu.test.js.snap b/assets/js/__snapshots__/WfHamburgerMenu.test.js.snap index 2b44db250..680884dca 100644 --- a/assets/js/__snapshots__/WfHamburgerMenu.test.js.snap +++ b/assets/js/__snapshots__/WfHamburgerMenu.test.js.snap @@ -59,6 +59,7 @@ exports[`WfHamburgerMenu renders logged in, non-read only 1`] = ` - - + + - - { this.state.colorPickerOpen ?
-
- -
- : null } - + + + ) } } diff --git a/assets/js/wfparameters/ChartSeriesSelect.test.js b/assets/js/wfparameters/ChartSeriesSelect.test.js index d3da7f0af..7ea937426 100644 --- a/assets/js/wfparameters/ChartSeriesSelect.test.js +++ b/assets/js/wfparameters/ChartSeriesSelect.test.js @@ -2,7 +2,6 @@ import React from 'react' import ChartSeriesSelect from './ChartSeriesSelect' import { mount } from 'enzyme' -import { sleep } from '../test-utils' describe('ChartSeriesSelect', () => { function wrapper (props = {}) { @@ -20,21 +19,6 @@ describe('ChartSeriesSelect', () => { ) } - /** - * Sleep long enough for react-color's "onChangeComplete" to fire. - * - * Usage: - * - * async () => { - * wrapper.find('[color]').simulate('click') - * await sleepThroughDebounce() - * expect(callback).toHaveBeenCalled() - * } - */ - function sleepThroughDebounce () { - return sleep(100) - } - it('should match snapshot', () => { expect(wrapper()).toMatchSnapshot() }) @@ -45,24 +29,21 @@ describe('ChartSeriesSelect', () => { expect(w.prop('onChange')).toHaveBeenCalledWith({ index: 2, column: 'bar', color: '#abcdef' }) }) - it('should change color', async () => { + it('should change color', () => { const w = wrapper() w.find('button[title="Pick color"]').simulate('click') - w.find('div[title="#fbaa6d"]').simulate('click') - await sleepThroughDebounce() + w.find('button[name="color-fbaa6d"]').simulate('click') expect(w.prop('onChange')).toHaveBeenCalledWith({ index: 2, column: 'foo', color: '#fbaa6d' }) // test that picker disappears w.update() - expect(w.find('div[title="#FBAA6D"]')).toHaveLength(0) + expect(w.find('button[name="color-fbaa6d"]')).toHaveLength(0) }) - it('should postpone color change until column is set', async () => { + it('should postpone color change until column is set', () => { const w = wrapper({ column: null, color: null }) w.find('button[title="Pick color"]').simulate('click') - w.update() - w.find('div[title="#fbaa6d"]').simulate('click') - await sleepThroughDebounce() + w.find('button[name="color-fbaa6d"]').simulate('click') expect(w.prop('onChange')).not.toHaveBeenCalled() w.find('ColumnParam[name="column"]').at(0).prop('onChange')('bar') expect(w.prop('onChange')).toHaveBeenCalledWith({ index: 2, column: 'bar', color: '#fbaa6d' }) diff --git a/assets/js/wfparameters/__snapshots__/ChartSeriesMultiSelect.test.js.snap b/assets/js/wfparameters/__snapshots__/ChartSeriesMultiSelect.test.js.snap index d512d9969..8d1c27713 100644 --- a/assets/js/wfparameters/__snapshots__/ChartSeriesMultiSelect.test.js.snap +++ b/assets/js/wfparameters/__snapshots__/ChartSeriesMultiSelect.test.js.snap @@ -70,34 +70,56 @@ exports[`ChartSeriesMultiSelect should match snapshot 1`] = `
- - + + +
+
- - + + +
+ - - + + + +