Switch to HSL(A) based input and output to fix HSL UX

WordPress · sarayourfriend · Aug 10, 2021 · Jul 23, 2021 · Jul 26, 2021 · Jul 27, 2021
commit f405c0aab73ea2e9e2258c2badf2090bd10e9c34
@@ -1,7 +1,7 @@
 /**
  * External dependencies
  */
-import colorize from 'tinycolor2';
+import colorize, { ColorFormats } from 'tinycolor2';
 
 /**
  * WordPress dependencies
@@ -20,13 +20,13 @@ import type { ColorType } from './types';
 import { space } from '../utils/space';
 
 interface ColorDisplayProps {
-	color: string;
+	color: ColorFormats.HSLA;
 	colorType: ColorType;
 	enableAlpha: boolean;
 }
 
 interface DisplayProps {
-	color: string;
+	color: ColorFormats.HSLA;
 	enableAlpha: boolean;
 }
 
@@ -80,8 +80,14 @@ const RgbDisplay = ( { color, enableAlpha }: DisplayProps ) => {
 	return <ValueDisplay values={ values } />;
 };
 
-const HexDisplay = ( { color }: DisplayProps ) => {
-	const colorWithoutHash = color.slice( 1 ).toUpperCase();
+const HexDisplay = ( { color, enableAlpha }: DisplayProps ) => {
+	const colorized = colorize( color );
+	const colorWithoutHash = ( enableAlpha
+		? colorized.toHex8String()
+		: colorized.toHexString()
+	)
+		.slice( 1 )
+		.toUpperCase();
 	return (
 		<FlexItem>
 			<Text color="blue">#</Text>
@@ -121,17 +127,22 @@ export const ColorDisplay = ( {
 				}
 				default:
 				case 'hex': {
-					return color.toUpperCase();
+					const colorized = colorize( color );
+					return enableAlpha
+						? colorized.toHex8String()
+						: colorized.toHexString();
 				}
 			}
 		},
-		() => setCopiedColor( color )
+		() => setCopiedColor( colorize( color ).toHex8String() )
 	);
 	return (
 		<Tooltip
 			content={
 				<Text color="white">
-					{ copiedColor === color ? __( 'Copied!' ) : __( 'Copy' ) }
+					{ copiedColor === colorize( color ).toHex8String()
+						? __( 'Copied!' )
+						: __( 'Copy' ) }
 				</Text>
 			}
 		>

@@ -1,3 +1,8 @@
+/**
+ * External dependencies
+ */
+import type { ColorFormats } from 'tinycolor2';
+
 /**
  * Internal dependencies
  */
@@ -7,8 +12,8 @@ import { HexInput } from './hex-input';
 
 interface ColorInputProps {
 	colorType: 'hsl' | 'hex' | 'rgb';
-	color: string;
-	onChange: ( value: string ) => void;
+	color: ColorFormats.HSLA;
+	onChange: ( value: ColorFormats.HSLA ) => void;
 	enableAlpha: boolean;
 }
 

@@ -3,7 +3,7 @@
  */
 // eslint-disable-next-line no-restricted-imports
 import type { Ref } from 'react';
