From 33e9caead32df6a32970cff4e7417bf101d476cb Mon Sep 17 00:00:00 2001 From: "alonso.torres" Date: Fri, 9 Oct 2026 11:50:56 +0200 Subject: [PATCH] :recycle: Move variant switch and swap setup to common The variant switch event picked its target variant, and the component swap event worked out where the new head goes, inside the frontend events. Code that runs without the frontend store, such as the pure composable runner, could not do the same switch or swap. The swap and the switch also checked for nesting loops with two different functions. Move them to app.common.logic.variants: - find-switch-target picks the nearest variant with the requested value. - generate-component-swap-in-place builds the swap changes keeping the old head's index, grid cell and swap-keep-attrs, plus the keep-touched step when asked. - swap-nesting-loop? checks that a swap or switch does not nest a component inside itself. The swap menu, the plugin swapComponent and the variants-switch event all use it. The frontend events behave as before, with two exceptions. Switching to a value no other variant has does nothing, where it threw in min-key. A swap also updates the layout of the shapes the keep-touched step adds. --- common/src/app/common/logic/variants.cljc | 72 +++++++++ .../logic/variants_switch_target_test.cljc | 153 ++++++++++++++++++ common/test/common_tests/runner.cljc | 2 + .../app/main/data/workspace/libraries.cljs | 49 +----- .../src/app/main/data/workspace/variants.cljs | 49 ++---- .../sidebar/options/menus/component.cljs | 3 +- frontend/src/app/plugins/shape.cljs | 3 +- 7 files changed, 251 insertions(+), 80 deletions(-) create mode 100644 common/test/common_tests/logic/variants_switch_target_test.cljc diff --git a/common/src/app/common/logic/variants.cljc b/common/src/app/common/logic/variants.cljc index 883a3a40da..446403eb3a 100644 --- a/common/src/app/common/logic/variants.cljc +++ b/common/src/app/common/logic/variants.cljc @@ -8,8 +8,10 @@ [app.common.logic.shapes :as cls] [app.common.logic.variant-properties :as clvp] [app.common.types.component :as ctk] + [app.common.types.components-list :as ctkl] [app.common.types.container :as ctn] [app.common.types.file :as ctf] + [app.common.types.shape.layout :as ctsl] [app.common.types.variant :as ctv] [app.common.uuid :as uuid])) @@ -241,3 +243,73 @@ orig-touched)] [changes parents-of-swapped])) +(defn- shape-index + "Position of `shape-id` among the children of `parent-id`; 0 when it is not + among them, nil when the parent does not exist." + [objects parent-id shape-id] + (when-let [parent (get objects parent-id)] + (or (d/index-of (:shapes parent) shape-id) 0))) + +(defn generate-component-swap-in-place + "Swap the copy head `shape` of `page` for a copy of the component + `component-id` of library data `ldata`. The new head takes the old one's + place: its index in the parent, its grid cell and its `swap-keep-attrs`. + With `keep-touched?`, the old head's overrides are carried over (the + variant switch flavour). Returns `[new-shape changes updated-parent-ids]`, + where the parent ids need a layout update." + [changes page libraries ldata shape component-id keep-touched?] + (let [objects (:objects page) + parent (get objects (:parent-id shape)) + orig-shapes (when keep-touched? + (cfh/get-children-with-self objects (:id shape))) + target-cell (when (ctsl/grid-layout? parent) + (ctsl/get-cell-by-shape-id parent (:id shape))) + index (shape-index objects (:parent-id shape) (:id shape)) + keep-props (select-keys shape ctk/swap-keep-attrs) + + [new-shape all-parents changes] + (cll/generate-component-swap changes objects shape ldata page libraries component-id + index target-cell keep-props keep-touched?) + + [changes parents-of-swapped] + (if keep-touched? + (generate-keep-touched changes new-shape shape orig-shapes page libraries ldata) + [changes []])] + [new-shape changes (concat all-parents parents-of-swapped)])) + +(defn find-switch-target + "The variant component that the variant copy head `shape` switches to when + its property at position `pos` takes the value `val`: among the other + variants with that value, the one whose properties are nearest to the + current ones. Nil when the property already has that value or no variant + has it." + [libraries shape pos val] + (let [component-id (:component-id shape) + component (ctf/get-component libraries (:component-file shape) component-id + :include-deleted? false)] + (when (not= val (get-in component [:variant-properties pos :value])) + (let [ldata (get-in libraries [(:component-file shape) :data]) + objects (-> (ctf/get-component-page ldata component) :objects) + target-props (-> (:variant-properties component) + (update pos assoc :value val)) + candidates (->> (cfv/find-variant-components ldata objects (:variant-id component)) + (remove #(= (:id %) component-id)) + (filter #(= (get-in % [:variant-properties pos :value]) val)) + (reverse))] + (when (seq candidates) + (apply min-key #(ctv/distance target-props (:variant-properties %)) candidates)))))) + +(defn swap-nesting-loop? + "Whether swapping, or switching, the copy head `shape` of `objects` for a + copy of the component `component-id` of library data `ldata` would nest a + component inside itself." + [objects shape ldata component-id] + (let [component (ctkl/get-component ldata component-id true) + page (ctf/get-component-page ldata component) + root (ctf/get-component-root ldata component)] + (boolean + (and page + root + (cfh/components-nesting-loop? + (cfh/get-children-with-self (:objects page) (:id root)) + (cfh/get-parents-with-self objects (:parent-id shape))))))) diff --git a/common/test/common_tests/logic/variants_switch_target_test.cljc b/common/test/common_tests/logic/variants_switch_target_test.cljc new file mode 100644 index 0000000000..77cf315e54 --- /dev/null +++ b/common/test/common_tests/logic/variants_switch_target_test.cljc @@ -0,0 +1,153 @@ +;; This Source Code Form is subject to the terms of the Mozilla Public +;; License, v. 2.0. If a copy of the MPL was not distributed with this +;; file, You can obtain one at http://mozilla.org/MPL/2.0/. +;; +;; Copyright (c) KALEIDOS SUBSIDIARY SL + +(ns common-tests.logic.variants-switch-target-test + (:require + [app.common.data :as d] + [app.common.files.changes-builder :as pcb] + [app.common.logic.variants :as clv] + [app.common.test-helpers.components :as thc] + [app.common.test-helpers.files :as thf] + [app.common.test-helpers.ids-map :as thi] + [app.common.test-helpers.shapes :as ths] + [app.common.test-helpers.variants :as thv] + [app.common.types.shape.layout :as ctl] + [clojure.test :as t])) + +(t/use-fixtures :each thi/test-fixture) + +(defn- libraries + [file] + {(:id file) file}) + +(defn- add-third-variant + "Add a component :c03 with properties p1v2, p2v1 to the variant :v01 built + by `thv/add-variant-two-properties`." + [file] + (let [variant-id (thi/id :v01)] + (-> file + (ths/add-sample-shape :m03 :type :frame :parent-label :v01 + :variant-id variant-id :variant-name "p1v2, p2v1") + (thc/make-component :c03 :m03) + (thc/update-component :c03 {:variant-id variant-id + :variant-properties [{:name "Property 1" :value "p1v2"} + {:name "Property 2" :value "p2v1"}]})))) + +(t/deftest switch-target-has-the-requested-value + (let [file (-> (thf/sample-file :file1) + (thv/add-variant :v01 :c01 :m01 :c02 :m02) + (thc/instantiate-component :c01 :copy01)) + target (clv/find-switch-target (libraries file) (ths/get-shape file :copy01) 0 "Value2")] + (t/is (= (thi/id :c02) (:id target))))) + +(t/deftest switch-target-is-nil-when-the-value-is-current + (let [file (-> (thf/sample-file :file1) + (thv/add-variant :v01 :c01 :m01 :c02 :m02) + (thc/instantiate-component :c01 :copy01))] + (t/is (nil? (clv/find-switch-target (libraries file) (ths/get-shape file :copy01) 0 "Value1"))))) + +(t/deftest switch-target-is-nil-when-no-variant-has-the-value + (let [file (-> (thf/sample-file :file1) + (thv/add-variant :v01 :c01 :m01 :c02 :m02) + (thc/instantiate-component :c01 :copy01))] + (t/is (nil? (clv/find-switch-target (libraries file) (ths/get-shape file :copy01) 0 "Missing"))))) + +(t/deftest switch-target-keeps-the-other-properties-when-it-can + ;; From p1v1, p2v1, setting Property 1 to p1v2 can give c02 (p1v2, p2v2) + ;; or c03 (p1v2, p2v1). c03 keeps Property 2, so it is nearer. + (let [file (-> (thf/sample-file :file1) + (thv/add-variant-two-properties :v01 :c01 :m01 :c02 :m02) + (add-third-variant) + (thc/instantiate-component :c01 :copy01)) + target (clv/find-switch-target (libraries file) (ths/get-shape file :copy01) 0 "p1v2")] + (t/is (= (thi/id :c03) (:id target))))) + +(t/deftest switch-into-own-main-is-a-nesting-loop + (let [file (-> (thf/sample-file :file1) + (thv/add-variant :v01 :c01 :m01 :c02 :m02) + (thc/instantiate-component :c01 :nested :parent-label :m02) + (thc/instantiate-component :c01 :copy01)) + objects (:objects (thf/current-page file)) + loop? #(clv/swap-nesting-loop? objects (ths/get-shape file %) (:data file) (thi/id :c02))] + (t/is (true? (loop? :nested))) + (t/is (false? (loop? :copy01))))) + +(t/deftest swap-in-place-keeps-index-and-layout-attrs + (let [file (-> (thf/sample-file :file1) + (thv/add-variant :v01 :c01 :m01 :c02 :m02) + (ths/add-sample-shape :board :type :frame) + (ths/add-sample-shape :first :parent-label :board) + (thc/instantiate-component :c01 :copy01 :parent-label :board) + (ths/add-sample-shape :last :parent-label :board) + (ths/update-shape :copy01 :layout-item-z-index 7)) + page (thf/current-page file) + board (ths/get-shape file :board) + index (d/index-of (:shapes board) (thi/id :copy01)) + [new-shape changes _] + (clv/generate-component-swap-in-place (pcb/empty-changes nil (:id page)) + page + (libraries file) + (:data file) + (ths/get-shape file :copy01) + (thi/id :c02) + false) + file' (thf/apply-changes file changes) + new-shape (ths/get-shape-by-id file' (:id new-shape))] + (t/is (= (thi/id :c02) (:component-id new-shape))) + (t/is (= index (d/index-of (:shapes (ths/get-shape file' :board)) (:id new-shape)))) + (t/is (= 7 (:layout-item-z-index new-shape))))) + +(defn- add-grid-board + "Add a board :board with a one-row, three-column grid layout, holding a shape + :first in the first column and a copy :copy01 of :c01 in the third, so the + second cell is free." + [file] + (let [file (-> file + (ths/add-sample-shape :board + :type :frame + :layout :grid + :layout-grid-dir :row + :layout-grid-rows [] + :layout-grid-columns [] + :layout-grid-cells {}) + (ths/add-sample-shape :first :parent-label :board) + (thc/instantiate-component :c01 :copy01 :parent-label :board)) + objects (:objects (thf/current-page file)) + board (-> (get objects (thi/id :board)) + (ctl/add-grid-row {:type :flex :value 1}) + (ctl/add-grid-column {:type :flex :value 1}) + (ctl/add-grid-column {:type :flex :value 1}) + (ctl/add-grid-column {:type :flex :value 1})) + place (fn [cells column label] + (let [[id _] (d/seek #(= column (:column (val %))) cells)] + (assoc-in cells [id :shapes] [(thi/id label)]))) + cells (-> (:layout-grid-cells board) + (place 1 :first) + (place 3 :copy01))] + (-> file + (ths/update-shape :board :layout-grid-rows (:layout-grid-rows board)) + (ths/update-shape :board :layout-grid-columns (:layout-grid-columns board)) + (ths/update-shape :board :layout-grid-cells cells)))) + +(t/deftest swap-in-place-keeps-the-grid-cell + (let [file (-> (thf/sample-file :file1) + (thv/add-variant :v01 :c01 :m01 :c02 :m02) + (add-grid-board)) + page (thf/current-page file) + cell (ctl/get-cell-by-shape-id (ths/get-shape file :board) (thi/id :copy01)) + [new-shape changes _] + (clv/generate-component-swap-in-place (pcb/empty-changes nil (:id page)) + page + (libraries file) + (:data file) + (ths/get-shape file :copy01) + (thi/id :c02) + false) + file' (thf/apply-changes file changes) + cell' (ctl/get-cell-by-shape-id (ths/get-shape file' :board) (:id new-shape))] + (t/is (= 3 (:column cell))) + (t/is (= (:id cell) (:id cell'))) + (t/is (= [(:id new-shape)] (:shapes cell'))))) diff --git a/common/test/common_tests/runner.cljc b/common/test/common_tests/runner.cljc index 884c9a7356..7a7248e8cd 100644 --- a/common/test/common_tests/runner.cljc +++ b/common/test/common_tests/runner.cljc @@ -77,6 +77,7 @@ [common-tests.logic.text-touched-test] [common-tests.logic.token-apply-test] [common-tests.logic.token-test] + [common-tests.logic.variants-switch-target-test] [common-tests.logic.variants-switch-test] [common-tests.logic.variants-test] [common-tests.math-test] @@ -185,6 +186,7 @@ 'common-tests.logic.token-apply-test 'common-tests.logic.token-test 'common-tests.logic.variants-switch-test + 'common-tests.logic.variants-switch-target-test 'common-tests.logic.variants-test 'common-tests.math-test 'common-tests.media-test diff --git a/frontend/src/app/main/data/workspace/libraries.cljs b/frontend/src/app/main/data/workspace/libraries.cljs index bc52e63736..6205eb3098 100644 --- a/frontend/src/app/main/data/workspace/libraries.cljs +++ b/frontend/src/app/main/data/workspace/libraries.cljs @@ -26,7 +26,6 @@ [app.common.types.container :as ctn] [app.common.types.file :as ctf] [app.common.types.library :as ctl] - [app.common.types.shape.layout :as ctsl] [app.common.types.typography :as ctt] [app.common.uuid :as uuid] [app.config :as cf] @@ -1034,29 +1033,6 @@ (rx/of (dwt.wasm/render-thumbnail file-id page-id root-id :persist? true))) (rx/of (update-component-thumbnail-sync state component-id file-id "component")))))) -(defn- find-shape-index - [objects id shape-id] - (let [object (get objects id)] - (when object - (let [shapes (:shapes object)] - (or (->> shapes - (map-indexed (fn [index shape] [shape index])) - (filter #(= shape-id (first %))) - first - second) - 0))))) - -(defn component-swap-nesting-loop? - [objects shape library-data component-id] - (let [component (ctkl/get-component library-data component-id true) - page (ctf/get-component-page library-data component) - root (ctf/get-component-root library-data component)] - (and page - root - (cfh/components-nesting-loop? - (cfh/get-children-with-self (:objects page) (:id root)) - (cfh/get-parents-with-self objects (:parent-id shape)))))) - (defn component-swap "Swaps a component with another one" [shape file-id id-new-component keep-touched?] @@ -1069,29 +1045,16 @@ ;; in the grid creating new rows/columns to make space (let [libraries (dsh/lookup-libraries state) page (dsh/lookup-page state) - objects (:objects page) - parent (get objects (:parent-id shape)) - ldata (dsh/lookup-file-data state file-id) - orig-shapes (when keep-touched? (cfh/get-children-with-self objects (:id shape))) - - ;; If the target parent is a grid layout we need to pass the target cell - target-cell (when (ctsl/grid-layout? parent) - (ctsl/get-cell-by-shape-id parent (:id shape))) - - index (find-shape-index objects (:parent-id shape) (:id shape)) - - ;; Store the properties that need to be maintained when the component is swapped - keep-props-values (select-keys shape ctk/swap-keep-attrs) undo-id (js/Symbol) undo-group (uuid/next) - [new-shape all-parents changes] + [new-shape changes update-layout-ids] (-> (pcb/empty-changes it (:id page)) (pcb/set-undo-group undo-group) - (cll/generate-component-swap objects shape ldata page libraries id-new-component - index target-cell keep-props-values keep-touched?)) + (clv/generate-component-swap-in-place page libraries ldata shape + id-new-component keep-touched?)) updated-objects (pcb/get-objects changes) new-children-ids (cfh/get-children-ids-with-self updated-objects (:id new-shape)) @@ -1103,11 +1066,7 @@ id)))) (vec)) - [changes parents-of-swapped] - (if keep-touched? - (clv/generate-keep-touched changes new-shape shape orig-shapes page libraries ldata) - [changes []]) - update-layout-ids (concat all-parents parents-of-swapped new-children-ids)] + update-layout-ids (concat update-layout-ids new-children-ids)] (rx/of (dwu/start-undo-transaction undo-id) (dch/commit-changes changes) diff --git a/frontend/src/app/main/data/workspace/variants.cljs b/frontend/src/app/main/data/workspace/variants.cljs index 09f6003a9d..2b0474463f 100644 --- a/frontend/src/app/main/data/workspace/variants.cljs +++ b/frontend/src/app/main/data/workspace/variants.cljs @@ -787,40 +787,23 @@ ptk/WatchEvent (watch [it state _] (let [libraries (dsh/lookup-libraries state) - component-id (:component-id shape) - component (ctf/get-component libraries (:component-file shape) component-id :include-deleted? false)] - ;; If the value is already val, do nothing - (when (not= val (dm/get-in component [:variant-properties pos :value])) - (let [current-page-objects (dsh/lookup-page-objects state) - variant-id (:variant-id component) - component-file-data (dm/get-in libraries [(:component-file shape) :data]) - component-page-objects (-> (dsh/get-page component-file-data (:main-instance-page component)) - (get :objects)) - variant-comps (cfv/find-variant-components component-file-data component-page-objects variant-id) - target-props (-> (:variant-properties component) - (update pos assoc :value val)) - valid-comps (->> variant-comps - (remove #(= (:id %) component-id)) - (filter #(= (dm/get-in % [:variant-properties pos :value]) val)) - (reverse)) - nearest-comp (apply min-key #(ctv/distance target-props (:variant-properties %)) valid-comps) - shape-parents (cfh/get-parents-with-self current-page-objects (:parent-id shape)) - nearest-comp-children (cfh/get-children-with-self component-page-objects (:main-instance-id nearest-comp)) - comps-nesting-loop? (seq? (cfh/components-nesting-loop? nearest-comp-children shape-parents)) + nearest-comp (clv/find-switch-target libraries shape pos val) - {:keys [on-error] - :or {on-error rx/throw}} (meta params)] - - ;; If there is no nearest-comp, do nothing - (when nearest-comp - (if comps-nesting-loop? - (do - (on-error) - (rx/empty)) - (rx/of - (dwl/component-swap shape (:component-file shape) (:id nearest-comp) true) - (ev/event (-> {::ev/name "variant-switch" ::ev/origin "workspace:design-tab"} - (merge (meta it))))))))))))) + {:keys [on-error] + :or {on-error rx/throw}} (meta params)] + ;; Nothing to do when the value is already val or no variant has it + (when nearest-comp + (if (clv/swap-nesting-loop? (dsh/lookup-page-objects state) + shape + (dm/get-in libraries [(:component-file shape) :data]) + (:id nearest-comp)) + (do + (on-error) + (rx/empty)) + (rx/of + (dwl/component-swap shape (:component-file shape) (:id nearest-comp) true) + (ev/event (-> {::ev/name "variant-switch" ::ev/origin "workspace:design-tab"} + (merge (meta it))))))))))) (defn variants-switch "Switch each shape (that must be a variant copy head) for the closest one with the property value passed as parameter" 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 5b4f23d1db..bcaa042e88 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 @@ -10,6 +10,7 @@ [app.common.data :as d] [app.common.data.macros :as dm] [app.common.files.variant :as cfv] + [app.common.logic.variants :as clv] [app.common.path-names :as cpn] [app.common.types.component :as ctk] [app.common.types.components-list :as ctkl] @@ -839,7 +840,7 @@ (let [data (dm/get-in libraries [current-library-id :data]) container (ctf/get-component-page data item) root-shape (ctf/get-component-root data item) - loop? (some #(dwl/component-swap-nesting-loop? + loop? (some #(clv/swap-nesting-loop? objects % data (:id item)) shapes)] [:> component-swap-item* {:key (dm/str (:id item)) diff --git a/frontend/src/app/plugins/shape.cljs b/frontend/src/app/plugins/shape.cljs index 9a86ebd0d3..3e236672a6 100644 --- a/frontend/src/app/plugins/shape.cljs +++ b/frontend/src/app/plugins/shape.cljs @@ -12,6 +12,7 @@ [app.common.geom.rect :as grc] [app.common.geom.shapes :as gsh] [app.common.json :as json] + [app.common.logic.variants :as clv] [app.common.path-names :as cpn] [app.common.record :as crc] [app.common.schema :as sm] @@ -1574,7 +1575,7 @@ (not (ctk/in-component-copy? shape)) (u/not-valid plugin-id :swapComponent "The shape is not a component copy instance") - (dwl/component-swap-nesting-loop? objects shape target-data target-id) + (clv/swap-nesting-loop? objects shape target-data target-id) (u/not-valid plugin-id :swapComponent "The swap would create a component nesting loop") :else