From dfc7919f7b5dfec89720973bde1e005a62e43dea Mon Sep 17 00:00:00 2001 From: makesomethingshit Date: Thu, 12 Nov 2026 18:11:56 +0900 Subject: [PATCH] :bug: Preserve HSL values while editing lightness (#12629) * :bug: 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 AI-assisted-by: GPT-6 * :bug: 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 AI-assisted-by: GPT-6 --------- Signed-off-by: makesomethingshit --- common/src/app/common/types/color.cljc | 7 +-- .../src/app/main/data/workspace/colors.cljs | 21 +++++--- .../app/main/ui/workspace/colorpicker.cljs | 11 ++-- .../workspace/colorpicker/color_inputs.cljs | 32 ++++-------- .../main/ui/workspace/colorpicker/hsl.cljs | 23 ++++++++ .../main/ui/workspace/colorpicker/hsva.cljs | 26 +++------- .../main/ui/workspace/colorpicker/ramp.cljs | 2 + .../data/workspace_colors_test.cljs | 52 +++++++++++++++++++ 8 files changed, 120 insertions(+), 54 deletions(-) create mode 100644 frontend/src/app/main/ui/workspace/colorpicker/hsl.cljs diff --git a/common/src/app/common/types/color.cljc b/common/src/app/common/types/color.cljc index 96e71a5fcb..0979f1214f 100644 --- 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 diff --git a/frontend/src/app/main/data/workspace/colors.cljs b/frontend/src/app/main/data/workspace/colors.cljs index 9594da29e8..6bd0ca3961 100644 --- 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) diff --git a/frontend/src/app/main/ui/workspace/colorpicker.cljs b/frontend/src/app/main/ui/workspace/colorpicker.cljs index 97c5ecd01d..08d9734da0 100644 --- 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?)))))) diff --git a/frontend/src/app/main/ui/workspace/colorpicker/color_inputs.cljs b/frontend/src/app/main/ui/workspace/colorpicker/color_inputs.cljs index 5d7d72cf71..ab4481f4fd 100644 --- 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" diff --git a/frontend/src/app/main/ui/workspace/colorpicker/hsl.cljs b/frontend/src/app/main/ui/workspace/colorpicker/hsl.cljs new file mode 100644 index 0000000000..31c95f1a9c --- /dev/null +++ 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})) diff --git a/frontend/src/app/main/ui/workspace/colorpicker/hsva.cljs b/frontend/src/app/main/ui/workspace/colorpicker/hsva.cljs index 5194488b5e..446c88f8f8 100644 --- 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)} diff --git a/frontend/src/app/main/ui/workspace/colorpicker/ramp.cljs b/frontend/src/app/main/ui/workspace/colorpicker/ramp.cljs index a8ddf66070..1e9b915141 100644 --- 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) diff --git a/frontend/test/frontend_tests/data/workspace_colors_test.cljs b/frontend/test/frontend_tests/data/workspace_colors_test.cljs index 5f5b25fc1e..5e8b8c6ce6 100644 --- 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