From 008134fde8dd753ea79c82189a9ed304bd08549b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9s=20Moya?= Date: Fri, 16 Jul 2021 13:54:55 +0200 Subject: [PATCH 1/3] :bug: Fix repeated names when duplicating object trees. --- CHANGES.md | 1 + .../app/main/data/workspace/selection.cljs | 54 +++++++++---------- 2 files changed, 28 insertions(+), 27 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index 7f5c39035..bf2839144 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -7,6 +7,7 @@ ### :bug: Bugs fixed - Add scroll bar to Teams menu [Taiga #1894](https://tree.taiga.io/project/penpot/issue/1894). +- Fix repeated names when duplicating artboards or groups [Taiga #1892](https://tree.taiga.io/project/penpot/issue/1892). ### :arrow_up: Deps updates ### :boom: Breaking changes diff --git a/frontend/src/app/main/data/workspace/selection.cljs b/frontend/src/app/main/data/workspace/selection.cljs index 386e8f3b9..c04bfbedf 100644 --- a/frontend/src/app/main/data/workspace/selection.cljs +++ b/frontend/src/app/main/data/workspace/selection.cljs @@ -257,8 +257,6 @@ (declare prepare-duplicate-frame-change) (declare prepare-duplicate-shape-change) -(def ^:private change->name #(get-in % [:obj :name])) - (defn update-indices "Fixes the indices for a set of changes after a duplication. We need to fix the indices to take into the account the movement of indices. @@ -290,19 +288,19 @@ "Prepare objects to paste: generate new id, give them unique names, move to the position of mouse pointer, and find in what frame they fit." - [objects page-id names ids delta] - (loop [names names - ids (seq ids) - chgs []] - (if ids - (let [id (first ids) - result (prepare-duplicate-change objects page-id names id delta) - result (if (vector? result) result [result])] - (recur - (into names (map change->name) result) - (next ids) - (into chgs result))) - chgs))) + [objects page-id unames ids delta] + (let [unames (volatile! unames) + update-unames! (fn [new-name] (vswap! unames conj new-name))] + (loop [ids (seq ids) + chgs []] + (if ids + (let [id (first ids) + result (prepare-duplicate-change objects page-id unames update-unames! id delta) + result (if (vector? result) result [result])] + (recur + (next ids) + (into chgs result))) + chgs)))) (defn duplicate-changes-update-indices "Parses the change set when duplicating to set-up the appropiate indices" @@ -317,32 +315,32 @@ (-> changes (update-indices index-map)))) (defn- prepare-duplicate-change - [objects page-id names id delta] + [objects page-id unames update-unames! id delta] (let [obj (get objects id)] (if (= :frame (:type obj)) - (prepare-duplicate-frame-change objects page-id names obj delta) - (prepare-duplicate-shape-change objects page-id names obj delta (:frame-id obj) (:parent-id obj))))) + (prepare-duplicate-frame-change objects page-id unames update-unames! obj delta) + (prepare-duplicate-shape-change objects page-id unames update-unames! obj delta (:frame-id obj) (:parent-id obj))))) (defn- prepare-duplicate-shape-change - [objects page-id names obj delta frame-id parent-id] + [objects page-id unames update-unames! obj delta frame-id parent-id] (when (some? obj) (let [id (uuid/next) - name (dwc/generate-unique-name names (:name obj)) + name (dwc/generate-unique-name @unames (:name obj)) + _ (update-unames! name) + renamed-obj (assoc obj :id id :name name) moved-obj (geom/move renamed-obj delta) parent-id (or parent-id frame-id) children-changes - (loop [names names - result [] + (loop [result [] cid (first (:shapes obj)) cids (rest (:shapes obj))] (if (nil? cid) result (let [obj (get objects cid) - changes (prepare-duplicate-shape-change objects page-id names obj delta frame-id id)] + changes (prepare-duplicate-shape-change objects page-id unames update-unames! obj delta frame-id id)] (recur - (into names (map change->name changes)) (into result changes) (first cids) (rest cids))))) @@ -361,11 +359,13 @@ children-changes)))) (defn- prepare-duplicate-frame-change - [objects page-id names obj delta] + [objects page-id unames update-unames! obj delta] (let [frame-id (uuid/next) - frame-name (dwc/generate-unique-name names (:name obj)) + frame-name (dwc/generate-unique-name @unames (:name obj)) + _ (update-unames! frame-name) + sch (->> (map #(get objects %) (:shapes obj)) - (mapcat #(prepare-duplicate-shape-change objects page-id names % delta frame-id frame-id))) + (mapcat #(prepare-duplicate-shape-change objects page-id unames update-unames! % delta frame-id frame-id))) frame (-> obj (assoc :id frame-id) From 6ced56301cd9a0b03aec82dff849541ab26644a1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9s=20Moya?= Date: Fri, 16 Jul 2021 15:11:29 +0200 Subject: [PATCH 2/3] :recycle: Optimice a bit of performance --- frontend/src/app/main/data/workspace/libraries.cljs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/frontend/src/app/main/data/workspace/libraries.cljs b/frontend/src/app/main/data/workspace/libraries.cljs index b267fb388..20d97a210 100644 --- a/frontend/src/app/main/data/workspace/libraries.cljs +++ b/frontend/src/app/main/data/workspace/libraries.cljs @@ -382,7 +382,7 @@ page-id (:current-page-id state) objects (wsh/lookup-page-objects state page-id) - unames (atom (dwc/retrieve-used-names objects)) + unames (volatile! (dwc/retrieve-used-names objects)) frame-id (cp/frame-id-by-position objects (gpt/add orig-pos delta)) @@ -391,7 +391,7 @@ (let [new-name (dwc/generate-unique-name @unames (:name new-shape))] (when (nil? (:parent-id original-shape)) - (swap! unames conj new-name)) + (vswap! unames conj new-name)) (cond-> new-shape true From fa99dea8fe21cfa33233fd602c68472deb46f623 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9s=20Moya?= Date: Fri, 16 Jul 2021 15:25:22 +0200 Subject: [PATCH 3/3] :books: Add some comments about possible code enhancements --- frontend/src/app/main/data/workspace.cljs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/frontend/src/app/main/data/workspace.cljs b/frontend/src/app/main/data/workspace.cljs index 7c0a57c7d..953479dea 100644 --- a/frontend/src/app/main/data/workspace.cljs +++ b/frontend/src/app/main/data/workspace.cljs @@ -1474,8 +1474,8 @@ (= :frame (get-in objects [(first selected) :type]))))) (defn- paste-shape - [{:keys [selected objects images] :as data} in-viewport?] - (letfn [;; Given a file-id and img (part generated by the + [{:keys [selected objects images] :as data} in-viewport?] ;; TODO: perhaps rename 'objects' to 'shapes', because it contains only + (letfn [;; Given a file-id and img (part generated by the ;; the shapes to paste, not the whole page tree of shapes ;; copy-selected event), uploads the new media. (upload-media [file-id imgpart] (->> (http/send! {:uri (:file-data imgpart) @@ -1583,7 +1583,7 @@ page-id (:current-page-id state) unames (-> (wsh/lookup-page-objects state page-id) - (dwc/retrieve-used-names)) + (dwc/retrieve-used-names)) ;; TODO: move this calculation inside prepare-duplcate-changes? rchanges (->> (dws/prepare-duplicate-changes objects page-id unames selected delta) (mapv (partial process-rchange media-idx))