From f4b16a255cd1bd9f1b1ab2a6f7f8ebec68191412 Mon Sep 17 00:00:00 2001 From: Pablo Alba Date: Tue, 15 Apr 2025 12:06:58 +0200 Subject: [PATCH] :sparkles: Copy values of same named properties moving a variant into another (#6288) * :sparkles: Copy values of same named properties moving a variant into another * :sparkles: Add MR changes --- common/src/app/common/files/validate.cljc | 2 +- .../app/common/logic/variant_properties.cljc | 136 +++++++++--------- common/src/app/common/types/variant.cljc | 100 +++++++++++-- frontend/playwright/ui/specs/variants.spec.js | 2 +- .../src/app/main/data/workspace/variants.cljs | 3 + 5 files changed, 163 insertions(+), 80 deletions(-) diff --git a/common/src/app/common/files/validate.cljc b/common/src/app/common/files/validate.cljc index 729c61eb89..2659f65336 100644 --- a/common/src/app/common/files/validate.cljc +++ b/common/src/app/common/files/validate.cljc @@ -436,7 +436,7 @@ child file page)) (when (not= prop-names (cfv/extract-properties-names child (:data file))) (report-error :invalid-variant-properties - (str/ffmt "Variant % has invalid properties" (:id child)) + (str/ffmt "Variant % has invalid properties %" (:id child) (vec prop-names)) child file page))))))) (defn- check-variant diff --git a/common/src/app/common/logic/variant_properties.cljc b/common/src/app/common/logic/variant_properties.cljc index 72e24183f9..c35cdfccda 100644 --- a/common/src/app/common/logic/variant_properties.cljc +++ b/common/src/app/common/logic/variant_properties.cljc @@ -117,86 +117,83 @@ [changes shapes] (reduce generate-make-shape-no-variant changes shapes)) + +(defn- generate-new-properties-from-variant + [shape min-props data container-name base-properties] + (let [component (ctcl/get-component data (:component-id shape) true) + add-name? (not= (:name component) container-name) + props (ctv/merge-properties base-properties + (:variant-properties component)) + new-props (- min-props + (+ (count props) + (if add-name? 1 0))) + props (ctv/add-new-props props (repeat new-props ""))] + + (if add-name? + (ctv/add-new-prop props (:name component)) + props))) + +(defn- generate-new-properties-from-non-variant + [shape min-props container-name base-properties] + (let [;; Remove container name from shape name if present + shape-name (ctv/remove-prefix (:name shape) container-name)] + (ctv/path-to-properties shape-name base-properties min-props))) + + (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)) + (let [data (pcb/get-library-data changes) + objects (pcb/get-objects changes) + variant-id (:id variant-container) ;; 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) + 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) + base-props (->> (get-in data [:components first-comp-id :variant-properties]) + (map #(assoc % :value ""))) + num-base-props (count base-props) - changes (nth - (iterate #(generate-add-new-property % (:id variant-container)) changes) - num-new-props) + [cpath cname] (cfh/parse-path-name (:name variant-container)) + container-name (:name variant-container) - changes (pcb/update-shapes changes (map :id shapes) - #(assoc % :variant-id (:id variant-container) - :name (:name variant-container)))] + generate-new-properties + (fn [shape min-props] + (if (ctk/is-variant? shape) + (generate-new-properties-from-variant shape min-props data container-name base-props) + (generate-new-properties-from-non-variant shape min-props container-name base-props))) + + total-props (reduce (fn [m shape] + (max m (count (generate-new-properties shape num-base-props)))) + 0 + shapes) + + num-new-props (if (or (zero? num-base-props) + (< total-props num-base-props)) + 0 + (- total-props num-base-props)) + + changes (nth + (iterate #(generate-add-new-property % variant-id) changes) + num-new-props) + + changes (pcb/update-shapes changes (map :id shapes) + #(assoc % :variant-id variant-id + :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))] + (if (or (zero? num-base-props) + (= variant-id (:variant-id shape))) + changes ;; do nothing more if we aren't changing the parent or there are no base props + (let [props (generate-new-properties shape total-props) + variant-name (ctv/properties-to-name props)] (-> (pcb/update-component changes (:component-id shape) - #(assoc % :variant-id (:id variant-container) + #(assoc % :variant-id variant-id :variant-properties props :name cname :path cpath) @@ -204,5 +201,4 @@ (pcb/update-shapes [(:id shape)] #(assoc % :variant-name variant-name)))))) changes - shapes))) - + shapes))) \ No newline at end of file diff --git a/common/src/app/common/types/variant.cljc b/common/src/app/common/types/variant.cljc index a3fb6348da..8a5bcb2a0e 100644 --- a/common/src/app/common/types/variant.cljc +++ b/common/src/app/common/types/variant.cljc @@ -74,6 +74,20 @@ 0)] (inc (max max-num (count properties))))) +(defn add-new-prop + "Adds a new property with generated name and provided value to the existing props list." + [props value] + (conj props {:name (str property-prefix (next-property-number props)) + :value value})) + +(defn add-new-props + "Adds new properties with generated names and provided values to the existing props list." + [props values] + (let [next-prop-num (next-property-number props) + xf (map-indexed (fn [i v] + {:name (str property-prefix (+ next-prop-num i)) + :value v}))] + (into props xf values))) (defn path-to-properties "From a list of properties and a name with path, assign each token of the @@ -81,15 +95,13 @@ ([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) + (let [cpath (cfh/split-path path) + total-props (max (count cpath) min-props) 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)))) + ;; Add empty strings to the end of cpath to reach the minimum number of properties + cpath (take total-props (concat cpath (repeat ""))) + remaining (drop (count properties) cpath)] + (add-new-props assigned remaining)))) (defn properties-map-to-string @@ -147,3 +159,75 @@ (when (= (:name prop) name) idx)) (map-indexed vector props))) + +(defn remove-prefix + "Removes the given prefix (with or without a trailing ' / ') from the beginning of the name" + [name prefix] + (let [long-name (str prefix " / ")] + (cond + (str/starts-with? name long-name) + (subs name (count long-name)) + + (str/starts-with? name prefix) + (subs name (count prefix)) + + :else + name))) + +(def ^:private xf:map-name + (map :name)) + +(defn- matching-indices + [props1 props2] + (let [names-in-p2 (into #{} xf:map-name props2) + xform (comp + (map-indexed (fn [index {:keys [name]}] + (when (contains? names-in-p2 name) + index))) + (filter some?))] + (into #{} xform props1))) + +(defn- find-index-by-name + "Returns the index of the first item in props with the given name, or nil if not found." + [name props] + (some (fn [[idx item]] + (when (= (:name item) name) + idx)) + (map-indexed vector props))) + +(defn- next-valid-position + "Returns the first non-negative integer not present in the used-pos set." + [used-pos] + (loop [p 0] + (if (contains? used-pos p) + (recur (inc p)) + p))) + +(defn- find-position + "Returns the index of the property with the given name in `props`, + or the next available index not in `used-pos` if not found." + [name props used-pos] + (or (find-index-by-name name props) + (next-valid-position used-pos))) + +(defn merge-properties + "Merges props2 into props1 with the following rules: + - For each property p2 in props2: + - Skip it if its value is empty. + - If props1 contains a property with the same name, update its value with that of p2. + - Otherwise, assign p2's value to the first unused property in props1. A property is considered used if: + - Its name exists in both props1 and props2, or + - Its value has already been updated during the merge. + - If no unused properties are available in props1, append a new property with a default name and p2's value." + [props1 props2] + (let [props2 (remove #(str/empty? (:value %)) props2)] + (-> (reduce + (fn [{:keys [props used-pos]} prop] + (let [pos (find-position (:name prop) props used-pos) + used-pos (conj used-pos pos)] + (if (< pos (count props)) + {:props (assoc-in (vec props) [pos :value] (:value prop)) :used-pos used-pos} + {:props (add-new-prop props (:value prop)) :used-pos used-pos}))) + {:props (vec props1) :used-pos (matching-indices props1 props2)} + props2) + :props))) \ No newline at end of file diff --git a/frontend/playwright/ui/specs/variants.spec.js b/frontend/playwright/ui/specs/variants.spec.js index 9cbc230ff2..9b042341b5 100644 --- a/frontend/playwright/ui/specs/variants.spec.js +++ b/frontend/playwright/ui/specs/variants.spec.js @@ -419,7 +419,7 @@ test("User cut paste a variant into another container", async ({ page }) => { const variant3 = await workspacePage.layers .getByTestId("layer-row") - .filter({ has: workspacePage.page.getByText("rectangle, Value 1") }) + .filter({ has: workspacePage.page.getByText("Value 1, rectangle") }) .filter({ has: workspacePage.page.locator(".icon-variant") }) .first(); diff --git a/frontend/src/app/main/data/workspace/variants.cljs b/frontend/src/app/main/data/workspace/variants.cljs index 37b65fc8ba..1566ba4e3f 100644 --- a/frontend/src/app/main/data/workspace/variants.cljs +++ b/frontend/src/app/main/data/workspace/variants.cljs @@ -403,6 +403,7 @@ (defn rename-variant + "Rename the variant container and all components belonging to this variant" [variant-id name] (ptk/reify ::rename-variant @@ -426,6 +427,8 @@ (defn rename-comp-or-variant-and-main + "If the component is in a variant, rename the variant. + If it is not, rename the component and its main" [component-id name] (ptk/reify ::rename-comp-or-variant-and-main