-import colorize from 'tinycolor2';
+import type { ColorFormats } from 'tinycolor2';
 
 /**
  * WordPress dependencies
@@ -24,19 +24,18 @@ import { HStack } from '../../h-stack';
 import Button from '../../button';
 import { Spacer } from '../../spacer';
 import { ColorfulWrapper, SelectControl } from './styles';
-import { HexColorPicker } from './hex-color-picker';
-import { RgbaColorPicker } from './rgba-color-picker';
 import { ColorDisplay } from './color-display';
 import { ColorInput } from './color-input';
+import { Picker } from './picker';
 import { useControlledValue } from '../../utils/hooks';
 
 import type { ColorType } from './types';
 
 interface ColorPickerProps {
 	enableAlpha?: boolean;
-	color?: string;
-	onChange?: ( hexColor: string ) => void;
-	defaultValue?: string;
+	color?: ColorFormats.HSL | ColorFormats.HSLA;
+	onChange?: ( hexColor: ColorFormats.HSL | ColorFormats.HSLA ) => void;
+	defaultValue?: ColorFormats.HSL | ColorFormats.HSLA;
 	copyFormat?: ColorType;
 }
 
@@ -46,9 +45,10 @@ const options = [
 	{ label: 'Hex', value: 'hex' as const },
 ];
 
-const getSafeColor = ( color: string | undefined, enableAlpha: boolean ) => {
-	const def = enableAlpha ? '#ffffffff' : '#ffffff';
-	return ! color ? def : color;
+const getSafeColor = (
+	color: ColorFormats.HSL | ColorFormats.HSLA | undefined
+): ColorFormats.HSLA => {
+	return color ? { a: 1, ...color } : { h: 0, s: 0, l: 100, a: 1 };
 };
 
 const ColorPicker = (
@@ -72,27 +72,27 @@ const ColorPicker = (
 	// Debounce to prevent rapid changes from conflicting with one another.
 	const debouncedSetColor = useDebounce( setColor );
 
-	const handleChange = ( nextValue: string ) => {
-		debouncedSetColor(
-			enableAlpha
-				? colorize( nextValue ).toHex8String()
-				: colorize( nextValue ).toHexString()
-		);
+	const handleChange = (
+		nextValue: ColorFormats.HSLA | ColorFormats.HSL
+	) => {
+		debouncedSetColor( nextValue );
 	};
 
-	// Use a safe default value for the color and remove the possibility of `undefined`. This is what react-colorful defaults to if it's not passed anything so it's the safest default
-	const safeColor = getSafeColor( color, enableAlpha );
+	// Use a safe default value for the color and remove the possibility of `undefined`.
+	const safeColor = getSafeColor( color );
 
 	const [ showInputs, setShowInputs ] = useState< boolean >( false );
 	const [ colorType, setColorType ] = useState< ColorType >(
 		copyFormat || 'hex'
 	);
 
-	const Picker = enableAlpha ? RgbaColorPicker : HexColorPicker;
-
 	return (
 		<ColorfulWrapper ref={ forwardedRef }>
-			<Picker onChange={ handleChange } color={ safeColor } />
+			<Picker
+				onChange={ handleChange }
+				color={ safeColor }
+				enableAlpha={ enableAlpha }
+			/>
 			<HStack justify="space-between">
 				{ showInputs ? (
 					<SelectControl

diff --git a/packages/components/src/ui/color-picker/hex-color-picker.tsx b/packages/components/src/ui/color-picker/hex-color-picker.tsx
@@ -1,7 +1,7 @@
 /**
  * External dependencies
  */
-import colorize from 'tinycolor2';
+import colorize, { ColorFormats } from 'tinycolor2';
 
 /**
  * Internal dependencies
@@ -12,8 +12,8 @@ import InputControl from '../../input-control';
 import { space } from '../utils/space';
 
 interface HexInputProps {
-	color: string;
-	onChange: ( value: string ) => void;
+	color: ColorFormats.HSLA;
+	onChange: ( value: ColorFormats.HSLA ) => void;
 	enableAlpha: boolean;
 }
 
@@ -24,6 +24,11 @@ export const HexInput = ( { color, onChange, enableAlpha }: HexInputProps ) => {
 		}
 	};
 
+	const colorized = colorize( color );
+	const value = enableAlpha
+		? colorized.toHex8String()
+		: colorized.toHexString();
+
 	return (
 		<InputControl
 			__unstableInputWidth="8em"
@@ -32,9 +37,9 @@ export const HexInput = ( { color, onChange, enableAlpha }: HexInputProps ) => {
 					#
 				</Spacer>
 			}
-			value={ color.slice( 1 ).toUpperCase() }
+			value={ value.slice( 1 ).toUpperCase() }
 			onChange={ ( nextValue ) =>
-				onChange( colorize( nextValue ).toHex8String() )
+				onChange( colorize( nextValue ).toHsl() )
 			}
 			onValidate={ handleValidate }
 			maxLength={ enableAlpha ? 8 : 6 }

@@ -1,21 +1,21 @@
 /**
  * External dependencies
  */
