From 4a0b826ddc543743503d83d2b221a04786d1fcba Mon Sep 17 00:00:00 2001 From: "alonso.torres" Date: Wed, 14 Oct 2026 15:46:15 +0200 Subject: [PATCH] :recycle: Build the changes of the variant property events once - Removing, reordering and setting the value of a variant property each wrote the component's new properties and then the variant name of its main, built from them. set-variant-properties does both. - ctv had two functions that find the position of a property by name; the private find-index-by-name goes. - generate-reorder-variant-poperties and the frontend event reorder-variant-poperties are spelled reorder-variant-properties. - Nine events of workspace/variants (set the value, the name, the error, the variant id, add, remove, remove the empty ones, reorder, and update after a rename in the layers) each looked up the page, its objects and the file data, built the same empty changes, and committed them inside an undo transaction. variant-changes builds those changes, and the transactions go: each one held a single commit, which already makes one undo entry on its own, or joins the transaction a caller has open. No behavior change: the golden masters and the suites are unchanged. AI-assisted-by: claude-opus-5-5 --- .../app/common/logic/variant_properties.cljc | 48 ++--- .../test_helpers/composable/comp/nodes.cljc | 2 +- common/src/app/common/types/variant.cljc | 10 +- .../src/app/main/data/workspace/variants.cljs | 175 ++++-------------- .../sidebar/options/menus/component.cljs | 4 +- .../composable_tests/interpreter.cljs | 2 +- 6 files changed, 63 insertions(+), 178 deletions(-) diff --git a/common/src/app/common/logic/variant_properties.cljc b/common/src/app/common/logic/variant_properties.cljc index fe87fa3e5e..a880dd6eaf 100644 --- a/common/src/app/common/logic/variant_properties.cljc +++ b/common/src/app/common/logic/variant_properties.cljc @@ -14,6 +14,16 @@ [app.common.types.variant :as ctv] [cuerdas.core :as str])) +(defn- set-variant-properties + "Set the variant properties of `component` to `props`, and the variant name + of its main to match them." + [changes component props] + (-> changes + (pcb/update-component (:id component) #(assoc % :variant-properties props) + {:apply-changes-local-library? true}) + (pcb/update-shapes [(:main-instance-id component)] + #(assoc % :variant-name (ctv/properties-to-name props))))) + (defn generate-update-property-name [changes variant-id pos new-name] (let [data (pcb/get-library-data changes) @@ -42,14 +52,8 @@ props (-> related-components first :variant-properties)] (if (and (seq props) (<= 0 pos) (< pos (count props))) (reduce (fn [changes component] - (let [props (:variant-properties component) - props (d/remove-at-index props pos) - main-id (:main-instance-id component) - name (ctv/properties-to-name props)] - (-> changes - (pcb/update-component (:id component) #(assoc % :variant-properties props) - {:apply-changes-local-library? true}) - (pcb/update-shapes [main-id] #(assoc % :variant-name name))))) + (set-variant-properties changes component + (d/remove-at-index (:variant-properties component) pos))) changes related-components) changes))) @@ -57,16 +61,9 @@ (defn generate-update-property-value [changes component-id pos value] - (let [data (pcb/get-library-data changes) - component (ctcl/get-component data component-id true) - main-id (:main-instance-id component) - name (-> (:variant-properties component) - (update pos assoc :value value) - ctv/properties-to-name)] - (-> changes - (pcb/update-component component-id #(assoc-in % [:variant-properties pos :value] value) - {:apply-changes-local-library? true}) - (pcb/update-shapes [main-id] #(assoc % :variant-name name))))) + (let [component (ctcl/get-component (pcb/get-library-data changes) component-id true)] + (set-variant-properties changes component + (update (:variant-properties component) pos assoc :value value)))) (defn generate-set-variant-error @@ -80,22 +77,15 @@ #(assoc % :variant-error value)))))) -(defn generate-reorder-variant-poperties +(defn generate-reorder-variant-properties [changes variant-id from-pos to-space-between-pos] (let [data (pcb/get-library-data changes) objects (pcb/get-objects changes) related-components (cfv/find-variant-components data objects variant-id)] (reduce (fn [changes component] - (let [props (:variant-properties component) - props (d/reorder props from-pos to-space-between-pos) - main-id (:main-instance-id component) - name (ctv/properties-to-name props)] - (-> changes - (pcb/update-component (:id component) - #(assoc % :variant-properties props) - {:apply-changes-local-library? true}) - (pcb/update-shapes [main-id] - #(assoc % :variant-name name))))) + (set-variant-properties changes component + (d/reorder (:variant-properties component) + from-pos to-space-between-pos))) changes related-components))) diff --git a/common/src/app/common/test_helpers/composable/comp/nodes.cljc b/common/src/app/common/test_helpers/composable/comp/nodes.cljc index 3f06d7ef41..3bf1572b54 100644 --- a/common/src/app/common/test_helpers/composable/comp/nodes.cljc +++ b/common/src/app/common/test_helpers/composable/comp/nodes.cljc @@ -1499,7 +1499,7 @@ (clvp/generate-add-new-property changes variant-id opts)) :remove (apply clvp/generate-remove-property changes args) :rename (apply clvp/generate-update-property-name changes args) - :reorder (apply clvp/generate-reorder-variant-poperties changes args) + :reorder (apply clvp/generate-reorder-variant-properties changes args) :set-value (apply clvp/generate-update-property-value changes args))) (defrecord EditVariantProperty [set-name edit params] diff --git a/common/src/app/common/types/variant.cljc b/common/src/app/common/types/variant.cljc index 660fec5304..3b255645f2 100644 --- a/common/src/app/common/types/variant.cljc +++ b/common/src/app/common/types/variant.cljc @@ -241,14 +241,6 @@ (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] @@ -261,7 +253,7 @@ "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) + (or (find-index-for-property-name props name) (next-valid-position used-pos))) (defn merge-properties diff --git a/frontend/src/app/main/data/workspace/variants.cljs b/frontend/src/app/main/data/workspace/variants.cljs index 2b0474463f..9dfd89a3f5 100644 --- a/frontend/src/app/main/data/workspace/variants.cljs +++ b/frontend/src/app/main/data/workspace/variants.cljs @@ -38,6 +38,16 @@ [beicon.v2.core :as rx] [potok.v2.core :as ptk])) +(defn- variant-changes + "The empty changes of the current page, with its objects and the data of + the file." + [it state] + (let [page-id (:current-page-id state) + data (dsh/lookup-file-data state)] + (-> (pcb/empty-changes it page-id) + (pcb/with-library-data data) + (pcb/with-objects (:objects (dsh/get-page data page-id)))))) + (defn update-properties-names-and-values "Compares the previous properties with the updated ones and executes the correspondent action for each one depending on if it needs to be removed, updated or added" @@ -49,26 +59,17 @@ ptk/WatchEvent (watch [it state _] - (let [page-id (:current-page-id state) - data (dsh/lookup-file-data state) - objects (-> (dsh/get-page data page-id) - (get :objects)) - - updated-properties (ctv/update-number-in-repeated-prop-names updated-properties) + (let [updated-properties (ctv/update-number-in-repeated-prop-names updated-properties) properties-to-remove (ctv/find-properties-to-remove previous-properties updated-properties) properties-to-add (ctv/find-properties-to-add previous-properties updated-properties) properties-to-update (ctv/find-properties-to-update previous-properties updated-properties) - changes (-> (pcb/empty-changes it page-id) - (pcb/with-objects objects) - (pcb/with-library-data data)) - changes (reduce (fn [changes {:keys [name]}] (-> changes (clvp/generate-update-property-value component-id (ctv/find-index-for-property-name previous-properties name) ""))) - changes + (variant-changes it state) properties-to-remove) changes (reduce @@ -84,18 +85,14 @@ (clvp/generate-add-new-property variant-id {:property-name name}) (clvp/generate-update-property-value component-id (+ idx (count previous-properties)) value))) changes - (map-indexed vector properties-to-add)) - - undo-id (js/Symbol)] + (map-indexed vector properties-to-add))] (rx/of (when (or (seq properties-to-remove) (seq properties-to-update)) (ev/event {::ev/name "variant-edit-property-value" ::ev/origin "workspace:rename-in-layers"})) (when (seq properties-to-add) (ev/event {::ev/name "variant-add-property" ::ev/origin "workspace:rename-in-layers"})) - (dwu/start-undo-transaction undo-id) - (dch/commit-changes changes) - (dwu/commit-undo-transaction undo-id)))))) + (dch/commit-changes changes)))))) (defn update-property-name "Update the variant property name on the position pos @@ -120,28 +117,15 @@ ptk/WatchEvent (watch [it state _] - (let [page-id (:current-page-id state) - data (dsh/lookup-file-data state) - objects (-> (dsh/get-page data page-id) - (get :objects)) - - related-components (cfv/find-variant-components data objects variant-id) - - props (-> related-components last :variant-properties) - valid-pos? (> (count props) pos) - prop-name (when valid-pos? (-> props (nth pos) :name)) - - changes (when valid-pos? - (-> (pcb/empty-changes it page-id) - (pcb/with-objects objects) - (pcb/with-library-data data) - (clvp/generate-update-property-name variant-id pos new-name))) - undo-id (js/Symbol)] + (let [data (dsh/lookup-file-data state) + objects (dsh/lookup-page-objects state) + props (-> (cfv/find-variant-components data objects variant-id) last :variant-properties) + valid-pos? (> (count props) pos) + prop-name (when valid-pos? (-> props (nth pos) :name))] (when (and valid-pos? (not= prop-name new-name)) (rx/of - (dwu/start-undo-transaction undo-id) - (dch/commit-changes changes) - (dwu/commit-undo-transaction undo-id) + (dch/commit-changes (-> (variant-changes it state) + (clvp/generate-update-property-name variant-id pos new-name))) (ev/event {::ev/name "variant-edit-property-name" ::ev/origin trigger}))))))) (defn update-property-value @@ -150,20 +134,8 @@ (ptk/reify ::update-property-value ptk/WatchEvent (watch [it state _] - (let [page-id (:current-page-id state) - data (dsh/lookup-file-data state) - objects (-> (dsh/get-page data page-id) - (get :objects)) - - changes (-> (pcb/empty-changes it page-id) - (pcb/with-library-data data) - (pcb/with-objects objects) - (clvp/generate-update-property-value component-id pos value)) - undo-id (js/Symbol)] - (rx/of - (dwu/start-undo-transaction undo-id) - (dch/commit-changes changes) - (dwu/commit-undo-transaction undo-id)))))) + (rx/of (dch/commit-changes (-> (variant-changes it state) + (clvp/generate-update-property-value component-id pos value))))))) (defn update-error "Sets or unsets an error for a component" @@ -173,20 +145,8 @@ (ptk/reify ::update-error ptk/WatchEvent (watch [it state _] - (let [page-id (:current-page-id state) - data (dsh/lookup-file-data state) - objects (-> (dsh/get-page data page-id) - (get :objects)) - - changes (-> (pcb/empty-changes it page-id) - (pcb/with-library-data data) - (pcb/with-objects objects) - (clvp/generate-set-variant-error component-id value)) - undo-id (js/Symbol)] - (rx/of - (dwu/start-undo-transaction undo-id) - (dch/commit-changes changes) - (dwu/commit-undo-transaction undo-id))))))) + (rx/of (dch/commit-changes (-> (variant-changes it state) + (clvp/generate-set-variant-error component-id value)))))))) (defn remove-property "Remove the variant property on the position pos @@ -195,21 +155,8 @@ (ptk/reify ::remove-property ptk/WatchEvent (watch [it state _] - (let [page-id (:current-page-id state) - data (dsh/lookup-file-data state) - objects (-> (dsh/get-page data page-id) - (get :objects)) - - changes (-> (pcb/empty-changes it page-id) - (pcb/with-library-data data) - (pcb/with-objects objects) - (clvp/generate-remove-property variant-id pos)) - - undo-id (js/Symbol)] - (rx/of - (dwu/start-undo-transaction undo-id) - (dch/commit-changes changes) - (dwu/commit-undo-transaction undo-id)))))) + (rx/of (dch/commit-changes (-> (variant-changes it state) + (clvp/generate-remove-property variant-id pos))))))) (defn remove-empty-properties "Remove every empty property for all components when their respective values are empty @@ -218,10 +165,8 @@ (ptk/reify ::remove-empty-properties ptk/WatchEvent (watch [it state _] - (let [page-id (:current-page-id state) - data (dsh/lookup-file-data state) - objects (-> (dsh/get-page data page-id) - (get :objects)) + (let [data (dsh/lookup-file-data state) + objects (dsh/lookup-page-objects state) variant-components (cfv/find-variant-components data objects variant-id) @@ -236,27 +181,19 @@ empty?)])) (reverse)) - changes (-> (pcb/empty-changes it page-id) - (pcb/with-library-data data) - (pcb/with-objects objects)) - changes (reduce (fn [changes [pos property-empty?]] (if property-empty? (-> changes (clvp/generate-remove-property variant-id pos)) changes)) - changes - properties-empty-pos) - - undo-id (js/Symbol)] + (variant-changes it state) + properties-empty-pos)] (when (seq (:redo-changes changes)) (rx/of (ev/event {::ev/name "variant-remove-property" ::ev/origin "workspace:rename-in-layers"}) - (dwu/start-undo-transaction undo-id) - (dch/commit-changes changes) - (dwu/commit-undo-transaction undo-id))))))) + (dch/commit-changes changes))))))) (defn add-new-property "Add a new variant property to all the components with this variant-id" @@ -264,43 +201,17 @@ (ptk/reify ::add-new-property ptk/WatchEvent (watch [it state _] - (let [page-id (:current-page-id state) - data (dsh/lookup-file-data state) - objects (-> (dsh/get-page data page-id) - (get :objects)) + (rx/of (dch/commit-changes (-> (variant-changes it state) + (clvp/generate-add-new-property variant-id options))))))) - changes (-> (pcb/empty-changes it page-id) - (pcb/with-library-data data) - (pcb/with-objects objects) - (clvp/generate-add-new-property variant-id options)) - - undo-id (js/Symbol)] - (rx/of - (dwu/start-undo-transaction undo-id) - (dch/commit-changes changes) - (dwu/commit-undo-transaction undo-id)))))) - -(defn reorder-variant-poperties +(defn reorder-variant-properties "Reorder properties by moving a property from some position to some space between positions" [variant-id from-pos to-space-between-pos] (ptk/reify ::reorder-variant-properties ptk/WatchEvent (watch [it state _] - (let [page-id (:current-page-id state) - data (dsh/lookup-file-data state) - objects (-> (dsh/get-page data page-id) - (get :objects)) - - changes (-> (pcb/empty-changes it page-id) - (pcb/with-library-data data) - (pcb/with-objects objects) - (clvp/generate-reorder-variant-poperties variant-id from-pos to-space-between-pos)) - - undo-id (js/Symbol)] - (rx/of - (dwu/start-undo-transaction undo-id) - (dch/commit-changes changes) - (dwu/commit-undo-transaction undo-id)))))) + (rx/of (dch/commit-changes (-> (variant-changes it state) + (clvp/generate-reorder-variant-properties variant-id from-pos to-space-between-pos))))))) (defn- set-variant-id "Sets the variant-id on a component" @@ -308,16 +219,8 @@ (ptk/reify ::set-variant-id ptk/WatchEvent (watch [it state _] - (let [page-id (:current-page-id state) - data (dsh/lookup-file-data state) - changes (-> (pcb/empty-changes it page-id) - (pcb/with-library-data data) - (pcb/update-component component-id #(assoc % :variant-id variant-id))) - undo-id (js/Symbol)] - (rx/of - (dwu/start-undo-transaction undo-id) - (dch/commit-changes changes) - (dwu/commit-undo-transaction undo-id)))))) + (rx/of (dch/commit-changes (-> (variant-changes it state) + (pcb/update-component component-id #(assoc % :variant-id variant-id)))))))) (defn- focus-property [variant-id] diff --git a/frontend/src/app/main/ui/workspace/sidebar/options/menus/component.cljs b/frontend/src/app/main/ui/workspace/sidebar/options/menus/component.cljs index bcaa042e88..2f95e07918 100644 --- a/frontend/src/app/main/ui/workspace/sidebar/options/menus/component.cljs +++ b/frontend/src/app/main/ui/workspace/sidebar/options/menus/component.cljs @@ -420,7 +420,7 @@ (mf/use-fn (mf/deps variant-id) (fn [from-pos to-space-between-pos] - (st/emit! (dwv/reorder-variant-poperties variant-id from-pos to-space-between-pos))))] + (st/emit! (dwv/reorder-variant-properties variant-id from-pos to-space-between-pos))))] [:* [:> h/sortable-container* {} @@ -1271,7 +1271,7 @@ (mf/use-fn (mf/deps variant-id) (fn [from-pos to-space-between-pos] - (st/emit! (dwv/reorder-variant-poperties variant-id from-pos to-space-between-pos)))) + (st/emit! (dwv/reorder-variant-properties variant-id from-pos to-space-between-pos)))) select-shapes-with-malformed (mf/use-fn diff --git a/frontend/test/frontend_tests/composable_tests/interpreter.cljs b/frontend/test/frontend_tests/composable_tests/interpreter.cljs index d33f9f95c2..f98a90fffb 100644 --- a/frontend/test/frontend_tests/composable_tests/interpreter.cljs +++ b/frontend/test/frontend_tests/composable_tests/interpreter.cljs @@ -251,7 +251,7 @@ :remove (apply dwv/remove-property args) :rename (let [[variant-id pos name] args] (dwv/update-property-name variant-id pos name {:trigger "workspace:design-tab"})) - :reorder (apply dwv/reorder-variant-poperties args) + :reorder (apply dwv/reorder-variant-properties args) :set-value (apply dwv/update-property-value args))]) (instance? n/Detach op)