🐛 Preserve HSL values while editing lightness (#12629)

* 🐛 Preserve HSL values while editing lightness Keep HSL editing components through black, white, and grayscale. Use the same draft for numeric inputs, sliders, and gradient previews. Strip the draft before emitting color data and reset it on other edits. Closes #12615 Signed-off-by: makesomethingshit <junsoo1172@gmail.com> AI-assisted-by: GPT-6 * 🐛 Keep HSL drafts consistent across gradient and ramp edits Match RGB components to HEX so gradient echoes preserve HSL drafts. Discard stale HSL values when editing Ramp hue, saturation or value, while preserving them for opacity-only changes. Cover grayscale gradient echoes and saturation restoration. Signed-off-by: makesomethingshit <junsoo1172@gmail.com> AI-assisted-by: GPT-6 --------- Signed-off-by: makesomethingshit <junsoo1172@gmail.com>
2026-11-12 13:00:37 +00:00 · 2026-11-12 18:11:56 +09:00 · 2026-11-12 18:11:56 +09:00 · dfc7919f7b
commit dfc7919f7b
parent b438676d6c
8 changed files with 120 additions and 54 deletions
--- a/common/src/app/common/types/color.cljc
+++ b/common/src/app/common/types/color.cljc
@ -535,9 +535,10 @@
                     (if (= max norm-g)
                       (+ 120 (* 60 (/ (- norm-b norm-r) (- max min))))
                       (+ 240 (* 60 (/ (- norm-r norm-g) (- max min)))))))
-        s      (if (and (> l 0) (<= l 0.5))
-                 (/ (- max min) (* 2 l))
-                 (/ (- max min) (- 2 (* 2 l))))]
+        s      (cond
+                 (= max min) 0
+                 (<= l 0.5) (/ (- max min) (* 2 l))
+                 :else (/ (- max min) (- 2 (* 2 l))))]
    [(mod (+ h 360) 360) s l]))

 (defn hex->hsv
--- a/frontend/src/app/main/data/workspace/colors.cljs
+++ b/frontend/src/app/main/data/workspace/colors.cljs
@ -830,13 +830,19 @@
      (assoc :opacity alpha)
      (d/without-nils)))

+(defn merge-color-components
+  [color changes]
+  (cond-> (merge color changes)
+    (and (:hex changes) (not (:hsl changes)))
+    (dissoc :hsl)))
+
 (defn clear-color-components
  [data]
-  (dissoc data :hex :alpha :r :g :b :h :s :v :image))
+  (dissoc data :hex :alpha :r :g :b :h :s :v :hsl :image))

 (defn clear-image-components
  [data]
-  (dissoc data :hex :alpha :r :g :b :h :s :v :color))
+  (dissoc data :hex :alpha :r :g :b :h :s :v :hsl :color))

 (defn- create-gradient
  [type]
@ -928,7 +934,9 @@
                            new-gradient (-> gradient
                                             (dissoc :stops)
                                             (assoc :shape-id shape-id))]
