diff --git a/.serena/memories/common/changes-architecture.md b/.serena/memories/common/changes-architecture.md index 92be62f865..a329bf80e4 100644 --- a/.serena/memories/common/changes-architecture.md +++ b/.serena/memories/common/changes-architecture.md @@ -49,3 +49,10 @@ To inspect what a UI action emitted, use `mem:frontend/cljs-repl` with the snipp When the builder lacks a main's container (inverse sync), use that main container's original index. Both rules preserve order after several siblings disappear (F158). + +- A move inverse can restore component nesting that existed before the + move, even when the normal component-loop check would reject creating + it now (F177). `change-parent` records `:restore-component-nesting` on + the inverse only when the original position already repeats a component. + The change applier still rejects object-tree cycles and copy-structure + edits. Normal forward moves keep the component-loop guard. diff --git a/common/src/app/common/files/changes.cljc b/common/src/app/common/files/changes.cljc index 333cbd82cb..f39f3bbb48 100644 --- a/common/src/app/common/files/changes.cljc +++ b/common/src/app/common/files/changes.cljc @@ -238,6 +238,7 @@ [:shapes ::sm/any] [:index {:optional true} [:maybe :int]] [:after-shape {:optional true} ::sm/any] + [:restore-component-nesting {:optional true} :boolean] [:allow-altering-copies {:optional true} :boolean]]] [:reorder-children @@ -755,7 +756,8 @@ (defmethod process-change :mov-objects ;; FIXME: ignore-touched is no longer used, so we can consider it deprecated - [data {:keys [parent-id shapes index page-id component-id #_ignore-touched after-shape allow-altering-copies syncing]}] + [data {:keys [parent-id shapes index page-id component-id #_ignore-touched after-shape + allow-altering-copies restore-component-nesting syncing]}] (letfn [(calculate-invalid-targets [objects shape-id] (let [reduce-fn #(into %1 (calculate-invalid-targets objects %2))] (->> (get-in objects [shape-id :shapes]) @@ -769,7 +771,11 @@ shape (get objects shape-id)] (and shape (not (invalid-targets parent-id)) - (not (cfh/components-nesting-loop? objects shape-id parent-id)) + ;; Undo may restore repeated component nesting that was + ;; already present before the move. Object cycles above + ;; and copy structure guards below still apply. + (or restore-component-nesting + (not (cfh/components-nesting-loop? objects shape-id parent-id))) (or ;; In some cases (like a component ;; swap) it's allowed to change the diff --git a/common/src/app/common/files/changes_builder.cljc b/common/src/app/common/files/changes_builder.cljc index c0379dc3fd..ff5274563d 100644 --- a/common/src/app/common/files/changes_builder.cljc +++ b/common/src/app/common/files/changes_builder.cljc @@ -553,6 +553,8 @@ :shapes [(:id shape)] :after-shape prev-sibling :index 0} ; index is used in case there is no after-shape (moving bottom shapes) + (cfh/components-nesting-loop? objects (:id shape) (:parent-id shape)) + (assoc :restore-component-nesting true) (:allow-altering-copies options) (assoc :allow-altering-copies true))))) diff --git a/common/test/common_tests/logic/move_shapes_test.cljc b/common/test/common_tests/logic/move_shapes_test.cljc index 7619e8abbf..8ac17de5ff 100644 --- a/common/test/common_tests/logic/move_shapes_test.cljc +++ b/common/test/common_tests/logic/move_shapes_test.cljc @@ -8,6 +8,10 @@ (:require [app.common.files.changes-builder :as pcb] [app.common.logic.shapes :as cls] + [app.common.test-helpers.components :as thc] + [app.common.test-helpers.composable.comp.nodes :as n] + [app.common.test-helpers.composable.comp.runner :as r] + [app.common.test-helpers.composable.core :as tm] [app.common.test-helpers.compositions :as tho] [app.common.test-helpers.files :as thf] [app.common.test-helpers.ids-map :as thi] @@ -21,6 +25,43 @@ (t/use-fixtures :each thi/test-fixture) +(t/deftest moving-out-of-existing-component-nesting-undoes-exactly + (let [file (-> (thf/sample-file :file) + (tho/add-simple-component :a :main-a :rect-a) + (tho/add-simple-component :other :main-other :rect-other) + (tho/add-frame :main-b) + (thc/instantiate-component :a :nested-a :parent-label :main-b) + (thc/make-component :b :main-b) + (tho/add-frame :main-c) + (thc/instantiate-component :b :nested-b :parent-label :main-c) + (thc/make-component :c :main-c) + (thc/instantiate-component :c :copy-c :children-labels [:copy-b :copy-a]) + (tho/add-simple-component :destination-component :destination :destination-child)) + nested (-> (tm/make-situation file) + (r/step (n/swap-shape :copy-a :other)) r/settle + (r/step (n/nest-copy :copy-c :main-a)) r/settle) + moved (-> nested (r/step (n/nest-copy :nested-a :destination)) r/settle) + undone (r/step moved (n/undo)) + page (thf/current-page (tm/file moved)) + forward (cls/generate-relocate (-> (pcb/empty-changes nil (:id page)) + (pcb/with-objects (:objects page)) + (pcb/with-library-data (:data (tm/file moved)))) + (thi/id :main-b) 0 #{(thi/id :nested-a)}) + refused (thf/apply-changes (tm/file moved) forward) + old-page (thf/current-page (tm/file nested)) + move-changes (-> (pcb/empty-changes nil (:id old-page)) + (pcb/with-objects (:objects old-page)) + (pcb/change-parent (thi/id :destination) [(tm/resolve-shape nested :nested-a)])) + inverse (first (filter #(= :mov-objects (:type %)) (:undo-changes move-changes))) + cyclic (assoc inverse :parent-id (first (:shapes (tm/resolve-shape moved :nested-a)))) + cycle-refused (thf/apply-changes (tm/file moved) {:redo-changes [cyclic]})] + (t/is (= (thi/id :destination) (:parent-id (tm/resolve-shape moved :nested-a)))) + (t/is (= (thi/id :destination) (:parent-id (ths/get-shape refused :nested-a)))) + (t/is (= (thn/normalize-file (tm/file moved)) (thn/normalize-file cycle-refused))) + (t/is (= (thn/normalize-file (tm/file nested)) (thn/normalize-file (tm/file undone)))) + (t/is (= (thn/normalize-file (tm/file moved)) + (thn/normalize-file (tm/file (r/step undone (n/redo)))))))) + (t/deftest moving-a-child-out-of-a-grid-clears-its-old-cell (let [file (-> (thf/sample-file :file1) (tho/add-grid-frame :grid :width 200 :height 100)