chore(circuit-ui): DSYS-823 | mark phone number & color inputs as sta…

…ble (#2731) * mark ColorInput and PhoneNumberInput as Stable. Improve readonly ColorInput styles * Revise PhoneNumberInput docs * add stories * remove irrelevant props * refactor and add tests * update docs --------- Co-authored-by: Connor Bär <[email protected]>
sumup-oss · Oct 29, 2024 · a4b5d2c · a4b5d2c
1 parent c15abee
commit a4b5d2c
Show file tree

Hide file tree

Showing 11 changed files with 212 additions and 172 deletions.
diff --git a/.changeset/hip-comics-agree.md b/.changeset/hip-comics-agree.md
@@ -0,0 +1,15 @@
+---
+"@sumup-oss/circuit-ui": major
+---
+
+Marked the `ColorInput` and `PhoneNumberInput` components as stable. Update the related imports:
+
+```diff
+- import { ColorInput, type ColorInputProps } from '@sumup-oss/circuit-ui/experimental';
++ import { ColorInput, type ColorInputProps } from '@sumup-oss/circuit-ui';
+```
+
+```diff
+- import { PhoneNumberInput, type PhoneNumberInputProps } from '@sumup-oss/circuit-ui/experimental';
++ import { PhoneNumberInput, type PhoneNumberInputProps } from '@sumup-oss/circuit-ui';
+```
diff --git a/.changeset/witty-rabbits-collect.md b/.changeset/witty-rabbits-collect.md
@@ -0,0 +1,5 @@
+---
+"@sumup-oss/eslint-plugin-circuit-ui": minor
+---
+
+Updated the `component-lifecycle-imports` ESLint rule to handle imports of `ColorInput` and `PhoneNumberInput` as experimental components.
diff --git a/packages/circuit-ui/components/ColorInput/ColorInput.mdx b/packages/circuit-ui/components/ColorInput/ColorInput.mdx
@@ -5,9 +5,46 @@ import * as Stories from './ColorInput.stories';
 
 # ColorInput
 
-<Status variant="experimental" />
+## Use cases and API
+<Status variant="stable" />
 
-The ColorInput component enables users to type or select a color.
+The ColorInput component enables users to type a color in a text field or select it using the browser native color picker.
+
+Use the ColorInput component to allow users to input a color value to personalize content such as UI themes, profile settings or other customizable experiences.
+
+
+The component only supports seven-character string representations of colors in hexadecimal format. Shorthand Hex values like `#fff` will not be normalized to `#ffffff` and values with alpha channel like `##ffffff50` will not be accepted.
+- Accepted: `#aabbcc`, `#AABBCC`
+- Not accepted: `#abc`, `#aabbccaa`, `#ABC`, `#AABBCCAA`
 
 <Story of={Stories.Base} />
+
 <Props />
+
+## Validations
+
+Use the `validationHint` prop to communication information about the state of the ColorInput component, along with one of the following validation props:
+- stand-alone: The user is informed of the expected response.
+- `showWarning`: The user is warned that the value is not recommended but still valid.
+- `showError`: The user is alerted that the value is invalid.
+- `showValid`: The user is reassured that the value is valid. Use sparingly.
+
+<Story of={Stories.Validations} />
+
+## Optional
+
+Use the `optionalLabel` prop to indicate that the field is optional. This can help reduce the cognitive load for the user by clearly indicating which fields are required and which are not. This label is only displayed when the `required` prop is falsy.
+
+<Story of={Stories.Optional} />
+
+## Readonly
+
+The ColorInput component supports a read-only state. Use the `readOnly` prop to indicate that the field is not currently editable.
+The read-only state is applied to the text input field of the ColorInput component, while disabling the color picker input to prevent interactions with the element.
+
+<Story of={Stories.Readonly} />
+
+## Disabled
+
+Disabled form fields can be confusing to users, so use them with caution. Use a disabled color input only if you need to communicate to the user that an option that existed before is not available for choosing now. Consider not displaying the field at all or add an explanation why the field is disabled.
+<Story of={Stories.Disabled} />
diff --git a/packages/circuit-ui/components/ColorInput/ColorInput.spec.tsx b/packages/circuit-ui/components/ColorInput/ColorInput.spec.tsx
@@ -28,21 +28,20 @@ import type { InputElement } from '../Input/index.js';
 import { ColorInput } from './ColorInput.js';
 
 describe('ColorInput', () => {
-  const baseProps = { label: 'Car color', pickerLabel: 'Pick car color' };
+  const baseProps = { label: 'Car color' };
 
   it('should merge a custom class name with the default ones', () => {
     const className = 'foo';
-    const { container } = render(
-      <ColorInput {...baseProps} inputClassName={className} />,
-    );
-    const input = container.querySelector('input[type="text"]');
-    expect(input?.className).toContain(className);
+    render(<ColorInput {...baseProps} inputClassName={className} />);
+    // eslint-disable-next-line @typescript-eslint/no-unused-vars
+    const [_, textInput] = screen.getAllByLabelText(baseProps.label);
+    expect(textInput?.className).toContain(className);
   });
 
   it('should forward a ref', () => {
     const ref = createRef<InputElement>();
-    const { container } = render(<ColorInput {...baseProps} ref={ref} />);
-    const input = container.querySelector("input[type='color']");
+    render(<ColorInput {...baseProps} ref={ref} />);
+    const [input] = screen.getAllByLabelText(baseProps.label);
     expect(ref.current).toBe(input);
   });
 
@@ -52,7 +51,7 @@ describe('ColorInput', () => {
     expect(actual).toHaveNoViolations();
   });
 
-  describe('Labeling', () => {
+  describe('semantics', () => {
     it('should accept a custom description via aria-describedby', () => {
       const customDescription = 'Custom description';
       const customDescriptionId = 'customDescriptionId';
@@ -66,133 +65,110 @@ describe('ColorInput', () => {
         customDescription,
       );
     });
-  });
+    it('should render as disabled', async () => {
+      render(<ColorInput {...baseProps} disabled />);
+      const [colorInput, textInput] = screen.getAllByLabelText(baseProps.label);
+
+      expect(colorInput).toBeDisabled();
+      expect(textInput).toBeDisabled();
+    });
+    it('should render as read-only', async () => {
+      render(<ColorInput {...baseProps} readOnly />);
+      const [colorInput, textInput] = screen.getAllByLabelText(baseProps.label);
+      expect(colorInput).toBeDisabled();
+      expect(textInput).toHaveAttribute('readonly');
+    });
 
-  it('should set value and default value on both inputs', () => {
-    const { container } = render(
-      <ColorInput {...baseProps} defaultValue="#ff11bb" />,
-    );
-    const colorPicker = container.querySelector(
-      "input[type='color']",
-    ) as HTMLInputElement;
-    const colorInput = container.querySelector(
-      "input[type='text']",
-    ) as HTMLInputElement;
-    expect(colorPicker.value).toBe('#ff11bb');
-    expect(colorInput.value).toBe('ff11bb');
+    it('should render as required', async () => {
+      render(<ColorInput {...baseProps} required />);
+      // eslint-disable-next-line @typescript-eslint/no-unused-vars
+      const [_, textInput] = screen.getAllByLabelText(baseProps.label);
+      expect(textInput).toBeRequired(); // text input
+    });
   });
 
-  describe('Synchronization', () => {
-    it('should update text input if color input changes', async () => {
-      const { container } = render(<ColorInput {...baseProps} />);
-      const colorPicker = container.querySelector(
-        "input[type='color']",
-      ) as HTMLInputElement;
-      const newValue = '#00ff00';
-
-      fireEvent.input(colorPicker, { target: { value: newValue } });
-
-      const colorInput = container.querySelector(
-        "input[type='text']",
-      ) as HTMLInputElement;
-      expect(colorInput.value).toBe(newValue.replace('#', ''));
+  describe('state', () => {
+    it('should display a default value on both inputs', () => {
+      render(<ColorInput {...baseProps} defaultValue="#ff11bb" />);
+      const [colorInput, textInput] = screen.getAllByLabelText(baseProps.label);
+
+      expect(colorInput).toHaveValue('#ff11bb');
+      expect(textInput).toHaveValue('ff11bb');
     });
 
-    it('should update color input if text input changes', async () => {
-      const { container } = render(<ColorInput {...baseProps} />);
-      const colorInput = container.querySelector(
-        "input[type='text']",
-      ) as HTMLInputElement;
-      const newValue = '00ff00';
-
-      await userEvent.type(colorInput, newValue);
-
-      const colorPicker = container.querySelector(
-        "input[type='color']",
-      ) as HTMLInputElement;
-      expect(colorPicker.value).toBe(`#${newValue}`);
+    it('should display an initial value', () => {
+      render(<ColorInput {...baseProps} value="#ff11bb" />);
+
+      const [colorInput, textInput] = screen.getAllByLabelText(baseProps.label);
+
+      expect(colorInput).toHaveValue('#ff11bb');
+      expect(textInput).toHaveValue('ff11bb');
+    });
+
+    it('should ignore an invalid value', () => {
+      render(<ColorInput {...baseProps} value="#fff" />);
+      const [colorInput, textInput] = screen.getAllByLabelText(baseProps.label);
+
+      expect(colorInput).toHaveValue('#000000');
+      expect(textInput).toHaveValue('fff');
     });
   });
 
-  describe('OnChange events', () => {
-    it('should trigger onChange event when color picker changes', async () => {
+  describe('user interactions', () => {
+    const newValue = '00ff00';
+    it('should update text input if color input changes', async () => {
       const onChange = vi.fn();
-      const { container } = render(
-        <ColorInput {...baseProps} onChange={onChange} />,
-      );
+      render(<ColorInput {...baseProps} onChange={onChange} />);
+      const [colorInput, textInput] = screen.getAllByLabelText(baseProps.label);
 
-      const colorPicker = container.querySelector(
-        "input[type='color']",
-      ) as HTMLInputElement;
-
-      fireEvent.input(colorPicker, { target: { value: '#00ff00' } });
+      fireEvent.input(colorInput, { target: { value: `#${newValue}` } });
 
+      expect(textInput).toHaveValue(newValue.replace('#', ''));
       expect(onChange).toHaveBeenCalledTimes(1);
     });
 
-    it('should trigger onChange event when color hex input changes', async () => {
+    it('should update color input if text input changes', async () => {
       const onChange = vi.fn();
-      const { container } = render(
-        <ColorInput {...baseProps} onChange={onChange} />,
-      );
+      render(<ColorInput {...baseProps} onChange={onChange} />);
+      const [colorInput, textInput] = screen.getAllByLabelText(baseProps.label);
 
-      const colorInput = container.querySelector(
-        "input[type='text']",
-      ) as HTMLInputElement;
-
-      await userEvent.type(colorInput, '00ff00');
+      await userEvent.type(textInput, newValue);
 
+      expect(colorInput).toHaveValue(`#${newValue}`);
       expect(onChange).toHaveBeenCalled();
     });
-  });
 
-  describe('Paste', () => {
     it('should handle paste events', async () => {
-      const { container } = render(<ColorInput {...baseProps} />);
-      const colorInput = container.querySelector(
-        "input[type='text']",
-      ) as HTMLInputElement;
-
-      await userEvent.click(colorInput);
-      await userEvent.paste('#00ff00');
-
-      const colorPicker = container.querySelector(
-        "input[type='color']",
-      ) as HTMLInputElement;
-      expect(colorPicker.value).toBe('#00ff00');
-      expect(colorInput.value).toBe('00ff00');
+      render(<ColorInput {...baseProps} />);
+      const [colorInput, textInput] = screen.getAllByLabelText(baseProps.label);
+
+      await userEvent.click(textInput);
+      await userEvent.paste(`#${newValue}`);
+
+      expect(colorInput).toHaveValue(`#${newValue}`);
+      expect(textInput).toHaveValue(newValue);
     });
 
     it('should ignore invalid paste event', async () => {
-      const { container } = render(<ColorInput {...baseProps} />);
-      const colorInput = container.querySelector(
-        "input[type='text']",
-      ) as HTMLInputElement;
+      render(<ColorInput {...baseProps} />);
+      const [colorInput, textInput] = screen.getAllByLabelText(baseProps.label);
 
-      await userEvent.click(colorInput);
+      await userEvent.click(textInput);
       await userEvent.paste('obviously invalid');
 
-      const colorPicker = container.querySelector(
-        "input[type='color']",
-      ) as HTMLInputElement;
-      expect(colorPicker.value).toBe('#000000');
-      expect(colorInput.value).toBe('');
+      expect(colorInput).toHaveValue('#000000');
+      expect(textInput).toHaveValue('');
     });
 
     it("should allow pasting color without '#'", async () => {
-      const { container } = render(<ColorInput {...baseProps} />);
-      const colorInput = container.querySelector(
-        "input[type='text']",
-      ) as HTMLInputElement;
-
-      await userEvent.click(colorInput);
-      await userEvent.paste('00ff00');
-
-      const colorPicker = container.querySelector(
-        "input[type='color']",
-      ) as HTMLInputElement;
-      expect(colorPicker.value).toBe('#00ff00');
-      expect(colorInput.value).toBe('00ff00');
+      render(<ColorInput {...baseProps} />);
+      const [colorInput, textInput] = screen.getAllByLabelText(baseProps.label);
+
+      await userEvent.click(textInput);
+      await userEvent.paste(newValue);
+
+      expect(colorInput).toHaveValue(`#${newValue}`);
+      expect(textInput).toHaveValue(newValue);
     });
   });
 });
diff --git a/packages/circuit-ui/components/ColorInput/ColorInput.stories.tsx b/packages/circuit-ui/components/ColorInput/ColorInput.stories.tsx
@@ -13,6 +13,8 @@
  * limitations under the License.
  */
 
+import { Stack } from '../../../../.storybook/components/index.js';
+
 import { ColorInput, type ColorInputProps } from './ColorInput.js';
 
 export default {
@@ -22,13 +24,47 @@ export default {
 
 const baseArgs = {
   label: 'Color',
-  pickerLabel: 'Pick color',
   placeholder: '#99ffbb',
   defaultValue: '#99ffbb',
 };
 
-export const Base = (args: ColorInputProps) => (
-  <ColorInput {...args} style={{ maxWidth: '250px' }} />
-);
+export const Base = (args: ColorInputProps) => <ColorInput {...args} />;
 
 Base.args = baseArgs;
+
+export const Optional = (args: ColorInputProps) => <ColorInput {...args} />;
+
+Optional.args = { ...baseArgs, optionalLabel: 'optional' };
+
+export const Readonly = (args: ColorInputProps) => <ColorInput {...args} />;
+
+Readonly.args = { ...baseArgs, readOnly: true };
+
+export const Disabled = (args: ColorInputProps) => <ColorInput {...args} />;
+
+Disabled.args = { ...baseArgs, disabled: true };
+
+export const Validations = (args: ColorInputProps) => (
+  <Stack>
+    <ColorInput
+      {...args}
+      defaultValue="#0096FF"
+      hasWarning
+      validationHint="Blue is not a Teletubby color :( "
+    />
+    <ColorInput
+      {...args}
+      defaultValue="#fff"
+      invalid
+      validationHint="Value must be a 6 character hexadecimal color"
+    />
+    <ColorInput
+      {...args}
+      defaultValue="#4a288d"
+      showValid
+      validationHint="Tinky-Winky!"
+    />
+  </Stack>
+);
+
+Validations.args = baseArgs;