-                        (if (and (= (:stops state) new-stops) (= (:gradient state) new-gradient))
+                        (if (and (= (mapv #(dissoc % :hsl) (:stops state))
+                                    new-stops)
+                                 (= (:gradient state) new-gradient))
                          state
                          (-> state
                              (assoc :opacity (:opacity data))
@ -1100,7 +1108,7 @@
              (fn [state]
                (let [type  (:type state)
                      state (-> state
-                                (update :current-color merge changes)
+                                (update :current-color merge-color-components changes)
                                (update :current-color materialize-color-components)
                                (update :current-color #(if (not= type :image) (dissoc % :image) %))
                                ;; current color can be a library one
@ -1108,9 +1116,8 @@
                                (update :current-color dissoc :ref-id :ref-file))]

                  (if-let [stop (:editing-stop state)]
-                    (update-in state [:stops stop] (fn [data] (->> changes
-                                                                   (merge data)
-                                                                   (materialize-color-components))))
+                    (update-in state [:stops stop] (fn [data] (-> (merge-color-components data changes)
+                                                                  (materialize-color-components))))

                    (-> state
                        (dissoc :gradient :stops :editing-stop)
--- a/frontend/src/app/main/ui/workspace/colorpicker.cljs
+++ b/frontend/src/app/main/ui/workspace/colorpicker.cljs
@ -39,6 +39,7 @@
   [app.main.ui.workspace.colorpicker.color-tokens :refer [token-section*]]
   [app.main.ui.workspace.colorpicker.gradients :refer [gradients*]]
   [app.main.ui.workspace.colorpicker.harmony :refer [harmony-selector*]]
+   [app.main.ui.workspace.colorpicker.hsl :as hsl]
   [app.main.ui.workspace.colorpicker.hsva :refer [hsva-selector*]]
   [app.main.ui.workspace.colorpicker.libraries :refer [libraries*]]
   [app.main.ui.workspace.colorpicker.ramp :refer [ramp-selector*]]
@ -88,10 +89,10 @@
          ;; saturation 0 → 1. For L: fix current saturation, sweep
          ;; lightness 0 → 0.5 (pure hue) → 1. All computed at the
          ;; current hue.
-          [_ cur-hsl-s cur-hsl-l] (cc/rgb->hsl rgb)
-          hsl-sat-from  [h 0.0 cur-hsl-l]
-          hsl-sat-to    [h 1.0 cur-hsl-l]
-          lightness-mid [h cur-hsl-s 0.5]
+          [hsl-h cur-hsl-s cur-hsl-l] (hsl/components current-color)
+          hsl-sat-from  [hsl-h 0.0 cur-hsl-l]
+          hsl-sat-to    [hsl-h 1.0 cur-hsl-l]
+          lightness-mid [hsl-h cur-hsl-s 0.5]

          format-hsl (fn [[h s l]]
                       (str/fmt "hsl(%s, %s, %s)"
@ -214,7 +215,7 @@
        (mf/use-fn
         (mf/deps current-color drag?)
         (fn [color]
-           (let [color (merge current-color color)
+           (let [color (dc/merge-color-components current-color color)
                 color (dc/materialize-color-components color)]
             (st/emit! (dc/update-colorpicker-color color (not drag?))))))

--- a/frontend/src/app/main/ui/workspace/colorpicker/color_inputs.cljs
+++ b/frontend/src/app/main/ui/workspace/colorpicker/color_inputs.cljs
@ -12,6 +12,7 @@
   [app.common.types.color :as cc]
   [app.main.ui.ds.controls.input :refer [input*]]
   [app.main.ui.ds.foundations.assets.icon :as i]
+   [app.main.ui.workspace.colorpicker.hsl :as hsl]
   [app.util.dom :as dom]
   [app.util.keyboard :as kbd]
   [rumext.v2 :as mf]))
@ -41,11 +42,7 @@
        ;; in sync with the inputs.
        hsb-mode  (or mode :hsb)

-        ;; Compute HSL from current RGB (derived; not stored on the color map)
-        [_hsl-h hsl-s hsl-l]
-        (if (and red green blue)
-          (cc/rgb->hsl [red green blue])
-          [0 0 0])
+        [hsl-h hsl-s hsl-l] (hsl/components color)

        refs {:hex   (mf/use-ref nil)
              :r     (mf/use-ref nil)
@ -104,17 +101,9 @@
                            :h h :s s :v v
                            :r r :g g :b b}))

-              ;; HSL changes: recompute RGB/HSV from the new HSL triple,
-              ;; reusing the current hue when only S or L changes.
-              (#{:hsl-s :hsl-l} property)
-              (let [new-s   (if (= property :hsl-s) val hsl-s)
-                    new-l   (if (= property :hsl-l) val hsl-l)
-                    [r g b] (cc/hsl->rgb [hue new-s new-l])
-                    hex     (cc/rgb->hex [r g b])
-                    [h s v] (cc/hex->hsv hex)]
-                (on-change {:hex hex
-                            :h h :s s :v v
-                            :r r :g g :b b}))
+              (and (= hsb-mode :hsl)
+                   (#{:h :hsl-s :hsl-l} property))
+              (on-change (hsl/update-color color property val))

              :else
              (let [{:keys [h s v]} (merge color (hash-map property val))
@ -159,6 +148,7 @@
     (fn []
       (doseq [ref-key (keys refs)]
         (let [property-val (case ref-key
+                              :h (if (= hsb-mode :hsl) hsl-h hue)
                              :hsl-s hsl-s
                              :hsl-l hsl-l
                              (get color ref-key))
@ -240,11 +230,11 @@
                     :property "Hue"
                     :aria-label "Hue"
                     :max 360
-                     :default-value hue
+                     :default-value hsl-h
                     :on-change (on-change-property :h 360)
                     :on-key-down (on-key-down-property :h 360)}]
         [:> input* {:id "saturation-value"
-                     :ref (:s refs)
+                     :ref (:hsl-s refs)
                     :type "number"
                     :min 0
                     :icon i/character-s
@ -252,9 +242,9 @@
                     :aria-label "Saturation"
                     :max 100
                     :step 1
-                     :default-value saturation
-                     :on-change (on-change-property :s 100)
-                     :on-key-down (on-key-down-property :s 100)}]
+                     :default-value (mth/precision (* hsl-s 100) 2)
+                     :on-change (on-change-property :hsl-s 100)
+                     :on-key-down (on-key-down-property :hsl-s 100)}]
         [:> input* {:id "lightness-value"
                     :ref (:hsl-l refs)
                     :type "number"
--- a/frontend/src/app/main/ui/workspace/colorpicker/hsl.cljs
+++ b/frontend/src/app/main/ui/workspace/colorpicker/hsl.cljs
@ -0,0 +1,23 @@
+;; This Source Code Form is subject to the terms of the Mozilla Public
+;; License, v. 2.0. If a copy of the MPL was not distributed with this
+;; file, You can obtain one at http://mozilla.org/MPL/2.0/.
+;;
+;; Copyright (c) KALEIDOS SUBSIDIARY SL
+
+(ns app.main.ui.workspace.colorpicker.hsl
+  (:require
+   [app.common.types.color :as cc]))
+
+(defn components
+  [{:keys [hsl r g b]}]
+  (or hsl (cc/rgb->hsl [(or r 0) (or g 0) (or b 0)])))
+
+(defn update-color
+  [color property value]
+  (let [index   (case property :h 0 :hsl-s 1 :hsl-l 2)
+        hsl     (assoc (components color) index value)
+        hex     (cc/rgb->hex (cc/hsl->rgb hsl))
+        [r g b] (cc/hex->rgb hex)
+        [h s v] (cc/hex->hsv hex)]
+    ;; RGB cannot retain the chosen hue and saturation at black or white.
+    {:hsl hsl :hex hex :h h :s s :v v :r r :g g :b b}))
--- a/frontend/src/app/main/ui/workspace/colorpicker/hsva.cljs
+++ b/frontend/src/app/main/ui/workspace/colorpicker/hsva.cljs
@ -8,20 +8,15 @@
  (:require-macros [app.main.style :as stl])
  (:require
   [app.common.types.color :as cc]
+   [app.main.ui.workspace.colorpicker.hsl :as hsl]
   [app.main.ui.workspace.colorpicker.slider-selector :refer [slider-selector*]]
   [rumext.v2 :as mf]))

 (mf/defc hsva-selector* [{:keys [color disable-opacity mode on-change on-start-drag on-finish-drag]}]
-  (let [{hue :h saturation :s value :v alpha :alpha
-         r-val :r g-val :g b-val :b} color
+  (let [{hue :h saturation :s value :v alpha :alpha} color
        hsl-mode? (= mode :hsl)

-        ;; Current HSL derived from RGB — used as the starting point
-        ;; for HSL saturation/lightness slider values and for
-        ;; recomputing the color when either is dragged.
-        [_ hsl-s hsl-l] (if (and r-val g-val b-val)
-                          (cc/rgb->hsl [r-val g-val b-val])
-                          [0 0 0])
+        [hsl-h hsl-s hsl-l] (hsl/components color)

        ;; HSB math — current default behavior.
        handle-change-hsv
@ -41,14 +36,7 @@
        handle-change-hsl
        (fn [key]
          (fn [new-value]
-            (let [new-s   (if (= key :hsl-s) new-value hsl-s)
-                  new-l   (if (= key :hsl-l) new-value hsl-l)
-                  [r g b] (cc/hsl->rgb [hue new-s new-l])
-                  hex     (cc/rgb->hex [r g b])
-                  [h s v] (cc/hex->hsv hex)]
-              (on-change {:hex hex
-                          :h h :s s :v v
-                          :r r :g g :b b}))))
+            (on-change (hsl/update-color color key new-value))))

        on-change-opacity (fn [new-alpha] (on-change {:alpha new-alpha}))]
    [:div {:class (stl/css :hsva-selector)}
@ -58,8 +46,10 @@
       {:class (stl/css :hsva-bar)
        :type :hue
        :max-value 360
-        :value hue
-        :on-change (handle-change-hsv :h)
+        :value (if hsl-mode? hsl-h hue)
+        :on-change (if hsl-mode?
+                     (handle-change-hsl :h)
+                     (handle-change-hsv :h))
        :on-start-drag on-start-drag
        :on-finish-drag on-finish-drag}]]
     [:div {:class (stl/css :hsva-row)}
--- a/frontend/src/app/main/ui/workspace/colorpicker/ramp.cljs
+++ b/frontend/src/app/main/ui/workspace/colorpicker/ramp.cljs
@ -97,6 +97,7 @@
         (mf/deps internal-color on-change)
         (fn [saturation value]
           (let [color (-> internal-color
+                           (dissoc :hsl)
                           (assoc :s saturation)
                           (assoc :v value)
                           (enrich-color-map))]
@ -108,6 +109,7 @@
         (mf/deps internal-color on-change)
         (fn [hue]
           (let [color (-> internal-color
+                           (dissoc :hsl)
                           (assoc :h hue)
                           enrich-color-map)]
             (reset! internal-color* color)
--- a/frontend/test/frontend_tests/data/workspace_colors_test.cljs
+++ b/frontend/test/frontend_tests/data/workspace_colors_test.cljs
@ -6,8 +6,10 @@

 (ns frontend-tests.data.workspace-colors-test
  (:require
+   [app.common.types.color :as cc]
   [app.common.uuid :as uuid]
   [app.main.data.workspace.colors :as dwc]
+   [app.main.ui.workspace.colorpicker.hsl :as hsl]
   [cljs.test :as t :include-macros true]
   [frontend-tests.helpers.async :as async]
   [potok.v2.core :as ptk]))
@ -84,6 +86,56 @@
        event (dwc/update-colorpicker-color {} true)]
    (t/is (some? (ptk/watch event valid-state nil)))))

+(t/deftest hsl-boundary-editing
+  (let [original (dwc/split-color-components {:color "#0d5af2" :opacity 1})
+        expected (hsl/components original)]
+    (doseq [lightness [0 1]]
+      (let [boundary (hsl/update-color original :hsl-l lightness)
+            restored (hsl/update-color boundary :hsl-l 0.5)]
+        (t/is (= expected (hsl/components restored)))
+        (t/is (= "#0d5af2" (:hex restored)))
+        (t/is (every? js/Number.isFinite (map restored [:r :g :b])))))
+    (let [gray (hsl/update-color original :hsl-s 0)
+          green (hsl/update-color gray :h 120)
+          saturated (hsl/update-color green :hsl-s 0.5)]
+      (t/is (= [120 0.5 0.5] (:hsl saturated))))))
+
+(t/deftest hsl-draft-stays-in-colorpicker
+  (let [color (dwc/materialize-color-components
+               (hsl/update-color (dwc/split-color-components
+                                  {:color "#0d5af2" :opacity 1}) :hsl-l 0))
+        state {:colorpicker {:type :color :current-color color}}
+        changed (ptk/update (dwc/update-colorpicker-color
+                             {:hex "#ff0000" :r 255 :g 0 :b 0 :h 0 :s 1 :v 255} false)
+                            state)]
+    (t/is (not (contains? (get-in changed [:colorpicker :current-color]) :hsl)))
+    (t/is (not (contains? (dwc/get-color-from-colorpicker-state (:colorpicker state)) :hsl)))
+    (let [gradient {:type :linear-gradient :current-color color
+                    :editing-stop 0 :stops [(assoc color :offset 0)]
+                    :gradient {} :opacity 1}]
+      (t/is (not (contains? (get-in (dwc/get-color-from-colorpicker-state gradient)
+                                    [:gradient :stops 0]) :hsl))))))
+
+(t/deftest hsl-white-is-finite
+  (t/is (= [0 0 1] (cc/rgb->hsl [255 255 255]))))
+
+(t/deftest hsl-gradient-echo-preserves-draft
+  (doseq [[property value] [[:hsl-l 0] [:hsl-l 1] [:hsl-s 0]]]
+    (let [original (dwc/split-color-components {:color "#0d5af2" :opacity 1})
+          color (assoc (dwc/materialize-color-components
+                        (merge original (hsl/update-color original property value))) :offset 0)
+          state {:colorpicker {:type :linear-gradient :current-color color
+                               :editing-stop 0 :stops [color]
+                               :gradient {:type :linear :shape-id nil} :opacity 1}}
+          data (dwc/get-color-from-colorpicker-state (:colorpicker state))
+          echoed (ptk/update (dwc/update-colorpicker data) state)
+          changed (ptk/update (dwc/update-colorpicker-color {:hex "#ff0000"} false) state)]
+      (t/is (= (:hsl color) (get-in echoed [:colorpicker :current-color :hsl])))
+      (when (= property :hsl-s)
+        (t/is (= "#406bbf" (:hex (hsl/update-color
+                                  (get-in echoed [:colorpicker :current-color]) :hsl-s 0.5)))))
+      (t/is (not (contains? (get-in changed [:colorpicker :stops 0]) :hsl))))))
+
 (t/deftest ^:async color-change-updates-the-shapes-of-a-property-in-one-event
  ;; Changing a color in a selection of many shapes must not commit once per
  ;; shape: each commit makes the page objects a new map, and everything that