-import colorize from 'tinycolor2';
+import type { ColorFormats } from 'tinycolor2';
 
 /**
  * Internal dependencies
  */
 import { InputWithSlider } from './input-with-slider';
 
 interface HslInputProps {
-	color: string;
-	onChange: ( color: string ) => void;
+	color: ColorFormats.HSLA;
+	onChange: ( color: ColorFormats.HSLA ) => void;
 	enableAlpha: boolean;
 }
 
 export const HslInput = ( { color, onChange, enableAlpha }: HslInputProps ) => {
-	const { h, s, l, a } = colorize( color ).toHsl();
+	const { h, s, l, a } = color;
 
 	return (
 		<>
@@ -26,31 +26,27 @@ export const HslInput = ( { color, onChange, enableAlpha }: HslInputProps ) => {
 				abbreviation="H"
 				value={ Math.trunc( h ) }
 				onChange={ ( nextH: number ) =>
-					onChange( colorize( { h: nextH, s, l, a } ).toHex8String() )
+					onChange( { h: nextH, s, l, a } )
 				}
 			/>
 			<InputWithSlider
 				min={ 0 }
 				max={ 100 }
 				label="Saturation"
 				abbreviation="S"
-				value={ Math.trunc( 100 * s ) }
+				value={ s <= 1 ? Math.trunc( 100 * s ) : s }
 				onChange={ ( nextS: number ) =>
-					onChange(
-						colorize( { h, s: nextS / 100, l, a } ).toHex8String()
-					)
+					onChange( { h, s: nextS / 100, l, a } )
 				}
 			/>
 			<InputWithSlider
 				min={ 0 }
 				max={ 100 }
 				label="Lightness"
 				abbreviation="L"
-				value={ Math.trunc( 100 * l ) }
+				value={ l <= 1 ? Math.trunc( 100 * l ) : l }
 				onChange={ ( nextL: number ) =>
-					onChange(
-						colorize( { h, s, l: nextL / 100, a } ).toHex8String()
-					)
+					onChange( { h, s, l: nextL / 100, a } )
 				}
 			/>
 			{ enableAlpha && (
@@ -61,14 +57,12 @@ export const HslInput = ( { color, onChange, enableAlpha }: HslInputProps ) => {
 					abbreviation="A"
 					value={ Math.trunc( 100 * a ) }
 					onChange={ ( nextA: number ) =>
-						onChange(
-							colorize( {
-								h,
-								s,
-								l,
-								a: nextA / 100,
-							} ).toHex8String()
-						)
+						onChange( {
+							h,
+							s,
+							l,
+							a: nextA / 100,
+						} )
 					}
 				/>
 			) }

@@ -0,0 +1,26 @@
+/**
+ * External dependencies
+ */
+import type { ColorFormats } from 'tinycolor2';
+import { HslColorPicker, HslaColorPicker } from 'react-colorful';
+
+interface PickerProps {
+	color: ColorFormats.HSL | ColorFormats.HSLA;
+	enableAlpha: boolean;
+	onChange: ( nextColor: ColorFormats.HSL | ColorFormats.HSLA ) => void;
+}
+
+export const Picker = ( { color, enableAlpha, onChange }: PickerProps ) => {
+	// RC accepts s and l as a range from 0 - 100, whereas tinycolor expects a decimal value representing the percentage
+	// so we need to make sure that we're giving RC the correct color format that it expects
+	const reactColorfulColor = {
+		h: color.h,
+		s: color.s <= 1 ? Math.floor( color.s * 100 ) : color.s,
+		l: color.l <= 1 ? Math.floor( color.l * 100 ) : color.l,
+		a: ( color as ColorFormats.HSLA ).a,
+	};
+
+	const Component = enableAlpha ? HslaColorPicker : HslColorPicker;
+
+	return <Component color={ reactColorfulColor } onChange={ onChange } />;
+};