From 785b61be2f28fd98f4bb30b98993db5d4f2377ab Mon Sep 17 00:00:00 2001 From: Pablo Alba Date: Mon, 7 Apr 2025 17:29:03 +0200 Subject: [PATCH] :sparkles: Fix corner cases of duplicate/copy-paste/cut-paste variants --- common/src/app/common/files/changes.cljc | 7 +- .../src/app/common/files/changes_builder.cljc | 5 +- common/src/app/common/logic/libraries.cljc | 43 +++++-- common/src/app/common/logic/shapes.cljc | 79 +----------- .../app/common/logic/variant_properties.cljc | 117 +++++++++++++++++- common/src/app/common/types/component.cljc | 2 - .../src/app/common/types/components_list.cljc | 1 - common/src/app/common/types/container.cljc | 23 +++- common/src/app/common/types/file.cljc | 29 +++-- common/src/app/common/types/variant.cljc | 20 +-- frontend/src/app/main/data/workspace.cljs | 7 +- .../src/app/main/data/workspace/variants.cljs | 3 +- .../src/app/main/ui/ds/controls/combobox.cljs | 6 + .../app/main/ui/workspace/context_menu.cljs | 2 +- .../sidebar/options/shapes/frame.cljs | 3 +- 15 files changed, 217 insertions(+), 130 deletions(-) diff --git a/common/src/app/common/files/changes.cljc b/common/src/app/common/files/changes.cljc index 71633fe7d..74c8e9fbb 100644 --- a/common/src/app/common/files/changes.cljc +++ b/common/src/app/common/files/changes.cljc @@ -354,8 +354,7 @@ [:map {:title "RestoreComponentChange"} [:type [:= :restore-component]] [:id ::sm/uuid] - [:page-id ::sm/uuid] - [:parent-id {:optional true} [:maybe ::sm/uuid]]]] + [:page-id ::sm/uuid]]] [:purge-component [:map {:title "PurgeComponentChange"} @@ -966,8 +965,8 @@ (ctf/delete-component data id skip-undelete? delta)) (defmethod process-change :restore-component - [data {:keys [id page-id parent-id]}] - (ctf/restore-component data id page-id parent-id)) + [data {:keys [id page-id]}] + (ctf/restore-component data id page-id)) (defmethod process-change :purge-component [data {:keys [id]}] diff --git a/common/src/app/common/files/changes_builder.cljc b/common/src/app/common/files/changes_builder.cljc index 9c2327413..ffdc5a61a 100644 --- a/common/src/app/common/files/changes_builder.cljc +++ b/common/src/app/common/files/changes_builder.cljc @@ -1039,13 +1039,12 @@ :page-id page-id}))) (defn restore-component - [changes id page-id delta parent-id] + [changes id page-id delta] (assert-library! changes) (-> changes (update :redo-changes conj {:type :restore-component :id id - :page-id page-id - :parent-id parent-id}) + :page-id page-id}) (update :undo-changes conj {:type :del-component :id id :delta delta}))) diff --git a/common/src/app/common/logic/libraries.cljc b/common/src/app/common/logic/libraries.cljc index f5fa371d1..f5bf33de5 100644 --- a/common/src/app/common/logic/libraries.cljc +++ b/common/src/app/common/logic/libraries.cljc @@ -15,6 +15,7 @@ [app.common.geom.shapes :as gsh] [app.common.logging :as log] [app.common.logic.shapes :as cls] + [app.common.logic.variant-properties :as clvp] [app.common.spec :as us] [app.common.text :as txt] [app.common.types.color :as ctc] @@ -125,11 +126,12 @@ update-new-shape (fn [new-shape _] (cond-> new-shape - ; Link the new main to the new component + ; Link the new main to the new component (= (:component-id new-shape) (:id component)) (assoc :component-id new-component-id) - (some? variant-id) + ; If it is the instance root, add it the variant-id + (and (ctk/instance-root? new-shape) (some? variant-id)) (assoc :variant-id variant-id) :always @@ -215,8 +217,17 @@ {:keys [force-frame?] :or {force-frame? false}}] (let [component (ctf/get-component libraries file-id component-id) - parent (when parent-id (get objects parent-id)) library (get libraries file-id) + parent (when parent-id (get objects parent-id)) + + ;; When we are intanciating a variant, it can't be on a variant-container + parent (when parent + (if (and (ctk/is-variant? component) + (ctk/is-variant-container? parent)) + (get objects (:parent-id parent)) + parent)) + parent-id (d/nilv (:id parent) parent-id) + frame-id (d/nilv (:frame-id parent) frame-id) [new-shape new-shapes] (ctn/make-component-instance page @@ -351,8 +362,10 @@ (prepare-restore-component changes library-data component-id page nil nil nil nil))) ([changes library-data component-id page position old-id parent-id frame-id] - (let [component (ctkl/get-deleted-component library-data component-id) - parent (get-in page [:objects parent-id]) + (let [library-data (or (pcb/get-library-data changes) library-data) + component (ctkl/get-deleted-component library-data component-id) + objects (or (pcb/get-objects changes) (:objects page)) + parent (get objects parent-id) main-inst (get-in component [:objects (:main-instance-id component)]) inside-component? (some? (ctn/get-instance-root (:objects page) parent)) shapes (cfh/get-children-with-self (:objects component) (:main-instance-id component)) @@ -379,9 +392,7 @@ inside-component? (dissoc :component-root) (not inside-component?) - (assoc :component-root true) - (and is-variant? (some? parent-id)) - (assoc :variant-id parent-id)) + (assoc :component-root true)) changes (-> changes (pcb/with-page page) @@ -391,8 +402,15 @@ (some? old-id) (pcb/amend-last-change #(assoc % :old-id old-id))) ; on copy/paste old id is used later to reorder the paster layers changes (reduce #(pcb/add-object %1 %2 {:ignore-touched true}) changes - (rest moved-shapes))] - {:changes (pcb/restore-component changes component-id (:id page) minusdelta parent-id) + (rest moved-shapes)) + changes (cond-> changes + ;; Remove variant info when restoring into a parent that is not a variant-container + (and is-variant? parent (not (ctk/is-variant-container? parent))) + (clvp/generate-make-shapes-no-variant [first-shape]) + ;; Add variant info and rename when restoring into a variant-container + (ctk/is-variant-container? parent) + (clvp/generate-make-shapes-variant [first-shape] parent))] + {:changes (pcb/restore-component changes component-id (:id page) minusdelta) :shape (first moved-shapes)}))) ;; ---- General library synchronization functions ---- @@ -2137,6 +2155,7 @@ ;; When we duplicate a variant along with its variant-container, we will duplicate it in-variant-container? (contains? ids-map (:variant-id main)) + restore-component #(let [{:keys [shape changes]} (prepare-restore-component changes @@ -2312,8 +2331,8 @@ update-unames! (fn [new-name] (vswap! unames conj new-name)) all-ids (reduce #(into %1 (cons %2 (cfh/get-children-ids all-objects %2))) (d/ordered-set) ids) - ;; We need ids-map for remapping the grid layout. But when duplicating the guides - ;; we calculate a new one because the components will have created new shapes. + ;; We need ids-map for remapping the grid layout. But when duplicating the guides + ;; we calculate a new one because the components will have created new shapes. ids-map (into {} (map #(vector % (uuid/next))) all-ids) changes (-> changes diff --git a/common/src/app/common/logic/shapes.cljc b/common/src/app/common/logic/shapes.cljc index 09ee0ea5a..7016f6175 100644 --- a/common/src/app/common/logic/shapes.cljc +++ b/common/src/app/common/logic/shapes.cljc @@ -16,9 +16,7 @@ [app.common.types.shape.interactions :as ctsi] [app.common.types.shape.layout :as ctl] [app.common.types.token :as cto] - [app.common.types.variant :as ctv] - [app.common.uuid :as uuid] - [cuerdas.core :as str])) + [app.common.uuid :as uuid])) (defn- generate-unapply-tokens "When updating attributes that have a token applied, we must unapply it, because the value @@ -367,82 +365,11 @@ ;; Remove variant info and rename when moving outside a variant-container (cond-> (not (ctk/is-variant-container? parent)) - ((fn [changes] - (reduce - (fn [changes shape] - (let [new-name (str/replace (:variant-name shape) #", " " / ") - [cpath cname] (cfh/parse-path-name new-name)] - (-> changes - (pcb/update-component (:component-id shape) - #(-> (dissoc % :variant-id :variant-properties) - (assoc :name cname - :path cpath)) - {:apply-changes-local-library? true}) - (pcb/update-shapes [(:id shape)] - #(-> (dissoc % :variant-id :variant-name) - (assoc :name new-name)))))) - changes - variant-shapes)))) + (clvp/generate-make-shapes-no-variant variant-shapes)) ;; Add variant info and rename when moving into a different variant-container (cond-> (ctk/is-variant-container? parent) - ((fn [changes] - (let [get-base-name #(if (some? (:variant-name %)) - (str/replace (:variant-name %) #", " " / ") - (:name %)) - - calc-num-props #(-> % - get-base-name - cfh/split-path - count) - - max-path-items (apply max (map calc-num-props child-heads)) - - first-comp-id (->> parent - :shapes - first - (get objects) - :component-id) - - data (pcb/get-library-data changes) - variant-properties (get-in data [:components first-comp-id :variant-properties]) - num-props (count variant-properties) - num-new-props (if (< max-path-items num-props) - 0 - (- max-path-items num-props)) - - changes (nth - (iterate #(clvp/generate-add-new-property % (:id parent)) changes) - num-new-props)] - (reduce - (fn [changes shape] - (if (= (:id parent) (:variant-id shape)) - changes ;; do nothing if we aren't changing the parent - (let [base-name (get-base-name shape) - - ;; we need to get the updated library data to have access to the current properties - data (pcb/get-library-data changes) - - props (ctv/path-to-properties - base-name - (get-in data [:components first-comp-id :variant-properties])) - - variant-name (ctv/properties-to-name props) - [cpath cname] (cfh/parse-path-name (:name parent))] - - (-> (pcb/update-component changes - (:component-id shape) - #(assoc % :variant-id (:id parent) - :variant-properties props - :name cname - :path cpath) - {:apply-changes-local-library? true}) - (pcb/update-shapes [(:id shape)] - #(assoc % :variant-id (:id parent) - :variant-name variant-name - :name (:name parent))))))) - changes - child-heads))))) + (clvp/generate-make-shapes-variant child-heads parent)) ;; Move the shapes (pcb/change-parent parent-id diff --git a/common/src/app/common/logic/variant_properties.cljc b/common/src/app/common/logic/variant_properties.cljc index 2c489fa17..72e24183f 100644 --- a/common/src/app/common/logic/variant_properties.cljc +++ b/common/src/app/common/logic/variant_properties.cljc @@ -7,7 +7,9 @@ (:require [app.common.data :as d] [app.common.files.changes-builder :as pcb] + [app.common.files.helpers :as cfh] [app.common.files.variant :as cfv] + [app.common.types.component :as ctk] [app.common.types.components-list :as ctcl] [app.common.types.variant :as ctv] [cuerdas.core :as str])) @@ -64,7 +66,7 @@ objects (pcb/get-objects changes) related-components (cfv/find-variant-components data objects variant-id) - props (-> related-components first :variant-properties) + props (-> related-components last :variant-properties) next-prop-num (ctv/next-property-number props) property-name (or property-name (str ctv/property-prefix next-prop-num)) @@ -91,3 +93,116 @@ related-components)] changes)) +(defn- generate-make-shape-no-variant + [changes shape] + (let [data (pcb/get-library-data changes) + component (ctcl/get-component data (:component-id shape) true) + new-name (str (:name component) + " / " + (if (ctk/is-variant? shape) + (str/replace (:variant-name shape) #", " " / ") + (:name shape))) + [cpath cname] (cfh/parse-path-name new-name)] + (-> changes + (pcb/update-component (:component-id shape) + #(-> (dissoc % :variant-id :variant-properties) + (assoc :name cname + :path cpath)) + {:apply-changes-local-library? true}) + (pcb/update-shapes [(:id shape)] + #(-> (dissoc % :variant-id :variant-name) + (assoc :name new-name)))))) + +(defn generate-make-shapes-no-variant + [changes shapes] + (reduce generate-make-shape-no-variant changes shapes)) + +(defn generate-make-shapes-variant + [changes shapes variant-container] + (let [data (pcb/get-library-data changes) + objects (pcb/get-objects changes) + + container-name (:name variant-container) + long-name (str container-name " / ") + + get-base-name (fn [shape] + (let [component (ctcl/get-component data (:component-id shape) true) + + name (if (some? (:variant-name shape)) + (str (:name component) + " / " + (str/replace (:variant-name shape) #", " " / ")) + (:name shape))] + ;; When the name starts by the same name that the container, + ;; we should ignore that part of the name + (cond + (str/starts-with? name long-name) + (subs name (count long-name)) + + (str/starts-with? name container-name) + (subs name (count container-name)) + + :else + name))) + + calc-num-props #(-> % + get-base-name + cfh/split-path + count) + + max-path-items (apply max (map calc-num-props shapes)) + + ;; If we are cut-pasting a variant-container, this will be null + ;; because it hasn't any shapes yet + first-comp-id (->> variant-container + :shapes + first + (get objects) + :component-id) + + variant-properties (get-in data [:components first-comp-id :variant-properties]) + num-props (count variant-properties) + num-new-props (if (or (nil? first-comp-id) + (< max-path-items num-props)) + 0 + (- max-path-items num-props)) + total-props (+ num-props num-new-props) + + changes (nth + (iterate #(generate-add-new-property % (:id variant-container)) changes) + num-new-props) + + changes (pcb/update-shapes changes (map :id shapes) + #(assoc % :variant-id (:id variant-container) + :name (:name variant-container)))] + (reduce + (fn [changes shape] + (if (or (nil? first-comp-id) + (= (:id variant-container) (:variant-id shape))) + changes ;; do nothing if we aren't changing the parent + (let [base-name (get-base-name shape) + + ;; we need to get the updated library data to have access to the current properties + data (pcb/get-library-data changes) + + props (ctv/path-to-properties + base-name + (get-in data [:components first-comp-id :variant-properties]) + total-props) + + + + variant-name (ctv/properties-to-name props) + [cpath cname] (cfh/parse-path-name (:name variant-container))] + (-> (pcb/update-component changes + (:component-id shape) + #(assoc % :variant-id (:id variant-container) + :variant-properties props + :name cname + :path cpath) + {:apply-changes-local-library? true}) + (pcb/update-shapes [(:id shape)] + #(assoc % :variant-name variant-name)))))) + changes + shapes))) + diff --git a/common/src/app/common/types/component.cljc b/common/src/app/common/types/component.cljc index 18802e4d7..09aa6b6f1 100644 --- a/common/src/app/common/types/component.cljc +++ b/common/src/app/common/types/component.cljc @@ -334,8 +334,6 @@ (let [parent (get objects (:parent-id shape))] ;; We don't want to change the structure of component copies (and (not (in-component-copy-not-head? shape)) - ;; We don't want to duplicate variants - (not (is-variant? shape)) ;; Non instance, non copy. We allow (or (not (instance-head? shape)) (not (in-component-copy? parent)))))) diff --git a/common/src/app/common/types/components_list.cljc b/common/src/app/common/types/components_list.cljc index a66430b74..fd7534168 100644 --- a/common/src/app/common/types/components_list.cljc +++ b/common/src/app/common/types/components_list.cljc @@ -111,7 +111,6 @@ [file-data component-id f & args] (d/update-in-when file-data [:components component-id] #(-> (apply f % args) (touch)))) - (defn set-component-modified [file-data component-id] (update-component file-data component-id identity)) diff --git a/common/src/app/common/types/container.cljc b/common/src/app/common/types/container.cljc index 118692b7f..23373b152 100644 --- a/common/src/app/common/types/container.cljc +++ b/common/src/app/common/types/container.cljc @@ -481,12 +481,31 @@ no-changes? (and (->> children (every? #(= parent-id (:parent-id %)))) (not pasting?)) + + ;; When pasting frames, children have the frames and their children + ;; We need to check only the top shapes + children-ids (set (map :id children)) + top-children (remove #(contains? children-ids (:parent-id %)) children) + + ;; Are all the top-children a main-instance of a component? all-main? - (->> children (every? #(ctk/main-instance? %)))] + (->> top-children (every? #(ctk/main-instance? %))) + + ;; Are all the top-children a main-instance of a cutted component? + all-comp-cut? + (when all-main? + (->> top-children + (map #(ctkl/get-component (dm/get-in libraries [(:component-file %) :data]) + (:component-id %) + true)) + (every? :deleted)))] (if (or no-changes? (and (not (invalid-structure-for-component? objects parent children pasting? libraries)) ;; If we are moving into a variant-container, all the items should be main - (or all-main? (not (ctk/is-variant-container? parent))))) + ;; so if we are pasting, only allow main instances that are cut-and-pasted + (or (not (ctk/is-variant-container? parent)) + (and (not pasting?) all-main?) + all-comp-cut?))) [parent-id (get-frame parent-id)] (recur (:parent-id parent) objects children pasting? libraries)))))) diff --git a/common/src/app/common/types/file.cljc b/common/src/app/common/types/file.cljc index 8f453f3cd..dbe637f45 100644 --- a/common/src/app/common/types/file.cljc +++ b/common/src/app/common/types/file.cljc @@ -428,18 +428,23 @@ (defn restore-component "Recover a deleted component and all its shapes and put all this again in place." - [file-data component-id page-id parent-id] - (let [update-page? (not (nil? page-id)) - component (ctkl/get-component file-data component-id true) - update-variant? (and (some? parent-id) - (ctk/is-variant? component))] - (-> file-data - (ctkl/update-component component-id #(dissoc % :objects)) - (ctkl/mark-component-undeleted component-id) - (cond-> update-page? - (ctkl/update-component component-id #(assoc % :main-instance-page page-id))) - (cond-> update-variant? - (ctkl/update-component component-id #(assoc % :variant-id parent-id)))))) + [file-data component-id page-id] + (let [update-page? (not (nil? page-id)) + component (ctkl/get-component file-data component-id true) + main-instance-page (or page-id (:main-instance-page component)) + main-instance (dm/get-in file-data [:pages-index main-instance-page + :objects (:main-instance-id component)])] + (cond-> file-data + :always + (-> + (ctkl/update-component component-id #(dissoc % :objects)) + (ctkl/mark-component-undeleted component-id)) + + update-page? + (ctkl/update-component component-id #(assoc % :main-instance-page page-id)) + + (ctk/is-variant? component) + (ctkl/update-component component-id #(assoc % :variant-id (:variant-id main-instance)))))) (defn purge-component "Remove permanently a component." diff --git a/common/src/app/common/types/variant.cljc b/common/src/app/common/types/variant.cljc index a8db1e981..a3fb6348d 100644 --- a/common/src/app/common/types/variant.cljc +++ b/common/src/app/common/types/variant.cljc @@ -78,14 +78,18 @@ (defn path-to-properties "From a list of properties and a name with path, assign each token of the path as value of a different property" - [path properties] - (let [next-prop-num (next-property-number properties) - cpath (cfh/split-path path) - assigned (mapv #(assoc % :value (nth cpath %2 "")) properties (range)) - remaining (drop (count properties) cpath) - new-properties (map-indexed (fn [i v] {:name (str property-prefix (+ next-prop-num i)) - :value v}) remaining)] - (into assigned new-properties))) + ([path properties] + (path-to-properties path properties 0)) + ([path properties min-props] + (let [next-prop-num (next-property-number properties) + cpath (cfh/split-path path) + assigned (mapv #(assoc % :value (nth cpath %2 "")) properties (range)) + ;; Add empty strings to the end of path to reach the minimum number of properties + cpath (take min-props (concat path (repeat ""))) + remaining (drop (count properties) cpath) + new-properties (map-indexed (fn [i v] {:name (str property-prefix (+ next-prop-num i)) + :value v}) remaining)] + (into assigned new-properties)))) (defn properties-map-to-string diff --git a/frontend/src/app/main/data/workspace.cljs b/frontend/src/app/main/data/workspace.cljs index 00e2c1c46..9e5587328 100644 --- a/frontend/src/app/main/data/workspace.cljs +++ b/frontend/src/app/main/data/workspace.cljs @@ -2082,12 +2082,8 @@ libraries (dsh/lookup-libraries state) ldata (dsh/lookup-file-data state file-id) - ;; full-libs (assoc-in libraries [(:id ldata) :data] ldata) - - full-libs libraries - [parent-id - frame-id] (ctn/find-valid-parent-and-frame-ids candidate-parent-id page-objects (vals objects) true full-libs) + frame-id] (ctn/find-valid-parent-and-frame-ids candidate-parent-id page-objects (vals objects) true libraries) index (if (= candidate-parent-id parent-id) index @@ -2101,7 +2097,6 @@ all-objects (merge page-objects objects) - drop-cell (when (ctl/grid-layout? all-objects parent-id) (gslg/get-drop-cell frame-id all-objects position)) diff --git a/frontend/src/app/main/data/workspace/variants.cljs b/frontend/src/app/main/data/workspace/variants.cljs index 8bc9f90d8..37b65fc8b 100644 --- a/frontend/src/app/main/data/workspace/variants.cljs +++ b/frontend/src/app/main/data/workspace/variants.cljs @@ -344,7 +344,8 @@ (rx/of (add-new-variant main-instance-id) - (dwu/commit-undo-transaction undo-id))))))) + (dwu/commit-undo-transaction undo-id) + (ptk/data-event :layout/update {:ids [variant-id]}))))))) (defn add-component-or-variant "Manage the shared shortcut, and do the pertinent action" diff --git a/frontend/src/app/main/ui/ds/controls/combobox.cljs b/frontend/src/app/main/ui/ds/controls/combobox.cljs index 04a16ace0..fdfedad0a 100644 --- a/frontend/src/app/main/ui/ds/controls/combobox.cljs +++ b/frontend/src/app/main/ui/ds/controls/combobox.cljs @@ -75,6 +75,7 @@ (let [open* (mf/use-state false) open (deref open*) + ;;use-memo-equal selected* (mf/use-state default-selected) selected (deref selected*) @@ -215,6 +216,11 @@ (mf/with-effect [options] (mf/set-ref-val! options-ref options)) + (mf/use-effect + (mf/deps default-selected) + (fn [] + (reset! selected* default-selected))) + [:div {:ref combobox-ref :class (stl/css-case :combobox-wrapper true diff --git a/frontend/src/app/main/ui/workspace/context_menu.cljs b/frontend/src/app/main/ui/workspace/context_menu.cljs index 8b6878a16..5132bdb9c 100644 --- a/frontend/src/app/main/ui/workspace/context_menu.cljs +++ b/frontend/src/app/main/ui/workspace/context_menu.cljs @@ -146,7 +146,7 @@ do-cut #(st/emit! (dw/copy-selected) (dw/delete-selected)) do-paste #(st/emit! (dw/paste-from-clipboard)) - do-duplicate #(st/emit! (dw/duplicate-selected true)) + do-duplicate #(st/emit! (dwv/duplicate-or-add-variant)) enabled-paste-props* (mf/use-state false) diff --git a/frontend/src/app/main/ui/workspace/sidebar/options/shapes/frame.cljs b/frontend/src/app/main/ui/workspace/sidebar/options/shapes/frame.cljs index fe4cc9c09..dd1c6ea3b 100644 --- a/frontend/src/app/main/ui/workspace/sidebar/options/shapes/frame.cljs +++ b/frontend/src/app/main/ui/workspace/sidebar/options/shapes/frame.cljs @@ -7,6 +7,7 @@ (ns app.main.ui.workspace.sidebar.options.shapes.frame (:require [app.common.data.macros :as dm] + [app.common.types.component :as ctk] [app.common.types.shape.layout :as ctl] [app.main.features :as features] [app.main.refs :as refs] @@ -72,7 +73,7 @@ is-grid-layout? (ctl/grid-layout? shape) is-layout-child-absolute? (ctl/item-absolute? shape) variants? (features/use-feature "variants/v1") - is-variant? (when variants? (:is-variant-container shape))] + is-variant? (when variants? (ctk/is-variant-container? shape))] [:* [:& layer-menu {:ids ids