✨ Don't override user provided color format (#6231)

frontend/playwright/ui/specs/tokens.spec.js

@@ -371,6 +371,59 @@ test.describe("Tokens: Tokens Tab", () => {
     await expect(tokensTabPanel.getByLabel("color.dark.primary")).toBeEnabled();
   });
 
+  test("User changes color token color while keeping custom color space", async ({
+    page,
+  }) => {
+    const { workspacePage, tokensUpdateCreateModal, tokenThemesSetsSidebar } =
+      await setupEmptyTokensFile(page);
+
+    const tokensTabPanel = page.getByRole("tabpanel", { name: "tokens" });
+    await tokensTabPanel.getByTitle("Add token: Color").click();
+
+    await expect(tokensUpdateCreateModal).toBeVisible();
+    const nameField = tokensUpdateCreateModal.getByLabel("Name");
+    const valueField = tokensUpdateCreateModal.getByLabel("Value");
+
+    await valueField.click();
+    await valueField.fill("hsv(1,1,1)");
+    await expect(
+      tokensUpdateCreateModal.getByText("Resolved value: #ff0400"),
+    ).toBeVisible();
+
+    const colorBullet = tokensUpdateCreateModal.getByTestId(
+      "token-form-color-bullet",
+    );
+    await colorBullet.click();
+
+    const valueSaturationSelector = tokensUpdateCreateModal.getByTestId(
+      "value-saturation-selector",
+    );
+    await expect(valueSaturationSelector).toBeVisible();
+
+    // Check if color space doesnt get overwritten when changing color via the picker
+    // Not testing for exact value to avoid flakiness of px click
+    await valueSaturationSelector.click({ position: { x: 100, y: 100 } });
+    await expect(valueField).not.toHaveValue("hsv(1,1,1)");
+    await expect(valueField).toHaveValue(/^hsv.*$/);
+
+    // Clearing the input field should pick hex
+    await valueField.fill("");
+    await expect(
+      tokensUpdateCreateModal.getByText("Resolved value: -"),
+    ).toBeVisible();
+    await valueSaturationSelector.click({ position: { x: 50, y: 50 } });
+    await expect(valueField).toHaveValue(/^#[A-Fa-f\d]+$/);
+
+    // Changing opacity for hex values converts to rgba
+    const sliderOpacity = tokensUpdateCreateModal.getByTestId("slider-opacity");
+    await sliderOpacity.click({ position: { x: 50, y: 0 } });
+    await expect(valueField).toHaveValue(/^rgba(.*)$/);
+
+    // Changing color now will stay in rgba
+    await valueSaturationSelector.click({ position: { x: 0, y: 0 } });
+    await expect(valueField).toHaveValue(/^rgba(.*)$/);
+  });
+
   test("User duplicate color token", async ({ page }) => {
     const { tokensSidebar, tokenContextMenuForToken } =
       await setupTokensFile(page);
@@ -511,7 +564,9 @@ test.describe("Tokens: Sets Tab", () => {
     // Creates nesting by renaming set with double click
     await tokenThemesSetsSidebar
       .getByRole("button", { name: "light-renamed" })
-      .dblclick();
+      .click({ button: "right" });
+    await expect(tokenContextMenuForSet).toBeVisible();
+    await tokenContextMenuForSet.getByText("Rename").click();
     await changeSetInput(tokenThemesSetsSidebar, "nested/light");
 
     await assertSetsList(tokenThemesSetsSidebar, [

frontend/src/app/main/ui/workspace/tokens/components/controls/input_token_color_bullet.cljs

@@ -20,7 +20,8 @@
   {::mf/props :obj
    ::mf/schema schema::input-token-color-bullet}
   [{:keys [color on-click]}]
-  [:div {:class (stl/css :input-token-color-bullet)
+  [:div {:data-testid "token-form-color-bullet"
+         :class (stl/css :input-token-color-bullet)
          :on-click on-click}
    (if-let [color' (wtt/color-bullet-color color)]
      [:> color-bullet {:color color' :mini true}]

frontend/src/app/main/ui/workspace/tokens/form.cljs

@@ -198,6 +198,13 @@
              (when-not (and dragging? hex)
                (reset! internal-color* selector-color)
                (on-change hex alpha)))))]
+    (mf/use-effect
+     (mf/deps color)
+     (fn []
+       ;; Update internal color when user changes input value
+       (when-let [color (tinycolor/valid-color color)]
+         (when-not (= (tinycolor/->hex-string color) (:hex internal-color))
+           (reset! internal-color* (hex->value color))))))
 
     (colorpicker/use-color-picker-css-variables! wrapper-node-ref internal-color)
     [:div {:ref wrapper-node-ref}
@@ -315,7 +322,8 @@
                            (valid-name? @token-name-ref))
 
         ;; Value
-        color (mf/use-state (when color? (:value token)))
+        color* (mf/use-state (when color? (:value token)))
+        color (deref color*)
         color-ramp-open* (mf/use-state false)
         color-ramp-open? (deref color-ramp-open*)
         value-input-ref (mf/use-ref nil)
@@ -338,7 +346,7 @@
 
                      :else
                      (:resolved-value token-or-err))]
-             (when color? (reset! color (if error? nil v)))
+             (when color? (reset! color* (if error? nil v)))
              (reset! token-resolve-result* v))))
 
         on-update-value-debounced (use-debonced-resolve-callback token-name-ref token active-theme-tokens set-resolve-value)
@@ -355,13 +363,28 @@
                              (reset! value-ref value')
                              (on-update-value-debounced value'))))
         on-update-color (mf/use-fn
-                         (mf/deps on-update-value-debounced)
+                         (mf/deps color on-update-value-debounced)
                          (fn [hex-value alpha]
-                           (let [color-value (if (= 1 alpha)
+                           (let [;; StyleDictionary will always convert to hex/rgba, so we take the format from the value input field
-                                               hex-value
+                                 prev-input-color (some-> (dom/get-value (mf/ref-val value-input-ref))
-                                               (-> (tinycolor/valid-color hex-value)
+                                                          (tinycolor/valid-color))
-                                                   (tinycolor/set-alpha alpha)
+                                 ;; If the input is a reference we will take the format from the computed value
-                                                   (tinycolor/->rgba-string)))]
+                                 prev-computed-color (when-not prev-input-color
+                                                       (some-> color (tinycolor/valid-color)))
+                                 prev-format (some-> (or prev-input-color prev-computed-color)
+                                                     (tinycolor/color-format))
+                                 to-rgba? (and
+                                           (< alpha 1)
+                                           (or (= prev-format "hex") (not prev-format)))
+                                 to-hex? (and (not prev-format) (= alpha 1))
+                                 format (cond
+                                          to-rgba? "rgba"
+                                          to-hex? "hex"
+                                          prev-format prev-format
+                                          :else "hex")
+                                 color-value (-> (tinycolor/valid-color hex-value)
+                                                 (tinycolor/set-alpha (or alpha 1))
+                                                 (tinycolor/->string format))]
                              (reset! value-ref color-value)
                              (dom/set-value! (mf/ref-val value-input-ref) color-value)
                              (on-update-value-debounced color-value))))
@@ -538,10 +561,10 @@
          :on-blur on-update-value}
         (when color?
           [:> input-token-color-bullet*
-           {:color @color :on-click on-display-colorpicker'}])]
+           {:color color
+            :on-click on-display-colorpicker'}])]
        (when color-ramp-open?
-         [:> ramp* {:color (some-> (or token-resolve-result (:value token))
+         [:> ramp* {:color (some-> color (tinycolor/valid-color))
-                                   (tinycolor/valid-color))
                     :on-change on-update-color}])
        [:& token-value-or-errors {:result-or-errors token-resolve-result}]]
 

frontend/src/app/main/ui/workspace/tokens/tinycolor.cljs

@@ -32,17 +32,24 @@
     (let [tc (tinycolor color-str)]
       (str/starts-with? (.getFormat tc) "hex"))))
 
-(defn ->string [^js tc]
+(defn ->string
-  (.toString tc))
+  "Stringify `tc` to `format`, uses `hex` as per default."
+  [^js tc format]
+  (let [format' (case format
+                  ;; Tinycolor `.toString` doesnt support the `a` suffix it gives you via `.getFormat`
+                  "rgba" "rgb"
+                  "hsva" "hsv"
+                  ;; Keep these formats
+                  "rgb"  "rgb"
+                  "hsv"  "hsv"
+                  ;; Fall back to hex as the default
+                  "hex")]
+    (.toString tc format')))
 
 (defn ->hex-string [^js tc]
   (assert (tinycolor? tc))
   (.toHexString tc))
 
-(defn ->rgba-string [^js tc]
-  (assert (tinycolor? tc))
-  (.toRgbString tc))
-
 (defn color-format [^js tc]
   (assert (tinycolor? tc))
   (.getFormat tc))