refactor: improve color picker (#4029)

Color control can be a thin wrapper around color picker

- fixed "inherit" keyword support
- moved intermediate value into color picker
- merge highlight and change low level events

apps/builder/app/builder/features/style-panel/controls/color/color-control.tsx

@@ -1,44 +1,13 @@
 import { useStore } from "@nanostores/react";
-import { useMemo, useState } from "react";
+import { useMemo } from "react";
 import { Flex } from "@webstudio-is/design-system";
 import { toValue } from "@webstudio-is/css-engine";
-import type { RgbValue, StyleValue } from "@webstudio-is/css-engine";
-import { colord } from "colord";
-import {
-  type CssColorPickerValueInput,
-  ColorPicker,
-} from "../../shared/color-picker";
-import { getStyleSource } from "../../shared/style-info";
+import { ColorPicker } from "../../shared/color-picker";
 import { styleConfigByName } from "../../shared/configs";
 import type { ControlProps } from "../types";
 import { AdvancedValueTooltip } from "../advanced-value-tooltip";
 import { createComputedStyleDeclStore } from "../../shared/model";
 
-const parseColor = (color?: StyleValue): RgbValue => {
-  const colordValue = colord(toValue(color));
-
-  if (colordValue.isValid()) {
-    const rgb = colordValue.toRgb();
-    return {
-      type: "rgb",
-      r: rgb.r,
-      g: rgb.g,
-      b: rgb.b,
-      alpha: rgb.a ?? 1,
-    };
-  }
-
-  // @todo what to show as default?
-  // Default to black
-  return {
-    type: "rgb",
-    r: 0,
-    g: 0,
-    b: 0,
-    alpha: 1,
-  };
-};
-
 export const ColorControl = ({
   property,
   items,
@@ -53,20 +22,12 @@ export const ColorControl = ({
   );
   const computedStyleDecl = useStore($computedStyleDecl);
 
-  const [intermediateValue, setIntermediateValue] =
-    useState<CssColorPickerValueInput>();
-
   const { items: defaultItems } = styleConfigByName(property);
-  const styleInfo = currentStyle[property];
 
   const setValue = setProperty(property);
 
-  let value = computedStyleDecl.cascadedValue;
-  if (value.type !== "rgb" && value.type !== "keyword") {
-    // Support previously set colors
-    value = parseColor(value);
-  }
-  const currentColor = parseColor(computedStyleDecl.usedValue);
+  const value = computedStyleDecl.cascadedValue;
+  const currentColor = computedStyleDecl.usedValue;
 
   return (
     <Flex align="center" gap="1">
@@ -82,38 +43,13 @@ export const ColorControl = ({
           currentColor={currentColor}
           property={property}
           value={value}
-          styleSource={getStyleSource(styleInfo)}
           keywords={(items ?? defaultItems).map((item) => ({
             type: "keyword",
             value: item.name,
           }))}
-          intermediateValue={intermediateValue}
-          onChange={(styleValue) => {
-            setIntermediateValue(styleValue);
-
-            if (styleValue === undefined) {
-              deleteProperty(property, { isEphemeral: true });
-              return;
-            }
-
-            if (styleValue.type !== "intermediate") {
-              setValue(styleValue, { isEphemeral: true });
-            }
-          }}
-          onHighlight={(styleValue) => {
-            if (styleValue !== undefined) {
-              setValue(styleValue, { isEphemeral: true });
-            } else {
-              deleteProperty(property, { isEphemeral: true });
-            }
-          }}
-          onChangeComplete={({ value }) => {
-            setValue(value);
-            setIntermediateValue(undefined);
-          }}
-          onAbort={() => {
-            deleteProperty(property, { isEphemeral: true });
-          }}
+          onChange={(styleValue) => setValue(styleValue, { isEphemeral: true })}
+          onChangeComplete={setValue}
+          onAbort={() => deleteProperty(property, { isEphemeral: true })}
         />
       </AdvancedValueTooltip>
     </Flex>

apps/builder/app/builder/features/style-panel/shared/color-picker.tsx

@@ -3,12 +3,12 @@ import { colord, extend, type RgbaColor } from "colord";
 import namesPlugin from "colord/plugins/names";
 import { useDebouncedCallback } from "use-debounce";
 import { RgbaColorPicker } from "react-colorful";
+import { EyedropperIcon } from "@webstudio-is/icons";
 import type {
-  InvalidValue,
-  KeywordValue,
-  RgbValue,
   StyleProperty,
   StyleValue,
+  KeywordValue,
+  RgbValue,
 } from "@webstudio-is/css-engine";
 import {
   Popover,
@@ -22,11 +22,9 @@ import {
 } from "@webstudio-is/design-system";
 import { toValue } from "@webstudio-is/css-engine";
 import { theme } from "@webstudio-is/design-system";
-import type { StyleSource } from "./style-info";
 import { CssValueInput } from "./css-value-input";
 import type { IntermediateStyleValue } from "./css-value-input/css-value-input";
 import { ColorThumb } from "./color-thumb";
-import { EyedropperIcon } from "@webstudio-is/icons";
 
 // To support color names
 extend([namesPlugin]);
@@ -48,27 +46,9 @@ const colorResultToRgbValue = (rgb: RgbaColor): RgbValue => {
   };
 };
 
-const styleValueResolve = (
-  value: RgbValue | KeywordValue,
-  currentColor: RgbValue
-): RgbValue | KeywordValue => {
-  if (
-    value.type === "keyword" &&
-    value.value.toLowerCase() === "currentcolor"
-  ) {
-    return {
-      type: "rgb",
-      r: currentColor.r,
-      g: currentColor.g,
-      b: currentColor.b,
-      alpha: currentColor.alpha,
-    };
-  }
-
-  return value;
-};
-
-const styleValueToRgbaColor = (value: CssColorPickerValueInput): RgbaColor => {
+const styleValueToRgbaColor = (
+  value: StyleValue | IntermediateStyleValue
+): RgbaColor => {
   const color = colord(
     value.type === "intermediate" ? value.value : toValue(value)
   ).toRgb();
@@ -125,18 +105,16 @@ const ColorPickerPopoverContent = ({
   onChange,
   onChangeComplete,
 }: {
-  value: CssColorPickerValueInput;
-  onChange: (value: CssColorPickerValueInput | undefined) => void;
-  onChangeComplete: (event: {
-    value: RgbValue | KeywordValue | InvalidValue;
-  }) => void;
+  value: StyleValue | IntermediateStyleValue;
+  onChange: (value: StyleValue | undefined) => void;
+  onChangeComplete: (value: StyleValue) => void;
 }) => {
   const [hex, setHex] = useState(() =>
     colord(styleValueToRgbaColor(value)).toHex()
   );
   const normalizedHex = normalizeHex(hex);
   const handleCompleteDebounced = useDebouncedCallback(
-    (newValue: RgbValue) => onChangeComplete({ value: newValue }),
+    (newValue: RgbValue) => onChangeComplete(newValue),
     500
   );
   /**
@@ -167,7 +145,7 @@ const ColorPickerPopoverContent = ({
           onChange={(newHex) => {
             setHex(newHex);
             const newValue = colorResultToRgbValue(colord(newHex).toRgb());
-            onChangeComplete({ value: newValue });
+            onChangeComplete(newValue);
           }}
         />
         <InputField
@@ -187,23 +165,12 @@ const ColorPickerPopoverContent = ({
   );
 };
 
-export type CssColorPickerValueInput =
-  | RgbValue
-  | KeywordValue
-  | IntermediateStyleValue
-  | InvalidValue;
-
 type ColorPickerProps = {
-  onChange: (value: CssColorPickerValueInput | undefined) => void;
-  onChangeComplete: (event: {
-    value: RgbValue | KeywordValue | InvalidValue;
-  }) => void;
-  onHighlight: (value: StyleValue | undefined) => void;
+  onChange: (value: StyleValue) => void;
+  onChangeComplete: (value: StyleValue) => void;
   onAbort: () => void;
-  intermediateValue: CssColorPickerValueInput | undefined;
-  value: RgbValue | KeywordValue;
-  currentColor: RgbValue;
-  styleSource: StyleSource;
+  value: StyleValue;
+  currentColor: StyleValue;
   keywords?: Array<KeywordValue>;
   property: StyleProperty;
   disabled?: boolean;
@@ -212,46 +179,21 @@ type ColorPickerProps = {
 export const ColorPicker = ({
   value,
   currentColor,
-  intermediateValue,
-  onChange,
-  onChangeComplete,
-  onHighlight,
-  onAbort,
-  styleSource,
   keywords,
   property,
   disabled,
+  onChange,
+  onChangeComplete,
+  onAbort,
 }: ColorPickerProps) => {
   const [displayColorPicker, setDisplayColorPicker] = useState(false);
   const { enableCanvasPointerEvents, disableCanvasPointerEvents } =
     useDisableCanvasPointerEvents();
 
-  const currentValue =
-    intermediateValue ?? styleValueResolve(value, currentColor);
-
-  const rgbValue = styleValueToRgbaColor(currentValue);
-
-  // Change prefix color in sync with color picker, don't change during input changed
-  let prefixColor = styleValueResolve(
-    currentValue.type === "keyword" || currentValue.type === "rgb"
-      ? currentValue
-      : value,
-    currentColor
-  );
-  // consider inherit on color same as currentColor
-  if (
-    property === "color" &&
-    prefixColor.type === "keyword" &&
-    prefixColor.value === "inherit"
-  ) {
-    prefixColor = {
-      type: "rgb",
-      r: currentColor.r,
-      g: currentColor.g,
-      b: currentColor.b,
-      alpha: currentColor.alpha,
-    };
-  }
+  const [intermediateValue, setIntermediateValue] = useState<
+    StyleValue | IntermediateStyleValue
+  >();
+  const currentValue = intermediateValue ?? value;
 
   const handleOpenChange = (open: boolean) => {
     setDisplayColorPicker(open);
@@ -274,7 +216,10 @@ export const ColorPicker = ({
         aria-label="Open color picker"
         onClick={() => setDisplayColorPicker((shown) => !shown)}
       >
-        <ColorThumb color={rgbValue} css={{ margin: theme.spacing[3] }} />
+        <ColorThumb
+          color={styleValueToRgbaColor(currentColor)}
+          css={{ margin: theme.spacing[3] }}
+        />
       </PopoverTrigger>
       <PopoverContent
         css={{
@@ -285,8 +230,18 @@ export const ColorPicker = ({
       >
         <ColorPickerPopoverContent
           value={currentValue}
-          onChange={onChange}
-          onChangeComplete={onChangeComplete}
+          onChange={(styleValue) => {
+            setIntermediateValue(styleValue);
+            if (styleValue) {
+              onChange(styleValue);
+            } else {
+              onAbort();
+            }
+          }}
+          onChangeComplete={(value) => {
+            setIntermediateValue(undefined);
+            onChangeComplete(value);
+          }}
         />
       </PopoverContent>
     </Popover>
@@ -295,7 +250,7 @@ export const ColorPicker = ({
   return (
     <CssValueInput
       disabled={disabled}
-      styleSource={styleSource}
+      styleSource="default"
       prefix={prefix}
       showSuffix={false}
       property={property}
@@ -303,33 +258,50 @@ export const ColorPicker = ({
       intermediateValue={intermediateValue}
       keywords={keywords}
       onChange={(styleValue) => {
+        if (styleValue === undefined) {
+          setIntermediateValue(styleValue);
+          onAbort();
+          return;
+        }
+        if (styleValue.type === "intermediate") {
+          setIntermediateValue(styleValue);
+          return;
+        }
         if (
-          styleValue?.type === "rgb" ||
-          styleValue?.type === "keyword" ||
-          styleValue?.type === "intermediate" ||
-          styleValue?.type === "invalid" ||
-          styleValue === undefined
+          styleValue.type === "rgb" ||
+          styleValue.type === "keyword" ||
+          styleValue.type === "invalid"
         ) {
+          setIntermediateValue(styleValue);
           onChange(styleValue);
           return;
         }
 
-        onChange({
+        setIntermediateValue({
           type: "intermediate",
           value: toValue(styleValue),
         });
       }}
-      onHighlight={onHighlight}
+      onHighlight={(styleValue) => {
+        if (styleValue) {
+          onChange(styleValue);
+        } else {
+          onAbort();
+        }
+      }}
       onChangeComplete={({ value }) => {
         if (value.type === "rgb" || value.type === "keyword") {
-          onChangeComplete({ value });
+          setIntermediateValue(undefined);
+          onChangeComplete(value);
           return;
         }
         // In case value is parsed to something wrong
-        onChange({
+        const invalidValue: StyleValue = {
           type: "invalid",
           value: toValue(value),
-        });
+        };
+        setIntermediateValue(invalidValue);
+        onChange(invalidValue);
       }}
       onAbort={onAbort}
     />

apps/builder/app/shared/style-object-model.test.tsx

@@ -1395,4 +1395,34 @@ describe("style value source", () => {
       styleSourceId: "boxLocal",
     });
   });
+
+  test("local with inherit keyword", () => {
+    const model = createModel({
+      css: `
+        bodyLocal {
+          color: red;
+        }
+        boxLocal {
+          color: inherit;
+        }
+      `,
+      jsx: (
+        <$.Body ws:id="body" ws:tag="body" class="bodyLocal">
+          <$.Box ws:id="box" ws:tag="div" class="boxLocal"></$.Box>
+        </$.Body>
+      ),
+    });
+    expect(
+      getComputedStyleDecl({
+        model,
+        instanceSelector: ["box", "body"],
+        property: "color",
+      }).source
+    ).toEqual({
+      name: "local",
+      instanceId: "box",
+      breakpointId: "base",
+      styleSourceId: "boxLocal",
+    });
+  });
 });

apps/builder/app/shared/style-object-model.ts

@@ -316,11 +316,9 @@ export const getComputedStyleDecl = ({
       (property === "color" && matchKeyword(cascadedValue, "currentcolor"))
     ) {
       specifiedValue = inheritedValue;
-      source = inheritedSource;
     } else if (matchKeyword(cascadedValue, "unset")) {
       if (inherited) {
         specifiedValue = inheritedValue;
-        source = inheritedSource;
       } else {
         specifiedValue = initialValue;
       }