From 71e7f130bc6ff2fa06ad9bf9702d4cbe353a2e7f Mon Sep 17 00:00:00 2001
From: Warren Seine <w.seine@criteo.com>
Date: Fri, 24 Apr 2020 20:06:54 +0200
Subject: [PATCH] Generalize color normalization (#1479) (#1483)

Following #1478, a bug was found related to transparent backgrounds. As Excalidraw only supports `transparent` as a valid transparent color, this commits generalizes the use of canvas to normalize color values.

It changes a few details:
- `rgba()` or `hsla()` syntaxes are not accepted anymore
- pasting values goes through the same normalization step, avoiding invalid values
- color validation is not regex-based anymore
- any CSS-valid black color is now accepted (e.g. previously, `rgb(0,0,0)` was rejected)
---
 src/components/ColorPicker.tsx | 50 ++++++++++++++++++++--------------
 1 file changed, 30 insertions(+), 20 deletions(-)

diff --git a/src/components/ColorPicker.tsx b/src/components/ColorPicker.tsx
index 18cd3e09..0701df77 100644
--- a/src/components/ColorPicker.tsx
+++ b/src/components/ColorPicker.tsx
@@ -7,15 +7,23 @@ import { t, getLanguage } from "../i18n";
 import { isWritableElement } from "../utils";
 import colors from "../colors";
 
-const standardizeColor = (
-  value: string,
+const normalizeColor = (
+  color: string,
   canvasContext: CanvasRenderingContext2D,
 ): string | null => {
-  const defaultHexColor = "#000000";
-  canvasContext.fillStyle = value;
+  // Excalidraw only supports "transparent" as a valid transparent value.
+  // Default canvas fill style value is `#000000`, which is also a valid
+  // Excalidraw color. Let's set it to another Canvas-valid but
+  // Excalidraw-invalid value to detect successful normalizations.
+  if (color === "transparent") {
+    return color;
+  }
+
+  const defaultColor = "rgba(0,0,0,0)";
+  canvasContext.fillStyle = defaultColor;
+  canvasContext.fillStyle = color;
   const hexColor = canvasContext.fillStyle;
-  canvasContext.fillStyle = defaultHexColor;
-  return hexColor !== defaultHexColor || value === "black" ? hexColor : null;
+  return hexColor.startsWith("#") ? hexColor : null;
 };
 
 // This is a narrow reimplementation of the awesome react-color Twitter component
@@ -190,7 +198,6 @@ const ColorInput = React.forwardRef(
     },
     ref,
   ) => {
-    const colorRegex = /^([0-9a-f]{3}|[0-9a-f]{6}|[0-9a-f]{8}|transparent)$/;
     const [innerValue, setInnerValue] = React.useState(color);
     const inputRef = React.useRef(null);
     const canvasContext = React.useRef<CanvasRenderingContext2D>(
@@ -203,6 +210,20 @@ const ColorInput = React.forwardRef(
 
     React.useImperativeHandle(ref, () => inputRef.current);
 
+    const changeColor = React.useCallback(
+      (inputValue: string) => {
+        const value = inputValue.toLowerCase();
+        if (canvasContext.current) {
+          const normalizedValue = normalizeColor(value, canvasContext.current);
+          if (normalizedValue) {
+            onChange(normalizedValue);
+          }
+        }
+        setInnerValue(value);
+      },
+      [canvasContext, onChange, setInnerValue],
+    );
+
     return (
       <label className="color-input-container">
         <div className="color-picker-hash">#</div>
@@ -210,20 +231,9 @@ const ColorInput = React.forwardRef(
           spellCheck={false}
           className="color-picker-input"
           aria-label={label}
-          onChange={(event) => {
-            const value = event.target.value.toLowerCase();
-            if (value.match(colorRegex)) {
-              onChange(value === "transparent" ? "transparent" : `#${value}`);
-            } else if (canvasContext.current) {
-              const hexColor = standardizeColor(value, canvasContext.current);
-              if (hexColor) {
-                onChange(hexColor);
-              }
-            }
-            setInnerValue(value);
-          }}
+          onChange={(event) => changeColor(event.target.value)}
           value={(innerValue || "").replace(/^#/, "")}
-          onPaste={(event) => onChange(event.clipboardData.getData("text"))}
+          onPaste={(event) => changeColor(event.clipboardData.getData("text"))}
           onBlur={() => setInnerValue(color)}
           ref={inputRef}
         />