From cb8f563bb2abe762f5c8b028974f14b3a05e9135 Mon Sep 17 00:00:00 2001 From: Alonso Torres Date: Thu, 29 Oct 2026 17:30:16 +0100 Subject: [PATCH] :bug: Keep swapped and moved children in place when a component syncs (#12716) * :bug: Keep swapped and moved children in place when a component syncs A sync resets an untouched shape of a copy to the main's geometry and the layout then moves it back. Children the sync keeps, because they are swapped or the user moved them, did not take the first move but did take the second, so each sync pushed them away from their parent by the gap between the copy and the main (e.g. a swapped icon in a copy that fills a wider layout). The sync now moves those children by the offset it applies to their parent. Flex-child attributes still sync as before, so swapped children keep their absolute position. Closes #12657 * :bug: Place swapped children where the swap puts them on sync and switch A swapped child that the user never moved now goes to the top-left of the shape it replaced, as a swap does: on a sync, the place of the main's slot occupant; on a variant switch, the place of the new variant's occupant. A swapped child the user moved keeps its offset inside its parent. This stops the variant switch from leaving the swapped child at its old position while the new variant is created at the main's size, and puts back children that drifted before the sync fix the next time their copy syncs. * :bug: Make kept children follow their parent's constraints on component sync When a sync resets the size of a copy's shape, for example a copy root that fills a wider layout, the layout then grows it back and moves its children by their constraints. Children the sync keeps (moved by the user, or swapped) only followed the parent's top-left, so with a right, center or scale constraint they drifted on every sync, and a swapped child with a scale constraint grew on every sync. The sync now passes the parent's geometry before and after down the tree, and kept children follow that change through their constraints. Untouched swapped children also undo the change through their constraints before they go to the swap place, so they keep their size. The default constraint modifier returns empty modifiers instead of an empty vector, which add-modifiers could not take. * :bug: Make swapped children follow their parent's constraints on switch A variant switch only moved a swapped child by the parent's position change. When the new variant's root had a different size, a user-moved child with a right constraint drifted, and a scale-constrained one kept its grown size. The switch now applies the parent change from the old parent to the new one through the child's constraints. An untouched swapped child then takes the place of the shape it replaces, as a swap does. * :bug: Sync nested swapped copies before the copies that hold them A swapped child is a copy of its own component, so one sync handles it twice: once on its own, and once when the copy that holds it places it. Each step read the shape from the data as it was before the sync, so the step that ran last won, and the order came from the order of shape ids. When the swapped child came last, it wrote its old size and position back over the placement, leaving its x and selrect apart, and a swapped child with a scale constraint grew on every sync. The sync now visits the most deeply nested shapes first, and each step reads the shapes as the earlier steps left them, so the placement always comes last and builds on the child's own sync. * :bug: Keep moved children with a scale constraint at their size on switch A variant switch carries the size of a child the user moved as an override. In a copy that fills a wider layout, a child with a scale constraint already has the size the layout gave it, so the switch carried that size onto the new variant, created at the main's size, and the layout then scaled it again: the child grew on every switch. Before comparing, the switch now resizes the previous child the way its constraints follow the change from its old parent to its new one. A child that only grew with the layout then matches the main and is not carried as an override. * :bug: Keep the content of swapped children at its size on sync and switch When a sync or a variant switch undid the parent change on a swapped child through its constraints, it applied the child's resize to every shape inside it. The layout then grows the child back through its own constraint, but its content follows its own constraints (left and top by default), so it stayed shrunk: the shape inside a swapped icon with a scale constraint lost most of its width on every switch. The change of the swapped child now goes down its subtree the way a layout update does, each shape through its own constraints. * :bug: Accept mixed and empty values in the layout size constraints menu With several shapes or a group selected, the sidebar merges the layout item attributes and tokens of the shapes: nil when a shape lacks one, :multiple when they differ. The props schema of layout-size-constraints* only accepted the values of a single shape, so selecting such shapes with fill sizing raised "invalid props". The schema now accepts nil and :multiple, as the numeric inputs those values go to do. * :recycle: Share the swapped child follow-parent step between sync and switch The component sync and the variant switch both make a swapped child and its children follow the parent change through their constraints and then move them to the swap place. Both now use generate-follow-parent, so the two paths stay the same. The helper that writes geometry without marking it touched is renamed to update-geometry-untouched. Adds a test for a swapped child without absolute position in a slot without layout, and fixes the test comments on why constraints are set again after a swap. --- .../app/common/geom/shapes/constraints.cljc | 2 +- common/src/app/common/logic/libraries.cljc | 135 +++++++++- common/src/app/common/logic/variants.cljc | 22 +- .../app/common/test_helpers/compositions.cljc | 28 ++ .../geom_shapes_constraints_test.cljc | 18 +- .../logic/comp_flex_interactions_test.cljc | 246 ++++++++++++++++-- .../logic/variants_switch_test.cljc | 144 ++++++++++ .../sidebar/options/menus/layout_item.cljs | 33 ++- 8 files changed, 563 insertions(+), 65 deletions(-) diff --git a/common/src/app/common/geom/shapes/constraints.cljc b/common/src/app/common/geom/shapes/constraints.cljc index ca92c93cc7..325a6d3069 100644 --- a/common/src/app/common/geom/shapes/constraints.cljc +++ b/common/src/app/common/geom/shapes/constraints.cljc @@ -235,7 +235,7 @@ (ctm/move-modifiers (displacement center-before center-after before-side-vector after-side-vector)))) (defmethod constraint-modifier :default [_ _ _ _ _ _] - []) + (ctm/empty)) (def const->type+axis {:left :start diff --git a/common/src/app/common/logic/libraries.cljc b/common/src/app/common/logic/libraries.cljc index ac5a516156..3a87e923c4 100644 --- a/common/src/app/common/logic/libraries.cljc +++ b/common/src/app/common/logic/libraries.cljc @@ -13,6 +13,7 @@ [app.common.files.helpers :as cfh] [app.common.files.variant :as cfv] [app.common.geom.matrix :as gmt] + [app.common.geom.modifiers :as gm] [app.common.geom.point :as gpt] [app.common.geom.rect :as grc] [app.common.geom.shapes :as gsh] @@ -27,6 +28,7 @@ [app.common.types.container :as ctn] [app.common.types.file :as ctf] [app.common.types.library :as ctl] + [app.common.types.modifiers :as ctm] [app.common.types.page :as ctp] [app.common.types.pages-list :as ctpl] [app.common.types.path.segment :as segment] @@ -577,15 +579,19 @@ (container-log :debug (:id container) :msg "Sync page in local file" :page-id (:id container)) (container-log :debug (:id container) :msg "Sync component in local library" :component-id (:id container))) - (let [linked-shapes (->> (vals (:objects container)) - (filter #(uses-assets? asset-type asset-id % library-id)))] + (let [objects (:objects container) + ;; Deepest first, so a copy places its swapped children after their own sync + linked-shapes (->> (vals objects) + (filter #(uses-assets? asset-type asset-id % library-id)) + (sort-by #(- (count (cfh/get-parent-ids objects (:id %))))))] (loop [shapes (seq linked-shapes) changes (-> changes (pcb/with-container container) - (pcb/with-objects (:objects container)))] + (pcb/with-objects objects))] (if-let [shape (first shapes)] (recur (next shapes) - (let [objects' (pcb/get-objects changes) + (let [changes (pcb/apply-changes-local changes) + objects' (pcb/get-objects changes) shape' (get objects' (:id shape))] ;; The shape could have been deleted in previous changes, if this nested component ;; comes from components-v1 era or if there has been some error with the swap slot. @@ -594,7 +600,7 @@ (generate-sync-shape asset-type changes library-id - container + (assoc container :objects objects') shape' libraries current-file-id) @@ -832,7 +838,8 @@ root-inst root-main reset? - initial-root?) + initial-root? + nil) ;; If the component is not found, because the master component has been ;; deleted or the library unlinked, do nothing. @@ -866,8 +873,88 @@ (recur shape-inst' next-data component') nil)))))) +(defn- geometry-sync-delta + "Offset the sync applies to the top-left of `dest-shape` when it copies the + geometry of `origin-shape`." + [dest-shape origin-shape dest-root origin-root] + (gpt/subtract (grc/rect->point (:selrect (reposition-shape origin-shape origin-root dest-root))) + (grc/rect->point (:selrect dest-shape)))) + +(defn- follow-parent-modifiers + "Modifiers that make `child` follow the change of its parent from + `parent-before` to `parent-after`, through the child constraints." + [child parent-before parent-after] + (let [sr-before (:selrect parent-before) + sr-after (:selrect parent-after) + modifiers (-> (ctm/empty) + (ctm/resize (gpt/point (/ (:width sr-after) (max 0.01 (:width sr-before))) + (/ (:height sr-after) (max 0.01 (:height sr-before)))) + (first (:points parent-before)) + (:transform parent-before) + (:transform-inverse parent-before)) + (ctm/move (gpt/subtract (first (:points parent-after)) + (first (:points parent-before)))))] + ;; A move doesn't need the constraints + (if (or (ctm/empty? modifiers) (ctm/only-move? modifiers)) + modifiers + (gsh/calc-child-modifiers parent-before child modifiers false + (:points child) + (:points parent-before) + (delay (:points parent-after)))))) + +(defn- follow-parent-modif-tree + "Modifiers that make `child` and its children follow the change of the + parent of `child` from `parent-before` to `parent-after`, each one through + its own constraints." + [objects child parent-before parent-after] + (let [modifiers (follow-parent-modifiers child parent-before parent-after)] + (if (ctm/empty? modifiers) + {} + (gm/set-objects-modifiers {(:id child) {:modifiers modifiers}} + (select-keys objects (cfh/get-children-ids-with-self objects (:id child))))))) + +(defn- resize-with-parent + "Resizes `shape` the way its constraints do when its parent changes from + `parent-before` to `parent-after`, keeping its top-left." + [shape parent-before parent-after] + (let [modifiers (follow-parent-modifiers shape parent-before parent-after)] + (if (ctm/empty? modifiers) + shape + (let [shape' (gsh/transform-shape shape modifiers)] + (gsh/move shape' (gpt/subtract (grc/rect->point (:selrect shape)) + (grc/rect->point (:selrect shape')))))))) + +(defn- update-geometry-untouched + "Applies `update-fn` to the shapes without marking them touched." + [changes ids update-fn container] + (pcb/concat-changes + changes + (-> (pcb/empty-changes) + (pcb/with-container container) + (pcb/with-objects (:objects container)) + (pcb/update-shapes ids update-fn {:ignore-touched true})))) + +(defn generate-follow-parent + "Makes `child` and its children follow the change of the parent of `child` + from `parent-before` to `parent-after`, each one through its own + constraints, without marking them touched. `delta-fn`, if given, receives + `child` after that and returns the offset to move the subtree by." + [changes container child parent-before parent-after delta-fn] + (let [objects (:objects container) + modif-tree (follow-parent-modif-tree objects child parent-before parent-after) + follow #(gsh/transform-shape % (dm/get-in modif-tree [(:id %) :modifiers])) + delta (if delta-fn (delta-fn (follow child)) (gpt/point 0 0))] + (cond-> changes + (or (seq modif-tree) (not (gpt/almost-zero? delta))) + (update-geometry-untouched (cfh/get-children-ids-with-self objects (:id child)) + #(gsh/move (follow %) delta) + container)))) + (defn- generate-sync-shape-direct-recursive - [changes container shape-inst component library file libraries shape-main root-inst root-main reset? initial-root?] + "Syncs `shape-inst` and its children from `shape-main`. `parent-change` is + the geometry of the parent of `shape-inst` before and after the sync, or + nil." + [changes container shape-inst component library file libraries shape-main root-inst root-main reset? initial-root? parent-change] (shape-log :debug (:id shape-inst) container :msg "Sync shape direct recursive" :shape-inst (str (:name shape-inst) " " (pretty-uuid (:id shape-inst))) @@ -879,6 +966,19 @@ (let [omit-touched? (not reset?) clear-remote-synced? (and initial-root? reset?) set-remote-synced? (and (not initial-root?) reset?) + geometry-kept? (and omit-touched? + (ctk/touched-group? shape-inst :geometry-group)) + main-geometry (reposition-shape shape-main root-main root-inst) + follow-modifiers (when (and geometry-kept? (some? parent-change)) + (let [[parent-before parent-after] parent-change] + (follow-parent-modifiers shape-inst parent-before parent-after))) + shape-after (cond + (not geometry-kept?) main-geometry + (some? follow-modifiers) (gsh/transform-shape shape-inst follow-modifiers) + :else shape-inst) + ;; Offset between where the sync leaves shape-inst and where the main puts it + main-gap (gpt/subtract (grc/rect->point (:selrect shape-after)) + (grc/rect->point (:selrect main-geometry))) changes (cond-> changes :always @@ -889,6 +989,11 @@ container omit-touched?) + (and (some? follow-modifiers) (not (ctm/empty? follow-modifiers))) + (update-geometry-untouched [(:id shape-inst)] + #(gsh/transform-shape % follow-modifiers) + container) + (ctsl/flex-layout? shape-main) (update-flex-child-copy-attrs shape-main shape-inst @@ -962,15 +1067,20 @@ root-inst root-main reset? - initial-root?)) + initial-root? + [shape-inst shape-after])) swapped (fn [changes child-inst child-main] (shape-log :trace (:id child-inst) container :msg "Match slot" :child-inst (str (:name child-inst) " " (pretty-uuid (:id child-inst))) :child-main (str (:name child-main) " " (pretty-uuid (:id child-main)))) - ;; For now we don't make any sync here. - changes) + ;; A swapped child follows its parent and, if the user didn't move it, + ;; goes to the place of the slot's occupant in the main + (generate-follow-parent changes container child-inst shape-inst shape-after + (when-not (ctk/touched-group? child-inst :geometry-group) + #(gpt/add (geometry-sync-delta % child-main root-inst root-main) + main-gap)))) moved (fn [changes child-inst child-main] (shape-log :trace (:id child-inst) container @@ -2192,7 +2302,10 @@ [changes current-shape previous-shape current-root prev-root origin-ref-shape container] (let [;; We need to sync only the position relative to the origin of the component. ;; (see update-attrs for a full explanation) - previous-shape (reposition-shape previous-shape prev-root current-root) + previous-shape (-> previous-shape + (resize-with-parent (get (:objects container) (:parent-id previous-shape)) + (get (pcb/get-objects changes) (:parent-id current-shape))) + (reposition-shape prev-root current-root)) touched (get previous-shape :touched #{}) text-auto? (and (cfh/text-shape? current-shape) (contains? #{:auto-height :auto-width} (:grow-type current-shape)))] diff --git a/common/src/app/common/logic/variants.cljc b/common/src/app/common/logic/variants.cljc index 883a3a40da..79de230b5a 100644 --- a/common/src/app/common/logic/variants.cljc +++ b/common/src/app/common/logic/variants.cljc @@ -4,6 +4,8 @@ [app.common.files.changes-builder :as pcb] [app.common.files.helpers :as cfh] [app.common.files.variant :as cfv] + [app.common.geom.point :as gpt] + [app.common.geom.rect :as grc] [app.common.logic.libraries :as cll] [app.common.logic.shapes :as cls] [app.common.logic.variant-properties :as clvp] @@ -72,7 +74,10 @@ copy that was swapped (orig-swapped-child), and its related shape on the new copy (related-shape-in-new), move the orig-swapped-child into the parent of related-shape-in-new, fix its swap-slot if needed, and then delete - related-shape-in-new" + related-shape-in-new. + + orig-swapped-child follows its parent through its constraints and, if the + user didn't move it, takes the place of related-shape-in-new." [changes related-shape-in-new orig-swapped-child ldata page swap-ref-id] (let [;; Before to the swap, temporary move the previous ;; shape to the root panel to avoid problems when @@ -83,15 +88,28 @@ (pcb/change-parent uuid/zero [orig-swapped-child] 0 {:allow-altering-copies true})) objects (pcb/get-objects changes) + page-objects (:objects page) prev-swap-slot (ctk/get-swap-slot orig-swapped-child) current-parent (get objects (:parent-id related-shape-in-new)) - pos (d/index-of (:shapes current-parent) (:id related-shape-in-new))] + pos (d/index-of (:shapes current-parent) (:id related-shape-in-new)) + + orig-child (get page-objects (:id orig-swapped-child)) + ;; If the user didn't move it, it goes to the place of related-shape-in-new + delta-fn (when-not (ctk/touched-group? orig-child :geometry-group) + #(gpt/subtract (grc/rect->point (:selrect related-shape-in-new)) + (grc/rect->point (:selrect %))))] (-> (pcb/concat-changes before-changes changes) ;; Move the previous shape to the new parent (pcb/change-parent (:parent-id related-shape-in-new) [orig-swapped-child] pos {:allow-altering-copies true}) + (cll/generate-follow-parent (ctn/make-container page :page) + orig-child + (get page-objects (:parent-id orig-child)) + current-parent + delta-fn) + ;; We need to update the swap slot only when it pointed ;; to the swap-ref-id. Oterwise this is a swapped item ;; inside a nested copy, so we need to keep it. diff --git a/common/src/app/common/test_helpers/compositions.cljc b/common/src/app/common/test_helpers/compositions.cljc index a4fa6b748b..e2efb1a170 100644 --- a/common/src/app/common/test_helpers/compositions.cljc +++ b/common/src/app/common/test_helpers/compositions.cljc @@ -10,6 +10,7 @@ [app.common.files.changes-builder :as pcb] [app.common.files.helpers :as cfh] [app.common.geom.point :as gpt] + [app.common.geom.shapes :as gsh] [app.common.logic.libraries :as cll] [app.common.logic.shapes :as cls] [app.common.logic.variants :as clv] @@ -443,6 +444,33 @@ :propagate-fn propagate-fn :library library))) +(defn change-shape + "Applies `update-fn` to the shape identified by shape-tag. With + `ignore-touched` the change is not an override, as when a layout changes it." + [file shape-tag update-fn & {:keys [ignore-touched]}] + (let [page (thf/current-page file)] + (thf/apply-changes + file + (cls/generate-update-shapes (pcb/empty-changes nil (:id page)) + #{(thi/id shape-tag)} + update-fn + (:objects page) + {:ignore-touched ignore-touched})))) + +(defn move-shape-tree + "Moves the shape identified by shape-tag and its children by `delta`. With + `ignore-touched` the move is not an override, as when a layout moves it." + [file shape-tag delta & {:keys [ignore-touched]}] + (let [page (thf/current-page file) + objects (:objects page)] + (thf/apply-changes + file + (cls/generate-update-shapes (pcb/empty-changes nil (:id page)) + (cfh/get-children-ids-with-self objects (thi/id shape-tag)) + #(gsh/move % delta) + objects + {:ignore-touched ignore-touched})))) + (defn update-color "Update the first fill color for the shape identified by shape-tag" [file shape-tag color & {:keys [page-label propagate-fn]}] diff --git a/common/test/common_tests/geom_shapes_constraints_test.cljc b/common/test/common_tests/geom_shapes_constraints_test.cljc index 4f6cd3ce4c..081d32325e 100644 --- a/common/test/common_tests/geom_shapes_constraints_test.cljc +++ b/common/test/common_tests/geom_shapes_constraints_test.cljc @@ -7,21 +7,11 @@ (ns common-tests.geom-shapes-constraints-test (:require [app.common.geom.shapes.constraints :as gsc] + [app.common.types.modifiers :as ctm] [clojure.test :as t])) ;; ---- constraint-modifier :default ---- -(t/deftest constraint-modifier-default-returns-empty-vector - (t/testing ":default method accepts 6 args and returns an empty vector" - ;; Before the fix the :default method only accepted 5 positional args - ;; (plus the dispatch value), so calling it with 6 args would throw an - ;; arity error. After the fix it takes [_ _ _ _ _ _] and returns []. - (let [result (gsc/constraint-modifier :unknown-constraint-type - :x nil nil nil nil)] - (t/is (vector? result)) - (t/is (empty? result)))) - - (t/testing ":default method returns [] for :scale-like unknown type on :y axis" - (let [result (gsc/constraint-modifier :some-other-unknown - :y nil nil nil nil)] - (t/is (= [] result))))) +(t/deftest constraint-modifier-default-returns-empty-modifiers + (t/is (= (ctm/empty) + (gsc/constraint-modifier :unknown-constraint-type :x nil nil nil nil)))) diff --git a/common/test/common_tests/logic/comp_flex_interactions_test.cljc b/common/test/common_tests/logic/comp_flex_interactions_test.cljc index 0fa3644c6f..b3a339bc51 100644 --- a/common/test/common_tests/logic/comp_flex_interactions_test.cljc +++ b/common/test/common_tests/logic/comp_flex_interactions_test.cljc @@ -10,7 +10,6 @@ [app.common.geom.point :as gpt] [app.common.geom.shapes :as gsh] [app.common.logic.libraries :as cll] - [app.common.logic.shapes :as cls] [app.common.math :as mth] [app.common.test-helpers.components :as thc] [app.common.test-helpers.compositions :as tho] @@ -19,6 +18,7 @@ [app.common.test-helpers.shapes :as ths] [app.common.types.component :as ctk] [app.common.types.container :as ctn] + [app.common.types.modifiers :as ctm] [clojure.test :as t])) (t/use-fixtures :each thi/test-fixture) @@ -39,17 +39,6 @@ (ths/add-interaction :main-child :main-popup) (ths/add-interaction :copy-child :copy-popup))) -(defn- update-shape - [file label update-fn] - (let [page (thf/current-page file)] - (thf/apply-changes - file - (cls/generate-update-shapes (pcb/empty-changes nil (:id page)) - #{(thi/id label)} - update-fn - (:objects page) - {})))) - (defn- destinations [shape] (mapv :destination (:interactions shape))) @@ -60,8 +49,8 @@ margin {:m1 5 :m2 5 :m3 5 :m4 5} ;; ==== Action - updated-file (update-shape file :main-child - #(assoc % :hidden true :layout-item-margin margin)) + updated-file (tho/change-shape file :main-child + #(assoc % :hidden true :layout-item-margin margin)) changes (cll/generate-sync-file-changes (pcb/empty-changes) nil :components @@ -85,7 +74,7 @@ file (setup-file) ;; ==== Action - updated-file (update-shape file :copy-child #(assoc % :hidden true)) + updated-file (tho/change-shape file :copy-child #(assoc % :hidden true)) page (thf/current-page updated-file) container (ctn/make-container page :page) changes (-> (pcb/empty-changes) @@ -144,7 +133,7 @@ ;; ==== Action library' (-> library - (update-shape :main1-child #(gsh/move % (gpt/point 1 0))) + (tho/change-shape :main1-child #(gsh/move % (gpt/point 1 0))) (tho/propagate-component-changes :component1)) file' (thf/apply-changes file (sync-file-from-library file library')) @@ -190,7 +179,7 @@ :layout :flex :layout-flex-dir :row}) (thc/instantiate-component :icon-a :nested-head {:parent-label :board-root}) - (update-shape :nested-head #(assoc % :layout-item-absolute true)) + (tho/change-shape :nested-head #(assoc % :layout-item-absolute true)) (thc/make-component :board :board-root))) (t/deftest test-sync-library-keeps-swapped-nested-absolute @@ -205,8 +194,8 @@ swapped (ths/get-shape file :copy-nested-head) ;; ==== Action - library' (update-shape library :board-root - #(assoc % :fills (ths/sample-fills-color :fill-color "#fabada"))) + library' (tho/change-shape library :board-root + #(assoc % :fills (ths/sample-fills-color :fill-color "#fabada"))) file' (thf/apply-changes file (sync-file-from-library file library')) swapped' (ths/get-shape file' :copy-nested-head)] @@ -229,8 +218,8 @@ file (swap-to-library-component file library :copy-nested-head :icon-b) ;; ==== Action - library' (update-shape library :nested-head - #(assoc % :layout-item-absolute false)) + library' (tho/change-shape library :nested-head + #(assoc % :layout-item-absolute false)) file' (thf/apply-changes file (sync-file-from-library file library')) swapped' (ths/get-shape file' :copy-nested-head)] @@ -250,7 +239,7 @@ :layout-flex-dir :row}) (thc/instantiate-component :icon-a :nested-head {:parent-label :board-root}) (ths/add-sample-shape :sibling :type :rect :name "Sibling" :parent-label :board-root) - (update-shape :nested-head #(assoc % :layout-item-absolute true)) + (tho/change-shape :nested-head #(assoc % :layout-item-absolute true)) (thc/make-component :board :board-root)) file (-> (thf/sample-file :file) (thc/instantiate-component :board @@ -260,8 +249,8 @@ file (swap-to-library-component file library :copy-nested-head :icon-b) ;; ==== Action - library' (update-shape library :sibling - #(assoc % :layout-item-margin margin)) + library' (tho/change-shape library :sibling + #(assoc % :layout-item-margin margin)) file' (thf/apply-changes file (sync-file-from-library file library')) sibling' (ths/get-shape file' :copy-sibling) swapped' (ths/get-shape file' :copy-nested-head)] @@ -270,3 +259,212 @@ (t/is (= margin (:layout-item-margin sibling'))) (t/is (true? (:layout-item-absolute swapped'))) (t/is (some? (ctk/get-swap-slot swapped'))))) + +(defn- setup-library-with-icon-slot + "Library: icon A, icon B, and a flex board whose `:slot` frame holds an + instance of icon A. The slot has a flex layout and the icon is absolute; + with `:layout? false` the slot has no layout and the icon is a plain child." + [& {:keys [layout?] :or {layout? true}}] + (-> (thf/sample-file :library) + (tho/add-simple-component :icon-a :icon-a-root :icon-a-child) + (tho/add-simple-component :icon-b :icon-b-root :icon-b-child) + (tho/add-frame :board-root {:name "Board" + :layout :flex + :layout-flex-dir :row}) + (tho/add-frame :slot (cond-> {:name "Slot" :parent-label :board-root} + layout? (assoc :layout :flex :layout-flex-dir :row))) + (thc/instantiate-component :icon-a :nested-head {:parent-label :slot}) + (cond-> layout? (tho/change-shape :nested-head #(assoc % :layout-item-absolute true))) + (thc/make-component :board :board-root))) + +(defn- instantiate-board + "File with a copy of the library board, labelling its slot and nested icon." + [library] + (let [file (-> (thf/sample-file :file) + (thc/instantiate-component :board + :copy-board + :library library + :children-labels [:copy-slot])) + slot (ths/get-shape file :copy-slot)] + (thi/set-id! :copy-nested-head (first (:shapes slot))) + file)) + +(defn- offset-in-slot + [file slot-label icon-label] + (let [slot (ths/get-shape file slot-label) + icon (ths/get-shape file icon-label)] + (gpt/point (- (:x icon) (:x slot)) + (- (:y icon) (:y slot))))) + +(defn- sync-after-unrelated-change + [file library root-label] + (let [library' (tho/change-shape library root-label + #(assoc % :fills (ths/sample-fills-color :fill-color "#fabada")))] + (thf/apply-changes file (sync-file-from-library file library')))) + +(t/deftest test-sync-places-swapped-child-in-moved-slot + (let [;; ==== Setup + library (setup-library-with-icon-slot) + file (-> (instantiate-board library) + (swap-to-library-component library :copy-nested-head :icon-b) + ;; The layout moves the slot, and a previous sync moved the icon away + (tho/move-shape-tree :copy-slot (gpt/point 800 0) :ignore-touched true) + (tho/move-shape-tree :copy-nested-head (gpt/point 800 0) :ignore-touched true)) + + ;; ==== Action + file' (sync-after-unrelated-change file library :board-root)] + + ;; ==== Check + (t/is (gpt/close? (offset-in-slot library :slot :nested-head) + (offset-in-slot file' :copy-slot :copy-nested-head))) + (t/is (true? (:layout-item-absolute (ths/get-shape file' :copy-nested-head)))))) + +(t/deftest test-sync-places-swapped-child-in-moved-slot-without-layout + (let [;; ==== Setup + library (setup-library-with-icon-slot :layout? false) + file (-> (instantiate-board library) + (swap-to-library-component library :copy-nested-head :icon-b) + ;; The layout moves the slot, and a previous sync moved the icon away + (tho/move-shape-tree :copy-slot (gpt/point 800 0) :ignore-touched true) + (tho/move-shape-tree :copy-nested-head (gpt/point 800 0) :ignore-touched true)) + + ;; ==== Action + file' (sync-after-unrelated-change file library :board-root)] + + ;; ==== Check + (t/is (gpt/close? (offset-in-slot library :slot :nested-head) + (offset-in-slot file' :copy-slot :copy-nested-head))))) + +(t/deftest test-sync-keeps-moved-child-in-moved-slot + (let [;; ==== Setup + library (setup-library-with-icon-slot) + file (-> (instantiate-board library) + (tho/move-shape-tree :copy-nested-head (gpt/point 5 0)) + (tho/move-shape-tree :copy-slot (gpt/point 800 0) :ignore-touched true)) + before (offset-in-slot file :copy-slot :copy-nested-head) + + ;; ==== Action + file' (sync-after-unrelated-change file library :board-root)] + + ;; ==== Check + (t/is (gpt/close? before (offset-in-slot file' :copy-slot :copy-nested-head))))) + +(t/deftest test-sync-keeps-user-moved-swapped-child-in-moved-slot + (let [;; ==== Setup + library (setup-library-with-icon-slot) + file (-> (instantiate-board library) + (swap-to-library-component library :copy-nested-head :icon-b) + (tho/move-shape-tree :copy-nested-head (gpt/point 5 0)) + (tho/move-shape-tree :copy-slot (gpt/point 800 0) :ignore-touched true)) + before (offset-in-slot file :copy-slot :copy-nested-head) + + ;; ==== Action + file' (sync-after-unrelated-change file library :board-root)] + + ;; ==== Check + (t/is (gpt/close? before (offset-in-slot file' :copy-slot :copy-nested-head))))) + +(defn- with-shapes-first + "Puts the shapes with the given labels first in the page objects, in that + order. The sync visits the shapes of a page in this order." + [file labels] + (let [page-id (:id (thf/current-page file)) + ids (mapv thi/id labels)] + (update-in file [:data :pages-index page-id :objects] + (fn [objects] + (apply array-map (concat (mapcat #(vector % (get objects %)) ids) + (mapcat identity (apply dissoc objects ids)))))))) + +(t/deftest test-sync-places-swapped-child-where-main-slot-occupant-is + (let [;; ==== Setup + library (setup-library-with-icon-slot) + file (-> (instantiate-board library) + (swap-to-library-component library :copy-nested-head :icon-b) + ;; The copy comes before its swapped icon, which the sync also syncs on its own + (with-shapes-first [:copy-board :copy-nested-head])) + + ;; ==== Action + library' (-> library + (tho/move-shape-tree :nested-head (gpt/point 4 4)) + (tho/change-shape :icon-b-root #(gsh/transform-shape % (ctm/change-size % 32 nil)))) + file' (thf/apply-changes file (sync-file-from-library file library')) + + ;; ==== Get + icon' (ths/get-shape file' :copy-nested-head)] + + ;; ==== Check + (t/is (gpt/close? (offset-in-slot library' :slot :nested-head) + (offset-in-slot file' :copy-slot :copy-nested-head))) + (t/is (mth/close? (:width (ths/get-shape library' :icon-b-root)) (:width icon'))) + (t/is (mth/close? (:x icon') (-> icon' :selrect :x))))) + +(defn- setup-library-with-root-icon + "Library: icon A, and a 300px wide flex card whose `:close` child, placed + directly in the root, is an absolute instance of icon A with the given + horizontal constraint." + [constraints-h] + (-> (thf/sample-file :library) + (tho/add-simple-component :icon-a :icon-a-root :icon-a-child + :root-params {:width 24 :height 24} + :child-params {:width 24 :height 24}) + (tho/add-simple-component :icon-b :icon-b-root :icon-b-child + :root-params {:width 24 :height 24} + :child-params {:width 24 :height 24}) + (tho/add-frame :card-root {:name "Card" + :layout :flex + :layout-flex-dir :row + :width 300 + :height 48}) + (thc/instantiate-component :icon-a :close {:parent-label :card-root}) + (tho/change-shape :close #(assoc % :layout-item-absolute true :constraints-h constraints-h)) + (thc/make-component :card :card-root))) + +(defn- instantiate-filled-card + "File with a copy of the card, widened by 800px and with its close icon + moved by `icon-shift`, the way the layout does when the copy fills a wider + parent." + [library icon-shift] + (-> (thf/sample-file :file) + (thc/instantiate-component :card + :copy-card + :library library + :children-labels [:copy-close]) + (tho/change-shape :copy-card + #(gsh/transform-shape % (ctm/change-size % (+ (:width %) 800) nil)) + :ignore-touched true) + (tho/move-shape-tree :copy-close (gpt/point icon-shift 0) :ignore-touched true))) + +(t/deftest test-sync-moved-root-child-follows-root-through-right-constraint + (let [;; ==== Setup + library (setup-library-with-root-icon :right) + file (-> (instantiate-filled-card library 800) + (tho/move-shape-tree :copy-close (gpt/point 5 0))) + + ;; ==== Action + file' (sync-after-unrelated-change file library :card-root)] + + ;; ==== Check + (t/is (gpt/close? (gpt/add (offset-in-slot library :card-root :close) (gpt/point 5 0)) + (offset-in-slot file' :copy-card :copy-close))))) + +(t/deftest test-sync-scales-swapped-root-child-back-with-root + (let [;; ==== Setup + library (setup-library-with-root-icon :scale) + file (-> (instantiate-filled-card library 0) + (swap-to-library-component library :copy-close :icon-b) + ;; The swap drops the constraints; set them again so this path runs + (tho/change-shape :copy-close #(assoc % :constraints-h :scale))) + + ;; ==== Action + file' (sync-after-unrelated-change file library :card-root) + + ;; ==== Get + scale (/ (:width (ths/get-shape file' :copy-card)) + (:width (ths/get-shape file :copy-card))) + inner #(ths/get-shape-by-id % (first (:shapes (ths/get-shape % :copy-close))))] + + ;; ==== Check + (t/is (mth/close? (* scale (:width (ths/get-shape file :copy-close))) + (:width (ths/get-shape file' :copy-close)))) + ;; The icon content follows the icon through its own (left) constraint + (t/is (mth/close? (:width (inner file)) (:width (inner file')))))) diff --git a/common/test/common_tests/logic/variants_switch_test.cljc b/common/test/common_tests/logic/variants_switch_test.cljc index 06e8b5ac34..acaa4faf06 100644 --- a/common/test/common_tests/logic/variants_switch_test.cljc +++ b/common/test/common_tests/logic/variants_switch_test.cljc @@ -12,6 +12,7 @@ [app.common.geom.shapes :as gsh] [app.common.logic.libraries :as cll] [app.common.logic.shapes :as cls] + [app.common.math :as mth] [app.common.test-helpers.components :as thc] [app.common.test-helpers.compositions :as tho] [app.common.test-helpers.files :as thf] @@ -19,6 +20,7 @@ [app.common.test-helpers.shapes :as ths] [app.common.test-helpers.variants :as thv] [app.common.types.component :as ctk] + [app.common.types.modifiers :as ctm] [clojure.test :as t])) (t/use-fixtures :each thi/test-fixture) @@ -3181,3 +3183,145 @@ (t/is (= (get-in synced-t font-size-path-0) "30")) (t/is (contains? (:touched synced-t) :content-group)) (t/is (contains? (:touched synced-t) :text-content-text)))))) + +;; ============================================================ +;; SWAPPED CHILD POSITION +;; ============================================================ + +(defn- setup-variant-with-swapped-icon + "Variants whose roots hold an icon A copy, placed 8px lower in the second + variant, and a copy of the first variant with its icon swapped to icon B." + [] + (-> (thf/sample-file :file1) + (tho/add-simple-component :icon-a :icon-a-root :icon-a-child) + (tho/add-simple-component :icon-b :icon-b-root :icon-b-child) + (thv/add-variant-with-copy :v01 :c01 :m01 :c02 :m02 :m01-icon :m02-icon :icon-a) + (tho/move-shape-tree :m02-icon (gpt/point 0 8)) + (thc/instantiate-component :c01 :copy01 :children-labels [:copy-icon]) + (thc/component-swap :copy-icon :icon-b :copy-icon-b))) + +(defn- offset-in-root + [file root-label child-label] + (let [root (ths/get-shape file root-label) + child (ths/get-shape file child-label)] + (gpt/point (- (:x child) (:x root)) + (- (:y child) (:y root))))) + +(t/deftest test-switch-places-swapped-child-where-new-variant-occupant-is + (let [;; ==== Setup + file (setup-variant-with-swapped-icon) + + ;; ==== Action + file' (tho/swap-component-in-shape file :copy01 :c02 {:new-shape-label :copy02 + :keep-touched? true})] + + ;; ==== Check + (t/is (gpt/close? (offset-in-root file :m02 :m02-icon) + (offset-in-root file' :copy02 :copy-icon-b))))) + +(defn- resize-width + [shape width] + (gsh/transform-shape shape (ctm/change-size shape width nil))) + +(defn- setup-card-copy + "Variants of a 300x48 flex card whose `:m01-icon` / `:m02-icon` is an + absolute icon A copy placed directly in the root at x 270, with the given + horizontal constraint. A copy of the first variant fills 800px more, the + way the layout grows it." + [constraints-h] + (let [place-icon (fn [file root-label icon-label] + (let [root (ths/get-shape file root-label) + icon (ths/get-shape file icon-label)] + (-> file + (tho/move-shape-tree icon-label (gpt/point (- (+ (:x root) 270) (:x icon)) + (- (+ (:y root) 12) (:y icon)))) + (tho/change-shape icon-label #(assoc % :layout-item-absolute true + :constraints-h constraints-h))))) + card (fn [file root-label] + (tho/change-shape file root-label #(-> % + (resize-width 300) + (gsh/transform-shape (ctm/change-size % nil 48)) + (assoc :layout :flex :layout-flex-dir :row))))] + (-> (thf/sample-file :file1) + (tho/add-simple-component :icon-a :icon-a-root :icon-a-child + :root-params {:width 24 :height 24} + :child-params {:width 24 :height 24}) + (tho/add-simple-component :icon-b :icon-b-root :icon-b-child + :root-params {:width 24 :height 24} + :child-params {:width 24 :height 24}) + (thv/add-variant-with-copy :v01 :c01 :m01 :c02 :m02 :m01-icon :m02-icon :icon-a) + (card :m01) + (card :m02) + (place-icon :m01 :m01-icon) + (place-icon :m02 :m02-icon) + (thc/instantiate-component :c01 :copy01 :children-labels [:copy-icon]) + (tho/change-shape :copy01 #(resize-width % 1100) :ignore-touched true) + (tho/move-shape-tree :copy-icon (gpt/point (case constraints-h :right 800 0) 0) :ignore-touched true)))) + +(defn- setup-card-variant + "Same as `setup-card-copy`, with the icon of the copy swapped to icon B." + [constraints-h] + (-> (setup-card-copy constraints-h) + (thc/component-swap :copy-icon :icon-b :copy-icon-b) + ;; The swap drops the constraints; set them again so this path runs + (tho/change-shape :copy-icon-b #(assoc % :constraints-h constraints-h)))) + +(defn- distance-to-right-edge + [file root-label child-label] + (let [root (ths/get-shape file root-label) + child (ths/get-shape file child-label)] + (- (+ (:x root) (:width root)) (:x child)))) + +(t/deftest test-switch-moved-swapped-child-follows-right-constraint + (let [;; ==== Setup + file (-> (setup-card-variant :right) + (tho/move-shape-tree :copy-icon-b (gpt/point 5 0))) + before (distance-to-right-edge file :copy01 :copy-icon-b) + + ;; ==== Action + file' (tho/swap-component-in-shape file :copy01 :c02 {:new-shape-label :copy02 + :keep-touched? true})] + + ;; ==== Check + (t/is (mth/close? before (distance-to-right-edge file' :copy02 :copy-icon-b))))) + +(t/deftest test-switch-scales-swapped-child-back-with-root + (let [;; ==== Setup + file (setup-card-variant :scale) + width (:width (ths/get-shape file :copy-icon-b)) + root-w (:width (ths/get-shape file :copy01)) + + ;; ==== Action + file' (tho/swap-component-in-shape file :copy01 :c02 {:new-shape-label :copy02 + :keep-touched? true}) + scale (/ (:width (ths/get-shape file' :copy02)) root-w) + inner #(ths/get-shape-by-id % (first (:shapes (ths/get-shape % :copy-icon-b))))] + + ;; ==== Check + (t/is (mth/close? (* width scale) + (:width (ths/get-shape file' :copy-icon-b)))) + ;; The icon content follows the icon through its own (left) constraint + (t/is (mth/close? (:width (inner file)) (:width (inner file')))))) + +(t/deftest test-switch-scales-moved-child-back-with-root + (let [;; ==== Setup + card (setup-card-copy :scale) + grow (/ (:width (ths/get-shape card :copy01)) + (:width (ths/get-shape card :m01))) + file (-> card + ;; The layout scales the icon with the copy + (tho/change-shape :copy-icon #(resize-width % (* grow (:width %))) :ignore-touched true) + (tho/move-shape-tree :copy-icon (gpt/point 5 0))) + width (:width (ths/get-shape file :copy-icon)) + root-w (:width (ths/get-shape file :copy01)) + + ;; ==== Action + file' (tho/swap-component-in-shape file :copy01 :c02 {:new-shape-label :copy02 + :keep-touched? true}) + + ;; ==== Get + copy02' (ths/get-shape file' :copy02) + icon' (get-in (thf/current-page file') [:objects (-> copy02' :shapes first)])] + + ;; ==== Check + (t/is (mth/close? (* width (/ (:width copy02') root-w)) (:width icon'))))) diff --git a/frontend/src/app/main/ui/workspace/sidebar/options/menus/layout_item.cljs b/frontend/src/app/main/ui/workspace/sidebar/options/menus/layout_item.cljs index 659aea2fb3..6a2aed7599 100644 --- a/frontend/src/app/main/ui/workspace/sidebar/options/menus/layout_item.cljs +++ b/frontend/src/app/main/ui/workspace/sidebar/options/menus/layout_item.cljs @@ -506,30 +506,37 @@ :title "Align self end" :id "align-self-end"}]]) +;; A multiple selection gives nil for missing attrs and :multiple for mixed ones +(def ^:private schema:number-value + [:maybe [:or :float :int [:= :multiple]]]) + (def ^:private schema:layout-item-props-schema [:map [:layout-item-margin {:optional true} - [:map - [:m1 {:optional true} [:or :float :int]] - [:m2 {:optional true} [:or :float :int]] - [:m3 {:optional true} [:or :float :int]] - [:m4 {:optional true} [:or :float :int]]]] + [:maybe + [:or + [:= :multiple] + [:map + [:m1 {:optional true} schema:number-value] + [:m2 {:optional true} schema:number-value] + [:m3 {:optional true} schema:number-value] + [:m4 {:optional true} schema:number-value]]]]] - [:layout-item-margin-type {:optional true} :keyword] + [:layout-item-margin-type {:optional true} [:maybe :keyword]] - [:layout-item-h-sizing {:optional true} :keyword] - [:layout-item-v-sizing {:optional true} :keyword] + [:layout-item-h-sizing {:optional true} [:maybe :keyword]] + [:layout-item-v-sizing {:optional true} [:maybe :keyword]] - [:layout-item-min-w {:optional true} [:or :float :int]] - [:layout-item-max-w {:optional true} [:or :float :int]] - [:layout-item-min-h {:optional true} [:or :float :int]] - [:layout-item-max-h {:optional true} [:or :float :int]]]) + [:layout-item-min-w {:optional true} schema:number-value] + [:layout-item-max-w {:optional true} schema:number-value] + [:layout-item-min-h {:optional true} schema:number-value] + [:layout-item-max-h {:optional true} schema:number-value]]) (def ^:private schema:layout-size-constraints [:map [:values schema:layout-item-props-schema] - [:applied-tokens [:maybe [:map-of :keyword :string]]] + [:applied-tokens [:maybe [:map-of :keyword [:maybe [:or :string [:= :multiple]]]]]] [:ids [::sm/vec ::sm/uuid]]]) (mf/defc layout-size-constraints*