From 4a489b408aa263298103f54d5733894d306a056b Mon Sep 17 00:00:00 2001 From: jamie Date: Wed, 27 Jan 2021 12:15:58 +0000 Subject: [PATCH 1/9] Add hue slider component --- .../PaletteColorPicker/HueSlider.tsx | 176 ++++++++++++++++++ 1 file changed, 176 insertions(+) create mode 100644 common/views/components/PaletteColorPicker/HueSlider.tsx diff --git a/common/views/components/PaletteColorPicker/HueSlider.tsx b/common/views/components/PaletteColorPicker/HueSlider.tsx new file mode 100644 index 0000000000..cf642083d5 --- /dev/null +++ b/common/views/components/PaletteColorPicker/HueSlider.tsx @@ -0,0 +1,176 @@ +import styled from 'styled-components'; +import React, { + FunctionComponent, + useCallback, + useEffect, + useLayoutEffect, + useRef, + useState, +} from 'react'; + +type Props = { + hue: number; + onChangeHue: (value: number) => void; +}; + +const HueBar = styled.div` + position: relative; + width: 100%; + height: 28px; + background: linear-gradient( + to right, + #f00 0%, + #ff0 17%, + #0f0 33%, + #0ff 50%, + #00f 67%, + #f0f 83%, + #f00 100% + ); +`; + +type HandleProps = { + leftOffset: number; +}; + +const Handle = styled.span.attrs(({ leftOffset }) => ({ + style: { + left: `${leftOffset * 100}%`, + }, +}))` + display: inline-block; + position: absolute; + top: 50%; + width: 6px; + height: 24px; + transform: translate(-50%, -50%); + background-color: white; + box-shadow: 0 0 1px rgba(black, 0.5); + border-radius: 2px; +`; + +type InteractionEvent = MouseEvent | TouchEvent; +type ReactInteractionEvent = React.MouseEvent | React.TouchEvent; + +const isTouch = (e: InteractionEvent): e is TouchEvent => 'touches' in e; +const clamp = (x: number, min = 0, max = 1) => + x > max ? max : x < min ? min : x; +const useIsomorphicLayoutEvent = + typeof window === 'undefined' ? useEffect : useLayoutEffect; +const nKeyboardDetents = 50; + +const getPosition = ( + node: T, + event: InteractionEvent +): number => { + const { left, width } = node.getBoundingClientRect(); + const { pageX } = isTouch(event) ? event.touches[0] : event; + return clamp((pageX - (left + window.pageXOffset)) / width); +}; + +const HueSlider: FunctionComponent = ({ + hue, + onChangeHue, + ...otherProps +}) => { + const container = useRef(null); + const hasTouched = useRef(false); + const [isDragging, setIsDragging] = useState(false); + const [position, setPosition] = useState(hue / 360); + + // Prevent mobile from handling mouse events as well as touch + const isValid = (event: InteractionEvent): boolean => { + if (hasTouched.current && !isTouch(event)) { + return false; + } + if (!hasTouched.current) { + hasTouched.current = isTouch(event); + } + return true; + }; + + const handleMove = useCallback((event: InteractionEvent) => { + event.preventDefault(); + const mouseIsDown = isTouch(event) + ? event.touches.length > 0 + : event.buttons > 0; + if (mouseIsDown && container.current) { + const pos = getPosition(container.current, event); + setPosition(pos); + } else { + setIsDragging(false); + } + }, []); + + const handleMoveStart = useCallback( + ({ nativeEvent: event }: ReactInteractionEvent) => { + event.preventDefault(); + if (isValid(event) && container.current) { + const pos = getPosition(container.current, event); + setPosition(pos); + setIsDragging(true); + } + }, + [] + ); + + const handleKeyDown = useCallback( + (event: React.KeyboardEvent) => { + const keyCode = parseInt(event.key, 10) || event.which || event.keyCode; + if (keyCode === 39 /* right */ || keyCode === 37 /* left */) { + event.preventDefault(); + const delta = (keyCode === 39 ? 360 : -360) / nKeyboardDetents; + const nextValue = clamp(hue + delta, 0, 360); + onChangeHue(nextValue); + } + }, + [hue, onChangeHue] + ); + + const handleMoveEnd = useCallback(() => { + setIsDragging(false); + onChangeHue(Math.round(position * 360)); + }, [onChangeHue, position]); + + const toggleDocumentEvents = useCallback( + (attach: boolean) => { + const toggleEvent = attach + ? window.addEventListener + : window.removeEventListener; + toggleEvent(hasTouched.current ? 'touchmove' : 'mousemove', handleMove); + toggleEvent(hasTouched.current ? 'touchend' : 'mouseup', handleMoveEnd); + }, + [handleMove, handleMoveEnd] + ); + + useIsomorphicLayoutEvent(() => { + toggleDocumentEvents(isDragging); + return () => { + if (isDragging) { + toggleDocumentEvents(false); + } + }; + }, [isDragging, toggleDocumentEvents]); + + useEffect(() => { + setPosition(hue / 360); + }, [hue]); + + return ( + + + + ); +}; + +export default HueSlider; From bfebadf0437cfc95e7ce2fbd4daea543d649774c Mon Sep 17 00:00:00 2001 From: jamie Date: Wed, 27 Jan 2021 12:16:37 +0000 Subject: [PATCH 2/9] Add hex/rgb/hsv conversions --- .../PaletteColorPicker/conversions.ts | 82 +++++++++++++++++++ 1 file changed, 82 insertions(+) create mode 100644 common/views/components/PaletteColorPicker/conversions.ts diff --git a/common/views/components/PaletteColorPicker/conversions.ts b/common/views/components/PaletteColorPicker/conversions.ts new file mode 100644 index 0000000000..4f46517fbb --- /dev/null +++ b/common/views/components/PaletteColorPicker/conversions.ts @@ -0,0 +1,82 @@ +type Hsv = { + h: number; + s: number; + v: number; +}; + +type Rgb = { + r: number; + g: number; + b: number; +}; + +const hexToRgb = (hex: string): Rgb => { + const x = parseInt(hex, 16); + return { + r: (x >> 16) & 0xff, + g: (x >> 8) & 0xff, + b: x & 0xff, + }; +}; + +const rgbToHex = (rgb: Rgb): string => + Object.values(rgb).reduce((hex, x) => { + const n = x.toString(16); + return hex + (n.length === 1 ? '0' + n : n); + }, ''); + +// See here for reference: +// https://en.wikipedia.org/wiki/HSL_and_HSV#Color_conversion_formulae + +const rgbToHsv = ({ r, g, b }: Rgb): Hsv => { + const max = Math.max(r, g, b); + const min = Math.min(r, g, b); + const delta = max - min; + + let hh = 0; + if (delta !== 0) { + if (max === r) { + hh = (g - b) / delta; + } else if (max === g) { + hh = 2 + (b - r) / delta; + } else { + hh = 4 + (r - g) / delta; + } + + if (hh < 0) { + hh += 6; + } + } + + return { + h: Math.round(60 * hh), + s: Math.round(max ? (100 * delta) / max : 0), + v: Math.round((100 * max) / 0xff), + }; +}; + +const hsvToRgb = ({ h, s, v }: Hsv): Rgb => { + if (s === 0) { + return { r: v, g: v, b: v }; + } + + h /= 60; + s /= 100; + v /= 100; + + const hh = Math.floor(h); + const m = v * (1 - s); + const n = v * (1 - s * (h - hh)); + const k = v * (1 - s * (1 - h + hh)); + const idx = hh % 6; + + return { + r: Math.round(0xff * [v, n, m, m, k, v][idx]), + g: Math.round(0xff * [k, v, v, n, m, m][idx]), + b: Math.round(0xff * [m, m, k, v, v, n][idx]), + }; +}; + +export const hexToHsv = (hex: string): Hsv => rgbToHsv(hexToRgb(hex)); + +export const hsvToHex = (hsv: Hsv): string => rgbToHex(hsvToRgb(hsv)); From c43d8c4aaac5a53177e99e9368f299f1b4f7f6ab Mon Sep 17 00:00:00 2001 From: jamie Date: Wed, 27 Jan 2021 12:17:56 +0000 Subject: [PATCH 3/9] Disable eslint prop-types rule for typescript --- .eslintrc.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.eslintrc.js b/.eslintrc.js index 259724d293..c3c1420411 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -75,6 +75,9 @@ module.exports = { 'error', { ignoreRestSiblings: true }, ], + // This rule does not support FunctionComponent and so + // makes using (eg) children props more of a pain than it should be + 'react/prop-types': 'off', }, }, ], From ac67615a16fea2e2d602a3b95cd1ec6ab5206b9e Mon Sep 17 00:00:00 2001 From: jamie Date: Wed, 27 Jan 2021 12:18:16 +0000 Subject: [PATCH 4/9] [wip] Add palette color picker component --- .../PaletteColorPicker/PaletteColorPicker.tsx | 64 +++++++++++++++++++ 1 file changed, 64 insertions(+) create mode 100644 common/views/components/PaletteColorPicker/PaletteColorPicker.tsx diff --git a/common/views/components/PaletteColorPicker/PaletteColorPicker.tsx b/common/views/components/PaletteColorPicker/PaletteColorPicker.tsx new file mode 100644 index 0000000000..881ef1015c --- /dev/null +++ b/common/views/components/PaletteColorPicker/PaletteColorPicker.tsx @@ -0,0 +1,64 @@ +import styled from 'styled-components'; +import { FunctionComponent } from 'react'; +import HueSlider from './HueSlider'; +import { hexToHsv, hsvToHex } from './conversions'; + +type Props = { + name: string; + color?: string; + onChangeColor: (color?: string) => void; +}; + +const palette: string[] = [ + 'e02020', + 'ff47d1', + 'fa6400', + 'f7b500', + '8b572a', + '6dd400', + '22bbff', + '8339e8', + '000000', + 'd9d3d3', +]; + +const Swatches = styled.div` + display: flex; + flex-wrap: wrap; +`; + +const Swatch = styled.button<{ color: string; selected: boolean }>` + height: 32px; + width: 32px; + border-radius: 50%; + display: inline-block; + background-color: ${({ color }) => `#${color}`}; + margin: 4px; + border: ${({ selected }) => (selected ? '3px solid #555' : 'none')}; +`; + +const PaletteColorPicker: FunctionComponent = ({ + name, + color = palette[0], + onChangeColor, +}) => ( + <> + + + {palette.map(swatch => ( + onChangeColor(swatch)} + /> + ))} + + onChangeColor(hsvToHex({ h, s: 80, v: 90 }))} + /> + +); + +export default PaletteColorPicker; From bfd157d32f1251c70e600608dccbfe21f676353e Mon Sep 17 00:00:00 2001 From: jamie Date: Wed, 27 Jan 2021 15:27:43 +0000 Subject: [PATCH 5/9] Finish UI and make the component uncontrolled --- .../PaletteColorPicker/PaletteColorPicker.tsx | 95 +++++++++++++++---- 1 file changed, 74 insertions(+), 21 deletions(-) diff --git a/common/views/components/PaletteColorPicker/PaletteColorPicker.tsx b/common/views/components/PaletteColorPicker/PaletteColorPicker.tsx index 881ef1015c..ee1d934dd1 100644 --- a/common/views/components/PaletteColorPicker/PaletteColorPicker.tsx +++ b/common/views/components/PaletteColorPicker/PaletteColorPicker.tsx @@ -1,5 +1,5 @@ import styled from 'styled-components'; -import { FunctionComponent } from 'react'; +import { FunctionComponent, useEffect, useState } from 'react'; import HueSlider from './HueSlider'; import { hexToHsv, hsvToHex } from './conversions'; @@ -22,6 +22,11 @@ const palette: string[] = [ 'd9d3d3', ]; +const Wrapper = styled.div` + padding-top: 6px; + max-width: 250px; +`; + const Swatches = styled.div` display: flex; flex-wrap: wrap; @@ -37,28 +42,76 @@ const Swatch = styled.button<{ color: string; selected: boolean }>` border: ${({ selected }) => (selected ? '3px solid #555' : 'none')}; `; +const Slider = styled(HueSlider)` + margin-top: 15px; +`; + +const ColorLabel = styled.label<{ active: boolean }>` + font-style: italic; + color: ${({ active }) => (active ? '#121212' : '#565656')}; + font-size: 14px; +`; + +const ClearButton = styled.button` + border: 0; + padding: 0; + background: none; + text-decoration: underline; + color: #121212; + font-size: 12px; +`; + +const TextWrapper = styled.div` + display: flex; + justify-content: space-between; + align-items: center; + margin-top: 8px; +`; + const PaletteColorPicker: FunctionComponent = ({ name, - color = palette[0], + color, onChangeColor, -}) => ( - <> - - - {palette.map(swatch => ( - onChangeColor(swatch)} - /> - ))} - - onChangeColor(hsvToHex({ h, s: 80, v: 90 }))} - /> - -); +}) => { + // Because the form is not controlled we need to maintain state internally + const [colorState, setColorState] = useState(color); + + useEffect(() => { + setColorState(color); + }, [color]); + + const handleColorChange = (color?: string) => { + setColorState(color); + onChangeColor(color); + }; + + return ( + + + + {palette.map(swatch => ( + handleColorChange(swatch)} + /> + ))} + + handleColorChange(hsvToHex({ h, s: 80, v: 90 }))} + /> + + + {colorState ? `#${colorState}` : 'None'} + + handleColorChange(undefined)}> + Clear + + + + ); +}; export default PaletteColorPicker; From 7f3f63d67417e2a254cc0a8b70df6d0feca2138c Mon Sep 17 00:00:00 2001 From: jamie Date: Wed, 27 Jan 2021 15:28:10 +0000 Subject: [PATCH 6/9] Add toggle for palette picker and use it --- .../views/components/ColorPicker/ColorPicker.tsx | 2 +- .../components/ModalFilters/ModalFilters.tsx | 16 ++++++++++++++-- .../SearchFiltersDesktop.tsx | 12 +++++++++--- .../SearchFiltersMobile/SearchFiltersMobile.tsx | 12 ++++++++++-- toggles/webapp/toggles.ts | 7 +++++++ 5 files changed, 41 insertions(+), 8 deletions(-) diff --git a/common/views/components/ColorPicker/ColorPicker.tsx b/common/views/components/ColorPicker/ColorPicker.tsx index 94f3cab71b..d2d2bf1ff4 100644 --- a/common/views/components/ColorPicker/ColorPicker.tsx +++ b/common/views/components/ColorPicker/ColorPicker.tsx @@ -12,7 +12,7 @@ import debounce from 'lodash.debounce'; interface Props { name: string; - color: string | null; + color?: string; onChangeColor: (color?: string) => void; } diff --git a/common/views/components/ModalFilters/ModalFilters.tsx b/common/views/components/ModalFilters/ModalFilters.tsx index c21d6ce40f..aa486130bb 100644 --- a/common/views/components/ModalFilters/ModalFilters.tsx +++ b/common/views/components/ModalFilters/ModalFilters.tsx @@ -1,4 +1,10 @@ -import { useState, useRef, FunctionComponent, ReactElement } from 'react'; +import { + useState, + useRef, + FunctionComponent, + ReactElement, + useContext, +} from 'react'; import NextLink from 'next/link'; import dynamic from 'next/dynamic'; import { worksLink } from '../../../services/catalogue/routes'; @@ -14,10 +20,14 @@ import ButtonSolid, { SolidButton, } from '../ButtonSolid/ButtonSolid'; import { SearchFiltersSharedProps } from '../SearchFilters/SearchFilters'; +import TogglesContext from '../TogglesContext/TogglesContext'; -const ColorPicker = dynamic(import('../ColorPicker/ColorPicker'), { +const OldColorPicker = dynamic(import('../ColorPicker/ColorPicker'), { ssr: false, }); +const PaletteColorPicker = dynamic( + import('../PaletteColorPicker/PaletteColorPicker') +); const ActiveFilters = styled(Space).attrs({ h: { @@ -93,6 +103,8 @@ const ModalFilters: FunctionComponent = ({ }: SearchFiltersSharedProps): ReactElement => { const [isActive, setIsActive] = useState(false); const openButtonRef = useRef(null); + const { paletteColorFilter } = useContext(TogglesContext); + const ColorPicker = paletteColorFilter ? PaletteColorPicker : OldColorPicker; function handleOkFiltersButtonClick() { setIsActive(false); diff --git a/common/views/components/SearchFiltersDesktop/SearchFiltersDesktop.tsx b/common/views/components/SearchFiltersDesktop/SearchFiltersDesktop.tsx index f4f9c76628..3d58d6f58d 100644 --- a/common/views/components/SearchFiltersDesktop/SearchFiltersDesktop.tsx +++ b/common/views/components/SearchFiltersDesktop/SearchFiltersDesktop.tsx @@ -9,12 +9,16 @@ import CheckboxRadio from '@weco/common/views/components/CheckboxRadio/CheckboxR import NextLink from 'next/link'; import dynamic from 'next/dynamic'; import { SearchFiltersSharedProps } from '../SearchFilters/SearchFilters'; -import { FunctionComponent, ReactElement, ReactNode } from 'react'; +import { FunctionComponent, ReactElement, ReactNode, useContext } from 'react'; import { searchFilterCheckBox } from '../../../text/arial-labels'; +import TogglesContext from '../TogglesContext/TogglesContext'; -const ColorPicker = dynamic(import('../ColorPicker/ColorPicker'), { +const OldColorPicker = dynamic(import('../ColorPicker/ColorPicker'), { ssr: false, }); +const PaletteColorPicker = dynamic( + import('../PaletteColorPicker/PaletteColorPicker') +); const ColorSwatch = styled.span` display: inline-block; @@ -75,6 +79,8 @@ const SearchFiltersDesktop: FunctionComponent = ({ imagesColor, aggregations, }: SearchFiltersSharedProps): ReactElement => { + const { paletteColorFilter } = useContext(TogglesContext); + const ColorPicker = paletteColorFilter ? PaletteColorPicker : OldColorPicker; const showWorkTypeFilters = workTypeFilters.some(f => f.count > 0) || workTypeInUrlArray.length > 0; @@ -238,7 +244,7 @@ const SearchFiltersDesktop: FunctionComponent = ({ > diff --git a/common/views/components/SearchFiltersMobile/SearchFiltersMobile.tsx b/common/views/components/SearchFiltersMobile/SearchFiltersMobile.tsx index cb77ba93c4..078fbbe335 100644 --- a/common/views/components/SearchFiltersMobile/SearchFiltersMobile.tsx +++ b/common/views/components/SearchFiltersMobile/SearchFiltersMobile.tsx @@ -4,6 +4,7 @@ import { useEffect, FunctionComponent, ReactElement, + useContext, } from 'react'; import dynamic from 'next/dynamic'; import useFocusTrap from '../../../hooks/useFocusTrap'; @@ -26,10 +27,14 @@ import { searchFilterCheckBox, searchFilterCloseButton, } from '../../../text/arial-labels'; +import TogglesContext from '../TogglesContext/TogglesContext'; -const ColorPicker = dynamic(import('../ColorPicker/ColorPicker'), { +const OldColorPicker = dynamic(import('../ColorPicker/ColorPicker'), { ssr: false, }); +const PaletteColorPicker = dynamic( + import('../PaletteColorPicker/PaletteColorPicker') +); const ShameButtonWrap = styled(Space).attrs({ v: { size: 'm', properties: ['padding-top', 'padding-bottom'] }, @@ -239,6 +244,9 @@ const SearchFiltersMobile: FunctionComponent = ({ (productionDatesTo ? 1 : 0) + (imagesColor ? 1 : 0); + const { paletteColorFilter } = useContext(TogglesContext); + const ColorPicker = paletteColorFilter ? PaletteColorPicker : OldColorPicker; + return ( = ({ h={{ size: 'm', properties: ['margin-right'] }} > diff --git a/toggles/webapp/toggles.ts b/toggles/webapp/toggles.ts index ecd770ebf3..6aaf311f4c 100644 --- a/toggles/webapp/toggles.ts +++ b/toggles/webapp/toggles.ts @@ -20,6 +20,13 @@ export default { defaultValue: false, description: 'Search filters will appear in a modal', }, + { + id: 'paletteColorFilter', + title: 'Palette-based colour filter', + description: + 'Use a new colour filtering UI that provides a fixed palette and a hue slider', + defaultValue: false, + }, { id: 'searchToolbar', title: 'Search toolbar', From 3b654616b3d6a7d0ddbc62c7556231b74367b171 Mon Sep 17 00:00:00 2001 From: jamie Date: Wed, 27 Jan 2021 15:39:09 +0000 Subject: [PATCH 7/9] Uppercase color hex string --- .../views/components/PaletteColorPicker/PaletteColorPicker.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common/views/components/PaletteColorPicker/PaletteColorPicker.tsx b/common/views/components/PaletteColorPicker/PaletteColorPicker.tsx index ee1d934dd1..e8567dbf06 100644 --- a/common/views/components/PaletteColorPicker/PaletteColorPicker.tsx +++ b/common/views/components/PaletteColorPicker/PaletteColorPicker.tsx @@ -104,7 +104,7 @@ const PaletteColorPicker: FunctionComponent = ({ /> - {colorState ? `#${colorState}` : 'None'} + {colorState ? `#${colorState.toUpperCase()}` : 'None'} handleColorChange(undefined)}> Clear From 122015dd72a1623ea1da08189bb97a02ef845ce0 Mon Sep 17 00:00:00 2001 From: jamie Date: Wed, 27 Jan 2021 16:01:35 +0000 Subject: [PATCH 8/9] Don't let dynamic imports break in jest --- common/babel.js | 6 ++++++ common/test/setupTests.js | 13 +++++++++++++ 2 files changed, 19 insertions(+) diff --git a/common/babel.js b/common/babel.js index 2db852517d..fd9c1c56bb 100644 --- a/common/babel.js +++ b/common/babel.js @@ -10,9 +10,15 @@ module.exports = function(api) { }, ], ]; + const env = { + test: { + plugins: ['dynamic-import-node'], + }, + }; return { presets, plugins, + env, }; }; diff --git a/common/test/setupTests.js b/common/test/setupTests.js index 82edfc9e5a..fde3f9bdb4 100644 --- a/common/test/setupTests.js +++ b/common/test/setupTests.js @@ -2,3 +2,16 @@ import { configure } from 'enzyme'; import Adapter from 'enzyme-adapter-react-16'; configure({ adapter: new Adapter() }); + +// This is required for dynamic imports to work in jest +// Solution from here: https://github.com/vercel/next.js/discussions/18855 +jest.mock('next/dynamic', () => (func: () => Promise) => { + let component: any = null; + func().then((module: any) => { + component = module.default; + }); + const DynamicComponent = (...args) => component(...args); + DynamicComponent.displayName = 'LoadableComponent'; + DynamicComponent.preload = jest.fn(); + return DynamicComponent; +}); From 74aff9ba8d6c3c1666f87facc00c18e5535f4ab3 Mon Sep 17 00:00:00 2001 From: jamie Date: Wed, 27 Jan 2021 16:44:41 +0000 Subject: [PATCH 9/9] Make buttons non-submitting, but make sure the submit actually happens --- common/utils/numeric.ts | 2 ++ .../PaletteColorPicker/HueSlider.tsx | 3 +-- .../PaletteColorPicker/PaletteColorPicker.tsx | 27 ++++++++++++------- 3 files changed, 20 insertions(+), 12 deletions(-) create mode 100644 common/utils/numeric.ts diff --git a/common/utils/numeric.ts b/common/utils/numeric.ts new file mode 100644 index 0000000000..3a863cd5e8 --- /dev/null +++ b/common/utils/numeric.ts @@ -0,0 +1,2 @@ +export const clamp = (x: number, min = 0, max = 1): number => + x > max ? max : x < min ? min : x; diff --git a/common/views/components/PaletteColorPicker/HueSlider.tsx b/common/views/components/PaletteColorPicker/HueSlider.tsx index cf642083d5..f92e82cd7f 100644 --- a/common/views/components/PaletteColorPicker/HueSlider.tsx +++ b/common/views/components/PaletteColorPicker/HueSlider.tsx @@ -7,6 +7,7 @@ import React, { useRef, useState, } from 'react'; +import { clamp } from '../../../utils/numeric'; type Props = { hue: number; @@ -53,8 +54,6 @@ type InteractionEvent = MouseEvent | TouchEvent; type ReactInteractionEvent = React.MouseEvent | React.TouchEvent; const isTouch = (e: InteractionEvent): e is TouchEvent => 'touches' in e; -const clamp = (x: number, min = 0, max = 1) => - x > max ? max : x < min ? min : x; const useIsomorphicLayoutEvent = typeof window === 'undefined' ? useEffect : useLayoutEffect; const nKeyboardDetents = 50; diff --git a/common/views/components/PaletteColorPicker/PaletteColorPicker.tsx b/common/views/components/PaletteColorPicker/PaletteColorPicker.tsx index e8567dbf06..efcb9b4ad1 100644 --- a/common/views/components/PaletteColorPicker/PaletteColorPicker.tsx +++ b/common/views/components/PaletteColorPicker/PaletteColorPicker.tsx @@ -1,5 +1,5 @@ import styled from 'styled-components'; -import { FunctionComponent, useEffect, useState } from 'react'; +import { FunctionComponent, useEffect, useRef, useState } from 'react'; import HueSlider from './HueSlider'; import { hexToHsv, hsvToHex } from './conversions'; @@ -32,7 +32,10 @@ const Swatches = styled.div` flex-wrap: wrap; `; -const Swatch = styled.button<{ color: string; selected: boolean }>` +const Swatch = styled.button.attrs({ type: 'button' })<{ + color: string; + selected: boolean; +}>` height: 32px; width: 32px; border-radius: 50%; @@ -52,7 +55,7 @@ const ColorLabel = styled.label<{ active: boolean }>` font-size: 14px; `; -const ClearButton = styled.button` +const ClearButton = styled.button.attrs({ type: 'button' })` border: 0; padding: 0; background: none; @@ -75,15 +78,19 @@ const PaletteColorPicker: FunctionComponent = ({ }) => { // Because the form is not controlled we need to maintain state internally const [colorState, setColorState] = useState(color); + const firstRender = useRef(true); useEffect(() => { setColorState(color); }, [color]); - const handleColorChange = (color?: string) => { - setColorState(color); - onChangeColor(color); - }; + useEffect(() => { + if (!firstRender.current) { + onChangeColor(color); + } else { + firstRender.current = false; + } + }, [colorState]); return ( @@ -94,19 +101,19 @@ const PaletteColorPicker: FunctionComponent = ({ key={swatch} color={swatch} selected={colorState === swatch} - onClick={() => handleColorChange(swatch)} + onClick={() => setColorState(swatch)} /> ))} handleColorChange(hsvToHex({ h, s: 80, v: 90 }))} + onChangeHue={h => setColorState(hsvToHex({ h, s: 80, v: 90 }))} /> {colorState ? `#${colorState.toUpperCase()}` : 'None'} - handleColorChange(undefined)}> + setColorState(undefined)}> Clear