🐛 Fix problem when assigning color from palette or assets

CHANGES.md

@@ -39,6 +39,7 @@
 - Fix manipulate duplicated project (delete, duplicate, rename, pin/unpin...) [Taiga #5027](https://tree.taiga.io/project/penpot/issue/5027)
 - Fix deleted files appear in search results [Taiga #5002](https://tree.taiga.io/project/penpot/issue/5002)
 - Fix problem with selected colors and texts [Taiga #5051](https://tree.taiga.io/project/penpot/issue/5051)
+- Fix problem when assigning color from palette or assets [Taiga #5050](https://tree.taiga.io/project/penpot/issue/5050)
 
 ### :heart: Community contributions by (Thank you!)
 - To @ondrejkonec: for contributing to the code with:

common/src/app/common/pages/helpers.cljc

@@ -37,8 +37,10 @@
    (= type :frame)))
 
 (defn group-shape?
-  [{:keys [type]}]
+  ([objects id]
-  (= type :group))
+   (group-shape? (get objects id)))
+  ([{:keys [type]}]
+   (= type :group)))
 
 (defn mask-shape?
   [{:keys [type masked-group?]}]

frontend/src/app/main/data/workspace/colors.cljs

@@ -8,6 +8,7 @@
   (:require
    [app.common.colors :as colors]
    [app.common.data :as d]
+   [app.common.data.macros :as dm]
    [app.common.pages.helpers :as cph]
    [app.main.broadcast :as mbc]
    [app.main.data.modal :as md]
@@ -81,6 +82,8 @@
         text-ids  (filter is-text? ids)
         shape-ids (remove is-text? ids)
 
+        undo-id (js/Symbol)
+
         attrs
         (cond-> {}
           (contains? color :color)
@@ -104,8 +107,10 @@
         transform-attrs #(transform % attrs)]
 
     (rx/concat
+     (rx/of (dwu/start-undo-transaction undo-id))
      (rx/from (map #(dwt/update-text-with-function % transform-attrs) text-ids))
-     (rx/of (dch/update-shapes shape-ids transform-attrs)))))
+     (rx/of (dch/update-shapes shape-ids transform-attrs))
+     (rx/of (dwu/commit-undo-transaction undo-id)))))
 
 (defn swap-attrs [shape attr index new-index]
   (let [first (get-in shape [attr index])
@@ -366,23 +371,33 @@
          (rx/of (dwu/commit-undo-transaction undo-id)))))))
 
 (defn apply-color-from-palette
-  [color is-alt?]
+  [color stroke?]
   (ptk/reify ::apply-color-from-palette
     ptk/WatchEvent
     (watch [_ state _]
       (let [objects  (wsh/lookup-page-objects state)
             selected (->> (wsh/lookup-selected state)
                           (cph/clean-loops objects))
-            selected-obj (keep (d/getf objects) selected)
+
-            select-shapes-for-color (fn [shape objects]
+            ids
-                                      (let [shapes (case (:type shape)
+            (loop [pending (seq selected)
-                                                     :group (cph/get-children objects (:id shape))
+                   result []]
-                                                     [shape])]
+              (if (empty? pending)
-                                        (->> shapes
+                result
-                                             (remove cph/group-shape?)
+                (let [cur (first pending)
-                                             (map :id))))
+                      ;; We treat frames with no fill the same as groups
-            ids (mapcat #(select-shapes-for-color % objects) selected-obj)]
+                      group? (or (cph/group-shape? objects cur)
-        (if is-alt?
+                                 (and (cph/frame-shape? objects cur)
+                                      (empty? (dm/get-in objects [cur :fills]))))
+
+                      pending
+                      (if group?
+                        (concat pending (dm/get-in objects [cur :shapes]))
+                        pending)
+
+                      result (cond-> result (not group?) (conj cur))]
+                  (recur (rest pending) result))))]
+        (if stroke?
           (rx/of (change-stroke ids (merge uc/empty-color color) 0))
           (rx/of (change-fill ids (merge uc/empty-color color) 0)))))))
 

frontend/src/app/main/ui/workspace/sidebar/assets.cljs

@@ -1150,25 +1150,9 @@
                        (:color color) (:color color)
                        :else (:value color))
 
-        ;; TODO: looks like the first argument is not necessary
-        ;; TODO: this code should be out of this UI component
         apply-color
-        (fn [_ event]
+        (fn [event]
-          (let [objects  (wsh/lookup-page-objects @st/state)
+          (st/emit! (dc/apply-color-from-palette (merge uc/empty-color color) (kbd/alt? event))))
-                selected (->> (wsh/lookup-selected @st/state)
-                              (cph/clean-loops objects))
-                selected-obj (keep (d/getf objects) selected)
-                select-shapes-for-color (fn [shape objects]
-                                          (let [shapes (case (:type shape)
-                                                         :group (cph/get-children objects (:id shape))
-                                                         [shape])]
-                                            (->> shapes
-                                                 (remove cph/group-shape?)
-                                                 (map :id))))
-                ids (mapcat #(select-shapes-for-color % objects) selected-obj)]
-            (if (kbd/alt? event)
-              (st/emit! (dc/change-stroke ids (merge uc/empty-color color) 0))
-              (st/emit! (dc/change-fill ids (merge uc/empty-color color) 0)))))
 
         rename-color
         (fn [name]
@@ -1277,8 +1261,7 @@
                                         :selected (contains? selected-colors (:id color)))
                            :on-context-menu on-context-menu
                            :on-click (when-not (:editing @state)
-                                       #(on-asset-click % (:id color)
+                                       #(on-asset-click % (:id color) apply-color))
-                                                        (partial apply-color (:id color))))
                            :ref item-ref
                            :draggable (and (not workspace-read-only?) (not (:editing @state)))
                            :on-drag-start on-color-drag-start