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