From 92d14e38b5083b0fedb35c8ee5ccc7ea85940fb7 Mon Sep 17 00:00:00 2001 From: Alonso Torres Date: Mon, 27 Jan 2025 12:36:00 +0100 Subject: [PATCH] :bug: Fix problem with multiple color changes (#5680) * :bug: Fix problem with multiple color changes * :bug: Fixes after review --- CHANGES.md | 2 +- .../options/menus/color_selection.cljs | 88 ++++++++++--------- .../sidebar/options/rows/color_row.cljs | 13 +-- .../sidebar/options/rows/color_row.scss | 4 + 4 files changed, 58 insertions(+), 49 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index 5a7c35cc7..5c2263f24 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -21,7 +21,7 @@ - Fix problem with alt key measures being stuck [Taiga #9348](https://tree.taiga.io/project/penpot/issue/9348) - Fix error when reseting stroke cap - Fix problem with strokes not refreshing in Safari [Taiga #9040](https://tree.taiga.io/project/penpot/issue/9040) - +- Fix problem with multiple color changes [Taiga #9631](https://tree.taiga.io/project/penpot/issue/9631) ## 2.4.3 (Unreleased) diff --git a/frontend/src/app/main/ui/workspace/sidebar/options/menus/color_selection.cljs b/frontend/src/app/main/ui/workspace/sidebar/options/menus/color_selection.cljs index a24c3053d..43a023e3a 100644 --- a/frontend/src/app/main/ui/workspace/sidebar/options/menus/color_selection.cljs +++ b/frontend/src/app/main/ui/workspace/sidebar/options/menus/color_selection.cljs @@ -10,6 +10,7 @@ [app.common.data :as d] [app.common.data.macros :as dm] [app.common.types.color :as ctc] + [app.common.uuid :as uuid] [app.main.data.workspace.colors :as dc] [app.main.data.workspace.selection :as dws] [app.main.store :as st] @@ -40,8 +41,9 @@ {::mf/wrap [#(mf/memo' % (mf/check-props ["shapes"]))] ::mf/wrap-props false} [{:keys [shapes file-id shared-libs]}] - (let [{:keys [groups library-colors colors]} (mf/with-memo [shapes file-id shared-libs] - (prepare-colors shapes file-id shared-libs)) + (let [{:keys [groups library-colors colors]} + (mf/with-memo [shapes file-id shared-libs] + (prepare-colors shapes file-id shared-libs)) state* (mf/use-state true) open? (deref state*) @@ -56,8 +58,15 @@ groups-ref (h/use-ref-value groups) prev-colors-ref (mf/use-ref nil) - ;; grouped-colors* (mf/use-var nil) - ;; prev-colors* (mf/use-var []) + initial-color-keys + (mf/use-memo + #(->> (concat colors library-colors) + (reduce + (fn [result color] + (assoc result color (dm/str (uuid/next)))) + {}))) + + color-keys* (mf/use-var initial-color-keys) on-change (mf/use-fn @@ -86,6 +95,7 @@ (mf/set-ref-val! prev-colors-ref (conj prev-colors color)))) + (swap! color-keys* assoc new-color (get @color-keys* old-color)) (st/emit! (dc/change-color-in-selected cops new-color old-color))))) on-open @@ -121,49 +131,41 @@ (when open? [:div {:class (stl/css :element-content)} [:div {:class (stl/css :selected-color-group)} - (for [[index color] (d/enumerate (take 3 library-colors))] - [:& color-row {:key (dm/str "library-color-" index) - :color color - :index index - :on-detach on-detach - :select-only select-only - :on-change #(on-change %1 color %2) - :on-open on-open - :on-close on-close}]) + ;; The hidden color is to solve a problem with the color picker. When a color is changed + ;; and is no longer a library color it disapears from the list of library colors. Because + ;; we need to keep the color picker open we need to maintain that color. The easier way + ;; is to render the color elements so even if the library color is no longer we have still + ;; the component to change it from the color picker. + (let [lib-colors (cond->> library-colors (not @expand-lib-color) (take 3)) + lib-colors (concat lib-colors colors)] + (for [[index color] (d/enumerate lib-colors)] + [:& color-row + {:key (get @color-keys* color) + :color color + :index index + :hidden (not (:id color)) + :on-detach on-detach + :select-only select-only + :on-change #(on-change %1 color %2) + :on-open on-open + :on-close on-close}])) (when (and (false? @expand-lib-color) (< 3 (count library-colors))) [:button {:class (stl/css :more-colors-btn) :on-click #(reset! expand-lib-color true)} - (tr "workspace.options.more-lib-colors")]) - (when @expand-lib-color - (for [[index color] (d/enumerate (drop 3 library-colors))] - [:& color-row {:key (dm/str "library-color-" index) - :color color - :index index - :on-detach on-detach - :select-only select-only - :on-change #(on-change %1 color %2) - :on-open on-open - :on-close on-close}]))] + (tr "workspace.options.more-lib-colors")])] + [:div {:class (stl/css :selected-color-group)} - (for [[index color] (d/enumerate (take 3 colors))] - [:& color-row {:key (dm/str "color-" index) - :color color - :index index - :select-only select-only - :on-change #(on-change %1 color %2) - :on-open on-open - :on-close on-close}]) + (for [[index color] (d/enumerate (cond->> colors (not @expand-color) (take 3)))] + [:& color-row + {:key (get @color-keys* color) + :color color + :index index + :select-only select-only + :on-change #(on-change %1 color %2) + :on-open on-open + :on-close on-close}]) + (when (and (false? @expand-color) (< 3 (count colors))) [:button {:class (stl/css :more-colors-btn) :on-click #(reset! expand-color true)} - (tr "workspace.options.more-colors")]) - - (when @expand-color - (for [[index color] (d/enumerate (drop 3 colors))] - [:& color-row {:key (dm/str "color-" (:color color)) - :color color - :index index - :select-only select-only - :on-change #(on-change %1 color %2) - :on-open on-open - :on-close on-close}]))]])])) + (tr "workspace.options.more-colors")])]])])) diff --git a/frontend/src/app/main/ui/workspace/sidebar/options/rows/color_row.cljs b/frontend/src/app/main/ui/workspace/sidebar/options/rows/color_row.cljs index a6beaa797..e0e98aca3 100644 --- a/frontend/src/app/main/ui/workspace/sidebar/options/rows/color_row.cljs +++ b/frontend/src/app/main/ui/workspace/sidebar/options/rows/color_row.cljs @@ -45,18 +45,20 @@ (if (= v :multiple) nil v)) (mf/defc color-row - [{:keys [index color disable-gradient disable-opacity disable-image disable-picker on-change - on-reorder on-detach on-open on-close on-remove + [{:keys [index color disable-gradient disable-opacity disable-image disable-picker hidden + on-change on-reorder on-detach on-open on-close on-remove disable-drag on-focus on-blur select-only select-on-focus]}] (let [shared-libs (mf/deref refs/libraries) hover-detach (mf/use-state false) on-change (h/use-ref-callback on-change) - src-colors (dm/get-in shared-libs [(:ref-file color) :data :colors]) - color-name (dm/get-in src-colors [(:ref-id color) :name]) + file-id (or (:ref-file color) (:file-id color)) + color-id (or (:ref-id color) (:id color)) + src-colors (dm/get-in shared-libs [file-id :data :colors]) + color-name (dm/get-in src-colors [color-id :name]) multiple-colors? (uc/multiple? color) - library-color? (and (:ref-id color) color-name (not multiple-colors?)) + library-color? (and (or (:id color) (:ref-id color)) color-name (not multiple-colors?)) gradient-color? (and (not multiple-colors?) (:gradient color) (dm/get-in color [:gradient :type])) @@ -187,6 +189,7 @@ [:div {:class (stl/css-case :color-data true + :hidden hidden :dnd-over-top (= (:over dprops) :top) :dnd-over-bot (= (:over dprops) :bot))} diff --git a/frontend/src/app/main/ui/workspace/sidebar/options/rows/color_row.scss b/frontend/src/app/main/ui/workspace/sidebar/options/rows/color_row.scss index 2b515875a..ff2e897e5 100644 --- a/frontend/src/app/main/ui/workspace/sidebar/options/rows/color_row.scss +++ b/frontend/src/app/main/ui/workspace/sidebar/options/rows/color_row.scss @@ -26,6 +26,10 @@ } } +.hidden { + display: none; +} + .color-info { --detach-icon-foreground-color: none;