Improve composite inputs (#2794)

sumup-oss · Nov 27, 2024 · 604c012 · 604c012
1 parent b864e42
commit 604c012
Show file tree

Hide file tree

Showing 13 changed files with 413 additions and 123 deletions.
diff --git a/.changeset/slimy-spoons-rule.md b/.changeset/slimy-spoons-rule.md
@@ -0,0 +1,5 @@
+---
+"@sumup-oss/circuit-ui": minor
+---
+
+Added support for typing and pasting 3-character hex code into the ColorInput component.
diff --git a/.changeset/swift-frogs-laugh.md b/.changeset/swift-frogs-laugh.md
@@ -0,0 +1,5 @@
+---
+"@sumup-oss/circuit-ui": minor
+---
+
+Improved parsing of the PhoneNumberInput component's `value` and `defaultValue` props.
diff --git a/packages/circuit-ui/components/ColorInput/ColorInput.spec.tsx b/packages/circuit-ui/components/ColorInput/ColorInput.spec.tsx
@@ -105,16 +105,17 @@ describe('ColorInput', () => {
     });
 
     it('should ignore an invalid value', () => {
-      render(<ColorInput {...baseProps} value="#fff" />);
+      render(<ColorInput {...baseProps} value="#ffg" />);
       const [colorInput, textInput] = screen.getAllByLabelText(baseProps.label);
 
       expect(colorInput).toHaveValue('#000000');
-      expect(textInput).toHaveValue('fff');
+      expect(textInput).toHaveValue('ffg');
     });
   });
 
   describe('user interactions', () => {
     const newValue = '00ff00';
+
     it('should update text input if color input changes', async () => {
       const onChange = vi.fn();
       render(<ColorInput {...baseProps} onChange={onChange} />);
@@ -124,6 +125,13 @@ describe('ColorInput', () => {
 
       expect(textInput).toHaveValue(newValue.replace('#', ''));
       expect(onChange).toHaveBeenCalledTimes(1);
+      expect(onChange).toHaveBeenCalledWith(
+        expect.objectContaining({
+          target: expect.objectContaining({
+            value: `#${newValue}`,
+          }),
+        }),
+      );
     });
 
     it('should update color input if text input changes', async () => {
@@ -134,40 +142,67 @@ describe('ColorInput', () => {
       await userEvent.type(textInput, newValue);
 
       expect(colorInput).toHaveValue(`#${newValue}`);
-      expect(onChange).toHaveBeenCalled();
+      expect(onChange).toHaveBeenCalledTimes(2);
+      expect(onChange).toHaveBeenCalledWith(
+        expect.objectContaining({
+          target: expect.objectContaining({
+            value: `#${newValue}`,
+          }),
+        }),
+      );
     });
 
     it('should handle paste events', async () => {
-      render(<ColorInput {...baseProps} />);
+      const onChange = vi.fn();
+      render(<ColorInput {...baseProps} onChange={onChange} />);
       const [colorInput, textInput] = screen.getAllByLabelText(baseProps.label);
 
       await userEvent.click(textInput);
       await userEvent.paste(`#${newValue}`);
 
       expect(colorInput).toHaveValue(`#${newValue}`);
       expect(textInput).toHaveValue(newValue);
+      expect(onChange).toHaveBeenCalledTimes(1);
+      expect(onChange).toHaveBeenCalledWith(
+        expect.objectContaining({
+          target: expect.objectContaining({
+            value: `#${newValue}`,
+          }),
+        }),
+      );
     });
 
     it('should ignore invalid paste event', async () => {
-      render(<ColorInput {...baseProps} />);
+      const onChange = vi.fn();
+      render(<ColorInput {...baseProps} onChange={onChange} />);
       const [colorInput, textInput] = screen.getAllByLabelText(baseProps.label);
 
       await userEvent.click(textInput);
       await userEvent.paste('obviously invalid');
 
       expect(colorInput).toHaveValue('#000000');
-      expect(textInput).toHaveValue('');
+      expect(textInput).toHaveValue('obviou');
+      expect(onChange).not.toHaveBeenCalled();
     });
 
     it("should allow pasting color without '#'", async () => {
-      render(<ColorInput {...baseProps} />);
+      const onChange = vi.fn();
+      render(<ColorInput {...baseProps} onChange={onChange} />);
       const [colorInput, textInput] = screen.getAllByLabelText(baseProps.label);
 
       await userEvent.click(textInput);
       await userEvent.paste(newValue);
 
       expect(colorInput).toHaveValue(`#${newValue}`);
       expect(textInput).toHaveValue(newValue);
+      expect(onChange).toHaveBeenCalledTimes(1);
+      expect(onChange).toHaveBeenCalledWith(
+        expect.objectContaining({
+          target: expect.objectContaining({
+            value: `#${newValue}`,
+          }),
+        }),
+      );
     });
   });
 });
diff --git a/packages/circuit-ui/components/ColorInput/ColorInput.stories.tsx b/packages/circuit-ui/components/ColorInput/ColorInput.stories.tsx
@@ -25,7 +25,6 @@ export default {
 
 const baseArgs = {
   label: 'Color',
-  placeholder: '#99ffbb',
   defaultValue: '#99ffbb',
 };
 
@@ -55,9 +54,10 @@ export const Validations = (args: ColorInputProps) => (
     />
     <ColorInput
       {...args}
-      defaultValue="#fff"
+      defaultValue=""
       invalid
-      validationHint="Value must be a 6 character hexadecimal color"
+      required
+      validationHint="Please enter a color"
     />
     <ColorInput
       {...args}

diff --git a/packages/circuit-ui/components/ColorInput/ColorInput.tsx b/packages/circuit-ui/components/ColorInput/ColorInput.tsx
@@ -17,6 +17,8 @@
 
 import {
   forwardRef,
+  useCallback,
+  useEffect,
   useId,
   useRef,
   type ChangeEventHandler,
@@ -36,6 +38,11 @@ import { applyMultipleRefs } from '../../util/refs.js';
 import { changeInputValue } from '../../util/input-value.js';
 
 import classes from './ColorInput.module.css';
+import {
+  isSameColor,
+  isValidColor,
+  normalizeColor,
+} from './ColorInputService.js';
 
 export interface ColorInputProps
   extends Omit<
@@ -76,7 +83,6 @@ export const ColorInput = forwardRef<HTMLInputElement, ColorInputProps>(
       hasWarning,
       showValid,
       hideLabel,
-      id,
       invalid,
       label,
       onChange,
@@ -101,55 +107,54 @@ export const ColorInput = forwardRef<HTMLInputElement, ColorInputProps>(
 
     const descriptionIds = clsx(validationHintId, descriptionId);
 
+    const updatePickerValue = useCallback((color: string) => {
+      if (!colorPickerRef.current || !isValidColor(color)) {
+        return;
+      }
+
+      changeInputValue(colorPickerRef.current, normalizeColor(color));
+    }, []);
+
+    const updateInputValue = useCallback((color?: string) => {
+      if (!colorInputRef.current || !color) {
+        return;
+      }
+
+      const currentColor = colorInputRef.current.value;
+
+      if (!isSameColor(currentColor, color)) {
+        changeInputValue(colorInputRef.current, color.trim().replace('#', ''));
+      }
+    }, []);
+
+    useEffect(() => {
+      updateInputValue(value);
+    }, [updateInputValue, value]);
+
     const handlePaste: ClipboardEventHandler<HTMLInputElement> = (event) => {
       if (!colorPickerRef.current || !colorInputRef.current || readOnly) {
         return;
       }
 
-      event.preventDefault();
-
       const pastedText = event.clipboardData.getData('text/plain').trim();
 
-      if (!pastedText || !/^#?[0-9A-F]{6}$/i.test(pastedText)) {
+      if (!pastedText || !isValidColor(pastedText)) {
         return;
       }
 
-      const pastedColor = pastedText.startsWith('#')
-        ? pastedText
-        : `#${pastedText}`;
-
-      colorPickerRef.current.value = pastedColor;
-      colorPickerRef.current.dispatchEvent(
-        new Event('change', { bubbles: true }),
-      );
+      event.preventDefault();
 
-      changeInputValue(colorInputRef.current, pastedColor.replace('#', ''));
+      updatePickerValue(pastedText);
+      updateInputValue(pastedText);
     };
 
-    const onPickerColorChange: ChangeEventHandler<HTMLInputElement> = (
-      event,
-    ) => {
-      if (colorInputRef.current) {
-        colorInputRef.current.value = event.target.value.replace('#', '');
-      }
-      if (onChange) {
-        onChange(event);
-      }
+    const onPickerChange: ChangeEventHandler<HTMLInputElement> = (event) => {
+      onChange?.(event);
+      updateInputValue(event.target.value);
     };
 
     const onInputChange: ChangeEventHandler<HTMLInputElement> = (event) => {
-      if (colorPickerRef.current) {
-        colorPickerRef.current.value = `#${event.target.value}`;
-      }
-      if (onChange) {
-        onChange({
-          ...event,
-          target: {
-            ...event.target,
-            value: `#${event.target.value}`,
-          },
-        });
-      }
+      updatePickerValue(event.target.value);
     };
 
     return (
@@ -174,16 +179,18 @@ export const ColorInput = forwardRef<HTMLInputElement, ColorInputProps>(
               type="color"
               aria-labelledby={labelId}
               aria-describedby={descriptionIds}
+              aria-invalid={invalid ? 'true' : undefined}
               className={classes['color-input']}
-              onChange={onPickerColorChange}
+              onChange={onPickerChange}
               disabled={disabled || readOnly}
-              defaultValue={defaultValue}
-              value={value}
+              required={required}
+              defaultValue={defaultValue && normalizeColor(defaultValue)}
+              value={value && normalizeColor(value)}
+              {...props}
             />
           </label>
           <span className={classes.symbol}>#</span>
           <input
-            id={id}
             ref={colorInputRef}
             type="text"
             aria-labelledby={labelId}
@@ -194,18 +201,16 @@ export const ColorInput = forwardRef<HTMLInputElement, ColorInputProps>(
               classes.input,
               inputClassName,
             )}
-            aria-invalid={invalid && 'true'}
+            aria-invalid={invalid ? 'true' : undefined}
             required={required}
             maxLength={6}
             pattern="[0-9a-f]{3,6}"
             readOnly={readOnly}
             disabled={disabled}
-            value={value?.replace('#', '')}
-            defaultValue={defaultValue?.replace('#', '')}
+            defaultValue={(defaultValue || value)?.replace('#', '')}
             placeholder={placeholder?.replace('#', '')}
             onChange={onInputChange}
             onPaste={handlePaste}
-            {...props}
           />
         </div>
         <FieldValidationHint