add comment

sumup-oss · Sep 8, 2024 · 897e526 · 897e526
1 parent 49ca3f7
commit 897e526
Show file tree

Hide file tree

Showing 4 changed files with 164 additions and 73 deletions.
diff --git a/package.json b/package.json
@@ -27,6 +27,7 @@
     "lint:fix": "biome check --write && foundry run eslint . --ext .js,.jsx,.ts,.tsx --fix",
     "lint:ci": "biome ci && foundry run eslint . --ext .js,.jsx,.ts,.tsx --quiet ",
     "lint:css": "foundry run stylelint '**/*.css'",
+    "lint:css:fix": "foundry run stylelint '**/*.css' --fix",
     "dev": "npm run docs:start",
     "docs": "npm run docs:start",
     "docs:start": "storybook dev -p 6006",
@@ -95,4 +96,4 @@
     "vitest": "^2.0.3",
     "vitest-github-actions-reporter": "^0.11.1"
   }
-}
+}
diff --git a/packages/circuit-ui/components/ColorInput/ColorInput.module.css b/packages/circuit-ui/components/ColorInput/ColorInput.module.css
@@ -1,36 +1,76 @@
-.suffix {
-  overflow: hidden;
-  border-top-right-radius: var(--cui-border-radius-byte);
-  border-bottom-right-radius: var(--cui-border-radius-byte);
-  pointer-events: auto !important;
-  border: none;
-  border-left: 1px solid var(--cui-border-normal);
+.wrapper {
+  position: relative;
+  display: flex;
+}
+
+.picker {
   width: var(--cui-spacings-exa);
   height: var(--cui-spacings-exa);
-  position: absolute;
-  top: 0;
-  right: 0;
-  box-shadow: none;
+  border-top-left-radius: var(--cui-border-radius-byte);
+  border-bottom-left-radius: var(--cui-border-radius-byte);
+  box-shadow: 0 0 0 1px var(--cui-border-normal);
 }
 
-.prefix {
-  display: flex;
-  align-items: center;
-  justify-content: center;
-  line-height: var(--cui-spacings-mega);
+.picker:hover {
+  background: var(--cui-bg-normal-hovered);
+  box-shadow: 0 0 0 1px var(--cui-border-normal-hovered);
 }
 
-.colorInput {
-  opacity: 0;
-  width: var(--cui-spacings-exa);
-  height: var(--cui-spacings-exa);
+.picker:focus-within {
+  background: var(--cui-bg-normal-pressed);
+  box-shadow: 0 0 0 1px var(--cui-border-normal-pressed);
+}
+
+.color-input {
+  width: var(--cui-spacings-giga);
+  height: var(--cui-spacings-giga);
+  padding: 0;
+  margin: var(--cui-spacings-kilo);
+  appearance: none;
   border: none;
-  box-shadow: none;
+  border-radius: 6px;
+  outline: none;
+  box-shadow: 0 0 0 1px var(--cui-border-normal);
+}
+
+.color-input::-moz-color-swatch {
+  border: none;
+}
+
+.color-input::-webkit-color-swatch-wrapper {
   padding: 0;
+  border-radius: 0;
+}
+
+.color-input::-webkit-color-swatch {
+  border: none;
+}
+
+.picker:hover .color-input {
+  box-shadow: 0 0 0 1px var(--cui-border-normal-hovered);
+}
+
+.picker:focus-within .color-input {
+  box-shadow: 0 0 0 1px var(--cui-border-normal-pressed);
+}
+
+.symbol {
+  position: absolute;
+  top: 0;
+  left: var(--cui-spacings-exa);
+  display: grid;
+  place-items: center center;
+  width: var(--cui-spacings-giga);
+  height: var(--cui-spacings-exa);
+  font-family: var(--cui-font-stack-mono);
+  color: var(--cui-fg-subtle);
 }
 
 .input {
+  padding-left: var(--cui-spacings-giga);
   font-family: var(--cui-font-stack-mono);
+  border-top-left-radius: 0;
+  border-bottom-left-radius: 0;
 }
 
 .input::placeholder {
@@ -40,7 +80,3 @@
 .colorpick {
   display: inline-block;
 }
-
-.colorpick:focus-within input {
-  box-shadow: 0 0 0 2px var(--cui-border-accent);
-}
diff --git a/packages/circuit-ui/components/ColorInput/ColorInput.spec.tsx b/packages/circuit-ui/components/ColorInput/ColorInput.spec.tsx
@@ -16,7 +16,7 @@
 import { describe, expect, it } from 'vitest';
 import { createRef } from 'react';
 
-import { render, axe } from '../../util/test-utils.js';
+import { render, axe, screen } from '../../util/test-utils.js';
 import type { InputElement } from '../Input/index.js';
 
 import { ColorInput } from './ColorInput.js';
@@ -36,4 +36,29 @@ describe('ColorInput', () => {
     const actual = await axe(container);
     expect(actual).toHaveNoViolations();
   });
+
+  describe('Labeling', () => {
+    const HEX_SYMBOL = '#';
+
+    it('should have the currency symbol as part of its accessible description', () => {
+      render(<ColorInput {...baseProps} />);
+      expect(screen.getByRole('textbox')).toHaveAccessibleDescription(
+        HEX_SYMBOL,
+      );
+    });
+
+    it('should accept a custom description via aria-describedby', () => {
+      const customDescription = 'Custom description';
+      const customDescriptionId = 'customDescriptionId';
+      render(
+        <>
+          <span id={customDescriptionId}>{customDescription}</span>
+          <ColorInput {...baseProps} aria-describedby={customDescriptionId} />
+        </>,
+      );
+      expect(screen.getByRole('textbox')).toHaveAccessibleDescription(
+        `${HEX_SYMBOL} ${customDescription}`,
+      );
+    });
+  });
 });
diff --git a/packages/circuit-ui/components/ColorInput/ColorInput.tsx b/packages/circuit-ui/components/ColorInput/ColorInput.tsx
@@ -17,18 +17,17 @@
 
 import {
   forwardRef,
-  useCallback,
-  useEffect,
   useId,
   useRef,
   useState,
   type ChangeEventHandler,
 } from 'react';
 
-import { Input, type InputElement, type InputProps } from '../Input/index.js';
+import type { InputElement, InputProps } from '../Input/index.js';
 import { clsx } from '../../styles/clsx.js';
-import { FieldLabel, FieldLabelText } from '../Field/index.js';
+import { FieldLabel, FieldLabelText, FieldWrapper } from '../Field/index.js';
 import { applyMultipleRefs } from '../../util/refs.js';
+import classes from '../Input/Input.module.css';
 
 import styles from './ColorInput.module.css';
 
@@ -43,7 +42,6 @@ export interface ColorInputProps
     | 'maxLength'
     | 'pattern'
     | 'renderPrefix'
-    | 'renderSuffix'
     | 'as'
   > {
   /**
@@ -67,15 +65,43 @@ export interface ColorInputProps
 
 export const ColorInput = forwardRef<InputElement, ColorInputProps>(
   (
-    { onChange, className, value, defaultValue, pickerLabel, ...props },
+    {
+      'aria-describedby': descriptionId,
+      'renderSuffix': RenderSuffix,
+      className,
+      defaultValue,
+      disabled,
+      hasWarning,
+      hideLabel,
+      id,
+      invalid,
+      label,
+      onChange,
+      optionalLabel,
+      pickerLabel,
+      readOnly,
+      required,
+      style,
+      value,
+      ...props
+    },
     ref,
   ) => {
     const [currentColor, setCurrentColor] = useState<string | undefined>(
       defaultValue,
     );
-    const colorDisplayRef = useRef<HTMLDivElement>(null);
     const colorPickerRef = useRef<InputElement>(null);
+
     const pickerId = useId();
+    const hexSymbolId = useId();
+    const inputFallbackId = useId();
+    const inputId = id || inputFallbackId;
+
+    const descriptionIds = clsx(hexSymbolId, descriptionId);
+
+    const suffix = RenderSuffix && <RenderSuffix className={classes.suffix} />;
+
+    const hasSuffix = Boolean(suffix);
 
     const onPickerColorChange: ChangeEventHandler<InputElement> = (e) => {
       setCurrentColor(e.target.value);
@@ -91,50 +117,53 @@ export const ColorInput = forwardRef<InputElement, ColorInputProps>(
       setCurrentColor(`#${e.target.value}`);
     };
 
-    useEffect(() => {
-      if (colorDisplayRef.current && currentColor) {
-        colorDisplayRef.current.style.backgroundColor = currentColor;
-      }
-    }, [currentColor]);
-
-    const renderSuffix = useCallback(
-      () => (
-        <div
-          className={clsx(styles.suffix)}
-          ref={colorDisplayRef}
-          style={{ backgroundColor: currentColor }}
-        >
-          <FieldLabel htmlFor={pickerId}>
+    return (
+      <FieldWrapper className={className} style={style} disabled={disabled}>
+        <FieldLabel htmlFor={inputId}>
+          <FieldLabelText
+            label={label}
+            hideLabel={hideLabel}
+            optionalLabel={optionalLabel}
+            required={required}
+          />
+        </FieldLabel>
+        <div className={styles.wrapper}>
+          <FieldLabel htmlFor={pickerId} className={styles.picker}>
             <FieldLabelText label={pickerLabel} hideLabel />
+            <input
+              type="color"
+              id={pickerId}
+              className={styles['color-input']}
+              onChange={onPickerColorChange}
+              ref={applyMultipleRefs(colorPickerRef, ref)}
+              aria-controls={inputId}
+              readOnly={readOnly}
+            />
           </FieldLabel>
+          <span className={styles.symbol} id={hexSymbolId}>
+            #
+          </span>
           <input
-            type="color"
-            id={pickerId}
-            className={styles.colorInput}
-            onChange={onPickerColorChange}
-            ref={applyMultipleRefs(colorPickerRef, ref)}
+            id={inputId}
+            aria-describedby={descriptionIds}
+            className={clsx(
+              classes.base,
+              !disabled && hasWarning && classes.warning,
+              hasSuffix && classes['has-suffix'],
+              styles.input,
+            )}
+            aria-invalid={invalid && 'true'}
+            required={required}
+            disabled={disabled}
+            maxLength={6}
+            pattern="[0-9a-f]{3,6}"
+            readOnly={readOnly}
+            value={currentColor ? currentColor.replace('#', '') : undefined}
+            onChange={onInputChange}
+            {...props}
           />
         </div>
-      ),
-      [],
-    );
-
-    return (
-      <Input
-        className={styles.colorpick}
-        renderPrefix={({ className: cn }) => (
-          <div className={clsx(cn, styles.prefix)}>
-            <span>#</span>
-          </div>
-        )}
-        renderSuffix={renderSuffix}
-        value={currentColor ? currentColor.replace('#', '') : undefined}
-        inputClassName={styles.input}
-        maxLength={6}
-        pattern="[0-9a-f]{3,6}"
-        onChange={onInputChange}
-        {...props}
-      />
+      </FieldWrapper>
     );
   },
 );