diff --git a/.serena/memories/common/changes-architecture.md b/.serena/memories/common/changes-architecture.md index a329bf80e4..b7ba2e1974 100644 --- a/.serena/memories/common/changes-architecture.md +++ b/.serena/memories/common/changes-architecture.md @@ -56,3 +56,11 @@ To inspect what a UI action emitted, use `mem:frontend/cljs-repl` with the snipp 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. + +Normal duplication checks the component nesting guard too (F181). A finite +repeated component tree can use a swapped child to cut recursion; adding +another child in its main can open an expanding branch. cld/allow-duplicate? +combines copy-structure and nesting guards for normal frontend/pure nodes. +The duplicate builder checks the actual destination after ALT placement; +a duplicate at the page root remains allowed. Undo still restores existing +nesting through its explicit inverse flag, not through a forward bypass. diff --git a/common/src/app/common/logic/duplicate.cljc b/common/src/app/common/logic/duplicate.cljc index 206290b240..62057ae908 100644 --- a/common/src/app/common/logic/duplicate.cljc +++ b/common/src/app/common/logic/duplicate.cljc @@ -494,6 +494,13 @@ objects (mapcat #(ctn/get-child-heads objects %) selected)))) +(defn allow-duplicate? + "Whether a duplicate at the same position preserves component structure." + [objects shape] + (and (some? shape) + (ctk/allow-duplicate? objects shape) + (not (cfh/components-nesting-loop? objects (:id shape) (:parent-id shape))))) + (defn generate-duplicate-changes "Prepare objects to duplicate: generate new ids, move them to the desired position, and recalculate parents and frames as needed." @@ -511,11 +518,12 @@ ;; This is because inside a variant-container can't be a copy ;; For other shape this way the layout won't be changed when duplicated ;; and if you move outside the layout will not change - shapes (map (fn [shape] - (cond-> shape - alt-duplication? - (assoc :parent-id uuid/zero :frame-id uuid/zero))) - shapes) + shapes (->> shapes + (map (fn [shape] + (cond-> shape + alt-duplication? + (assoc :parent-id uuid/zero :frame-id uuid/zero)))) + (remove #(cfh/components-nesting-loop? all-objects (:id %) (:parent-id %)))) changes (-> changes 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 f888032fce..b9e04c08ac 100644 --- a/common/src/app/common/test_helpers/composable/comp/nodes.cljc +++ b/common/src/app/common/test_helpers/composable/comp/nodes.cljc @@ -988,9 +988,9 @@ shape-id (tm/target-shape-id situation target) page (thf/current-page the-file) objects (:objects page) - _ (when-not (ctk/allow-duplicate? objects (get objects shape-id)) + _ (when-not (cld/allow-duplicate? objects (get objects shape-id)) ;; the frontend's `duplicate-shapes` drops it and does nothing - (throw (ex-info "The shape cannot be duplicated (inside a copy)" + (throw (ex-info "The shape cannot be duplicated in its current parent" {:type ::duplicate-refused :target target}))) changes (-> (pcb/empty-changes nil) (cld/generate-duplicate-changes objects diff --git a/common/src/app/common/test_helpers/composable/gen.cljc b/common/src/app/common/test_helpers/composable/gen.cljc index ddf03b4c18..5f805289a9 100644 --- a/common/src/app/common/test_helpers/composable/gen.cljc +++ b/common/src/app/common/test_helpers/composable/gen.cljc @@ -11,6 +11,7 @@ (:refer-clojure :exclude [flush]) (:require [app.common.files.helpers :as cfh] + [app.common.logic.duplicate :as cld] [app.common.logic.shapes :as cls] [app.common.test-helpers.composable.comp.files :as cf] [app.common.test-helpers.composable.comp.nodes :as n] @@ -120,7 +121,8 @@ :add-child (ctk/main-instance? shape) :remove-child (and (= :rect (:type shape)) (nil? (:shape-ref shape)) (some ctk/main-instance? (cfh/get-parents-with-self objects (:id shape)))) - :duplicate (and (top-copy shape) (< (count objects) 120)) + :duplicate (and (top-copy shape) (< (count objects) 120) + (cld/allow-duplicate? objects shape)) :nest (and (top-copy shape) (not (overridden (:id shape)))) :delete (ctk/main-instance? shape) false)) diff --git a/common/test/common_tests/logic/copying_and_duplicating_test.cljc b/common/test/common_tests/logic/copying_and_duplicating_test.cljc index 2c8f1161ee..404ab7a991 100644 --- a/common/test/common_tests/logic/copying_and_duplicating_test.cljc +++ b/common/test/common_tests/logic/copying_and_duplicating_test.cljc @@ -8,10 +8,15 @@ (:require [app.common.files.changes :as ch] [app.common.files.changes-builder :as pcb] + [app.common.geom.point :as gpt] + [app.common.logic.duplicate :as cld] [app.common.logic.libraries :as cll] [app.common.logic.shapes :as cls] [app.common.pprint :as pp] [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] @@ -24,6 +29,38 @@ (t/use-fixtures :each thi/test-fixture) +(t/deftest duplicating-existing-component-nesting-does-not-open-an-expanding-branch + (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) + (thc/instantiate-component :b :copy-b :children-labels [:copy-a]) + (tho/add-rect :safe)) + nested (-> (tm/make-situation file) + (r/step (n/swap-shape :copy-a :other)) r/settle + (r/step (n/nest-copy :copy-b :main-a)) r/settle) + file (tm/file nested) + page (thf/current-page file) + objects (:objects page) + id (thi/id :nested-a) + generate (fn [ids & options] + (apply cld/generate-duplicate-changes (pcb/empty-changes) + objects page ids (gpt/point 0 0) {(:id file) file} + (:data file) (:id file) options)) + refused (generate #{id}) + mixed (generate #{id (thi/id :safe)}) + added (filter #(= :add-obj (:type %)) (:redo-changes mixed)) + outside (generate #{id} :alt-duplication? true) + root (first (filter #(and (= :add-obj (:type %)) (= id (:old-id %))) + (:redo-changes outside)))] + (t/is (empty? (:redo-changes refused))) + (t/is (= (:data file) (:data (thf/apply-changes file refused)))) + (t/is (not-any? #(= id (:old-id %)) added)) + (t/is (some #(= (thi/id :safe) (:old-id %)) added)) + (t/is (= uuid/zero (get-in root [:obj :parent-id]))))) + (defn- setup [] (-> (thf/sample-file :file1) (tho/add-simple-component :simple-1 :frame-simple-1 :rect-simple-1 diff --git a/frontend/src/app/main/data/workspace/selection.cljs b/frontend/src/app/main/data/workspace/selection.cljs index 063e85cb04..d7c42ae959 100644 --- a/frontend/src/app/main/data/workspace/selection.cljs +++ b/frontend/src/app/main/data/workspace/selection.cljs @@ -473,7 +473,9 @@ objects (:objects page) ids (into #{} (comp (map (d/getf objects)) - (filter #(ctk/allow-duplicate? objects %)) + (filter #(if alt-duplication? + (ctk/allow-duplicate? objects %) + (cld/allow-duplicate? objects %))) (map :id)) ids)] (when (seq ids)