diff --git a/common/src/app/common/files/repair.cljc b/common/src/app/common/files/repair.cljc index 4034a15ab..8aead05bc 100644 --- a/common/src/app/common/files/repair.cljc +++ b/common/src/app/common/files/repair.cljc @@ -326,7 +326,8 @@ (log/dbg :hint "repairing shape :nested-main-not-allowed" :id (:id shape) :name (:name shape) :page-id page-id) (-> (pcb/empty-changes nil page-id) (pcb/with-file-data file-data) - (pcb/update-shapes [(:id shape)] repair-shape)))) + (pcb/update-shapes [(:id shape)] repair-shape) + (pcb/change-parent uuid/zero [shape] nil {:component-swap true})))) (defmethod repair-error :root-copy-not-allowed [_ {:keys [shape page-id] :as error} file-data _] diff --git a/common/src/app/common/files/validate.cljc b/common/src/app/common/files/validate.cljc index 1c25b82dc..514e48ab1 100644 --- a/common/src/app/common/files/validate.cljc +++ b/common/src/app/common/files/validate.cljc @@ -393,7 +393,8 @@ (check-shape-copy-root-top shape file page libraries))) (if (ctk/main-instance? shape) - (if (= context :not-component) + ;; mains can't be nested into mains + (if (or (= context :not-component) (= context :main-top)) (report-error :nested-main-not-allowed "Nested main component only allowed inside other component" shape file page) diff --git a/common/src/app/common/types/container.cljc b/common/src/app/common/types/container.cljc index b00a41b1a..cdb8f7e3d 100644 --- a/common/src/app/common/types/container.cljc +++ b/common/src/app/common/types/container.cljc @@ -387,3 +387,49 @@ (if (ctk/in-component-copy? parent) true (has-any-copy-parent? objects (:parent-id shape)))))) + +(defn has-any-main? + "Check if the shape has any children or parent that is a main component." + [objects shape] + (let [children (cfh/get-children-with-self objects (:id shape)) + parents (cfh/get-parents objects (:id shape))] + (or + (some ctk/main-instance? children) + (some ctk/main-instance? parents)))) + +(defn valid-shape-for-component? + "Check if a main component can be generated from this shape in terms of nested components: + - A main can't be the ancestor of another main + - A main can't be nested in copies" + [objects shape] + (and + (not (has-any-main? objects shape)) + (not (has-any-copy-parent? objects shape)))) + +(defn- invalid-structure-for-component? + "Check if the structure generated nesting children in parent is invalid in terms of nested components" + [objects parent children] + (let [selected-main-instance? (some true? (map #(has-any-main? objects %) children)) + parent-in-component? (in-any-component? objects parent) + comps-nesting-loop? (not (->> children + (map #(cfh/components-nesting-loop? objects (:id %) (:id parent))) + (every? nil?)))] + (or + ;;We don't want to change the structure of component copies + (ctk/in-component-copy? parent) + ;; If we are moving something containing a main instance the container can't be part of a component (neither main nor copy) + (and selected-main-instance? parent-in-component?) + ;; Avoid placing a shape as a direct or indirect child of itself, + ;; or inside its main component if it's in a copy. + comps-nesting-loop?))) + +(defn find-valid-parent-and-frame-ids + "Navigate trough the ancestors until find one that is valid" + [parent-id objects children] + (let [parent (get objects parent-id)] + (if (invalid-structure-for-component? objects parent children) + (find-valid-parent-and-frame-ids (:parent-id parent) objects children) + [parent-id + (if (= :frame (:type parent)) + parent-id + (:frame-id parent))]))) diff --git a/frontend/src/app/main/data/workspace.cljs b/frontend/src/app/main/data/workspace.cljs index ee5cf792d..243772c99 100644 --- a/frontend/src/app/main/data/workspace.cljs +++ b/frontend/src/app/main/data/workspace.cljs @@ -1845,18 +1845,9 @@ tree-root (get-tree-root-shapes pobjects) only-one-root-shape? (and (< 1 (count pobjects)) - (= 1 (count tree-root))) - all-objects (merge page-objects pobjects) - comps-nesting-loop? (not (->> (keys pobjects) - (map #(cfh/components-nesting-loop? all-objects % (:id base))) - (every? nil?)))] + (= 1 (count tree-root)))] (cond - comps-nesting-loop? - ;; Avoid placing a shape as a direct or indirect child of itself, - ;; or inside its main component if it's in a copy. - [uuid/zero uuid/zero (gpt/subtract position orig-pos)] - (selected-frame? state) (if (or (any-same-frame-from-selected? state (keys pobjects)) @@ -1869,7 +1860,7 @@ paste-y (:y selected-frame-obj) delta (gpt/subtract (gpt/point paste-x paste-y) orig-pos)] - [(:frame-id base) parent-id delta index]) + [parent-id delta index]) ;; Paste inside selected frame otherwise (let [selected-frame-obj (get page-objects (first page-selected)) @@ -1902,20 +1893,19 @@ ;; - Align it to the limits on the x and y axis ;; - Respect the distance of the object to the right and bottom in the original frame (gpt/point paste-x paste-y))] - [frame-id frame-id delta (dec (count (:shapes selected-frame-obj)))])) + [frame-id delta (dec (count (:shapes selected-frame-obj)))])) (empty? page-selected) (let [frame-id (ctst/top-nested-frame page-objects position) delta (gpt/subtract position orig-pos)] - [frame-id frame-id delta]) + [frame-id delta]) :else - (let [frame-id (:frame-id base) - parent-id (:parent-id base) + (let [parent-id (:parent-id base) delta (if in-viewport? (gpt/subtract position orig-pos) (gpt/subtract (gpt/point (:selrect base)) orig-pos))] - [frame-id parent-id delta index])))) + [parent-id delta index])))) ;; Change the indexes of the pasted shapes (change-add-obj-index [objects selected index change] @@ -1953,64 +1943,65 @@ (ptk/reify ::paste-shapes ptk/WatchEvent (watch [it state _] - (let [file-id (:current-file-id state) - page (wsh/lookup-page state) + (let [file-id (:current-file-id state) + page (wsh/lookup-page state) - media-idx (->> (:media pdata) - (d/index-by :prev-id)) + media-idx (->> (:media pdata) + (d/index-by :prev-id)) - selected (:selected pdata) - objects (:objects pdata) + selected (:selected pdata) + objects (:objects pdata) - position (deref ms/mouse-position) + position (deref ms/mouse-position) ;; Calculate position for the pasted elements - [frame-id - parent-id + [candidate-parent-id delta - index] (calculate-paste-position state objects selected position) + index] (calculate-paste-position state objects selected position) - ;; We don't want to change the structure of component - ;; copies If the parent-id or the frame-id are - ;; component-copies, we need to get the first not copy - ;; parent - parent-id (:id (ctn/get-first-not-copy-parent (:objects page) parent-id)) - frame-id (:id (ctn/get-first-not-copy-parent (:objects page) frame-id)) + page-objects (:objects page) - objects (update-vals objects (partial process-shape file-id frame-id parent-id)) - all-objects (merge (:objects page) objects) + [parent-id + frame-id] (ctn/find-valid-parent-and-frame-ids candidate-parent-id page-objects (vals objects)) - libraries (wsh/get-libraries state) - ldata (wsh/get-file state file-id) + index (if (= candidate-parent-id parent-id) + index + 0) - drop-cell (when (ctl/grid-layout? all-objects parent-id) - (gslg/get-drop-cell frame-id all-objects position)) + objects (update-vals objects (partial process-shape file-id frame-id parent-id)) - changes (-> (dws/prepare-duplicate-changes all-objects page selected delta it libraries ldata file-id) - (pcb/amend-changes (partial process-rchange media-idx)) - (pcb/amend-changes (partial change-add-obj-index objects selected index))) + all-objects (merge page-objects objects) + + libraries (wsh/get-libraries state) + ldata (wsh/get-file state file-id) + + drop-cell (when (ctl/grid-layout? all-objects parent-id) + (gslg/get-drop-cell frame-id all-objects position)) + + changes (-> (dws/prepare-duplicate-changes all-objects page selected delta it libraries ldata file-id) + (pcb/amend-changes (partial process-rchange media-idx)) + (pcb/amend-changes (partial change-add-obj-index objects selected index))) ;; Adds a resize-parents operation so the groups are ;; updated. We add all the new objects - changes (->> (:redo-changes changes) - (filter add-obj?) - (map :id) - (pcb/resize-parents changes)) + changes (->> (:redo-changes changes) + (filter add-obj?) + (map :id) + (pcb/resize-parents changes)) - selected (into (d/ordered-set) - (comp - (filter add-obj?) - (filter #(contains? selected (:old-id %))) - (map :obj) - (map :id)) - (:redo-changes changes)) + selected (into (d/ordered-set) + (comp + (filter add-obj?) + (filter #(contains? selected (:old-id %))) + (map :obj) + (map :id)) + (:redo-changes changes)) - changes (cond-> changes - (some? drop-cell) - (pcb/update-shapes [parent-id] - #(ctl/add-children-to-cell % selected all-objects drop-cell))) - - undo-id (js/Symbol)] + changes (cond-> changes + (some? drop-cell) + (pcb/update-shapes [parent-id] + #(ctl/add-children-to-cell % selected all-objects drop-cell))) + undo-id (js/Symbol)] (rx/of (dwu/start-undo-transaction undo-id) (dch/commit-changes changes) diff --git a/frontend/src/app/main/data/workspace/libraries.cljs b/frontend/src/app/main/data/workspace/libraries.cljs index 54047f0e3..d24b8c95f 100644 --- a/frontend/src/app/main/data/workspace/libraries.cljs +++ b/frontend/src/app/main/data/workspace/libraries.cljs @@ -340,12 +340,16 @@ (ptk/reify ::add-component ptk/WatchEvent (watch [_ state _] - (let [objects (wsh/lookup-page-objects state) - selected (->> (wsh/lookup-selected state) - (cfh/clean-loops objects) - (remove #(ctn/has-any-copy-parent? objects (get objects %)))) ;; We don't want to change the structure of component copies - components-v2 (features/active-feature? state "components/v2")] - (rx/of (add-component2 selected components-v2)))))) + (let [objects (wsh/lookup-page-objects state) + selected (->> (wsh/lookup-selected state) + (cfh/clean-loops objects)) + selected-objects (map #(get objects %) selected) + components-v2 (features/active-feature? state "components/v2") + ;; We don't want to change the structure of component copies + can-make-component (every? true? (map #(ctn/valid-shape-for-component? objects %) selected-objects))] + + (when can-make-component + (rx/of (add-component2 selected components-v2))))))) (defn add-multiple-components "Add several new components to current file library, from the currently selected shapes." @@ -353,19 +357,22 @@ (ptk/reify ::add-multiple-components ptk/WatchEvent (watch [_ state _] - (let [components-v2 (features/active-feature? state "components/v2") - objects (wsh/lookup-page-objects state) - selected (->> (wsh/lookup-selected state) - (cfh/clean-loops objects) - (remove #(ctn/has-any-copy-parent? objects (get objects %)))) ;; We don't want to change the structure of component copies - added-components (map - #(add-component2 [%] components-v2) - selected) + (let [components-v2 (features/active-feature? state "components/v2") + objects (wsh/lookup-page-objects state) + selected (->> (wsh/lookup-selected state) + (cfh/clean-loops objects)) + selected-objects (map #(get objects %) selected) + ;; We don't want to change the structure of component copies + can-make-component (every? true? (map #(ctn/valid-shape-for-component? objects %) selected-objects)) + added-components (map + #(add-component2 [%] components-v2) + selected) undo-id (js/Symbol)] - (rx/concat - (rx/of (dwu/start-undo-transaction undo-id)) - (rx/from added-components) - (rx/of (dwu/commit-undo-transaction undo-id))))))) + (when can-make-component + (rx/concat + (rx/of (dwu/start-undo-transaction undo-id)) + (rx/from added-components) + (rx/of (dwu/commit-undo-transaction undo-id)))))))) (defn rename-component "Rename the component with the given id, in the current file library." diff --git a/frontend/src/app/main/data/workspace/transforms.cljs b/frontend/src/app/main/data/workspace/transforms.cljs index de8195d90..dda92054b 100644 --- a/frontend/src/app/main/data/workspace/transforms.cljs +++ b/frontend/src/app/main/data/workspace/transforms.cljs @@ -560,13 +560,14 @@ (rx/map (fn [[move-vector mod?]] - (let [position (gpt/add from-position move-vector) - exclude-frames (if mod? exclude-frames exclude-frames-siblings) - target-frame (ctst/top-nested-frame objects position exclude-frames) - flex-layout? (ctl/flex-layout? objects target-frame) - grid-layout? (ctl/grid-layout? objects target-frame) - drop-index (when flex-layout? (gslf/get-drop-index target-frame objects position)) - cell-data (when (and grid-layout? (not mod?)) (gslg/get-drop-cell target-frame objects position))] + (let [position (gpt/add from-position move-vector) + exclude-frames (if mod? exclude-frames exclude-frames-siblings) + target-frame (ctst/top-nested-frame objects position exclude-frames) + [target-frame _] (ctn/find-valid-parent-and-frame-ids target-frame objects shapes) + flex-layout? (ctl/flex-layout? objects target-frame) + grid-layout? (ctl/grid-layout? objects target-frame) + drop-index (when flex-layout? (gslf/get-drop-index target-frame objects position)) + cell-data (when (and grid-layout? (not mod?)) (gslg/get-drop-cell target-frame objects position))] (array move-vector target-frame drop-index cell-data)))) (rx/take-until stopper))] @@ -587,16 +588,12 @@ [(assoc move-vector :x 0) :x] :else - [move-vector nil]) + [move-vector nil])] - nesting-loop? (some #(cfh/components-nesting-loop? objects (:id %) target-frame) shapes) - is-component-copy? (ctk/in-component-copy? (get objects target-frame))] - (cond-> (dwm/create-modif-tree ids (ctm/move-modifiers move-vector)) - (and (not nesting-loop?) (not is-component-copy?)) - (dwm/build-change-frame-modifiers objects selected target-frame drop-index cell-data) - :always - (dwm/set-modifiers false false {:snap-ignore-axis snap-ignore-axis})))))) + (-> (dwm/create-modif-tree ids (ctm/move-modifiers move-vector)) + (dwm/build-change-frame-modifiers objects selected target-frame drop-index cell-data) + (dwm/set-modifiers false false {:snap-ignore-axis snap-ignore-axis})))))) (->> move-stream (rx/with-latest-from ms/mouse-position-alt) diff --git a/frontend/src/app/main/ui/workspace/context_menu.cljs b/frontend/src/app/main/ui/workspace/context_menu.cljs index da56b6338..12fde79e3 100644 --- a/frontend/src/app/main/ui/workspace/context_menu.cljs +++ b/frontend/src/app/main/ui/workspace/context_menu.cljs @@ -422,13 +422,13 @@ (let [components-v2 (features/use-feature "components/v2") single? (= (count shapes) 1) objects (deref refs/workspace-page-objects) - any-in-copy? (some true? (map #(ctn/has-any-copy-parent? objects %) shapes)) + can-make-component (every? true? (map #(ctn/valid-shape-for-component? objects %) shapes)) heads (filter ctk/instance-head? shapes) components-menu-entries (cmm/generate-components-menu-entries heads components-v2) do-add-component #(st/emit! (dwl/add-component)) do-add-multiple-components #(st/emit! (dwl/add-multiple-components))] [:* - (when-not any-in-copy? ;; We don't want to change the structure of component copies + (when can-make-component ;; We don't want to change the structure of component copies [:* [:& menu-separator] diff --git a/frontend/src/app/main/ui/workspace/sidebar/layer_item.cljs b/frontend/src/app/main/ui/workspace/sidebar/layer_item.cljs index cb7cd5f04..fe1db4200 100644 --- a/frontend/src/app/main/ui/workspace/sidebar/layer_item.cljs +++ b/frontend/src/app/main/ui/workspace/sidebar/layer_item.cljs @@ -10,7 +10,6 @@ [app.common.data :as d] [app.common.data.macros :as dm] [app.common.files.helpers :as cfh] - [app.common.types.component :as ctk] [app.common.types.container :as ctn] [app.common.types.shape.layout :as ctl] [app.common.uuid :as uuid] @@ -261,7 +260,7 @@ on-drop (mf/use-fn - (mf/deps id index objects expanded?) + (mf/deps id index objects expanded? selected) (fn [side _data] (let [shape (get objects id) @@ -276,6 +275,8 @@ :else (cfh/get-parent-id objects id)) + [parent-id _] (ctn/find-valid-parent-and-frame-ids parent-id objects (map #(get objects %) selected)) + parent (get objects parent-id) to-index (cond @@ -283,9 +284,7 @@ (and expanded? (= side :bot) (d/not-empty? (:shapes shape))) (count (:shapes parent)) (= side :top) (inc index) :else index)] - - (when-not (ctk/in-component-copy? parent) ;; We don't want to change the structure of component copies - (st/emit! (dw/relocate-selected-shapes parent-id to-index)))))) + (st/emit! (dw/relocate-selected-shapes parent-id to-index))))) on-hold (mf/use-fn diff --git a/frontend/test/frontend_tests/state_components_test.cljs b/frontend/test/frontend_tests/state_components_test.cljs index fb341f12e..7cb62b212 100644 --- a/frontend/test/frontend_tests/state_components_test.cljs +++ b/frontend/test/frontend_tests/state_components_test.cljs @@ -170,7 +170,7 @@ (dwl/add-component) :the/end)))) -(t/deftest test-add-component-from-component +(t/deftest test-add-component-from-component-instance (t/async done (let [state (-> thp/initial-state @@ -178,30 +178,27 @@ (thp/sample-shape :shape1 :rect {:name "Rect 1"}) (thp/make-component :main1 :component1 - [(thp/id :shape1)])) + [(thp/id :shape1)]) + (thp/instantiate-component :instance1 (thp/id :component1))) store (the/prepare-store state done (fn [new-state] - ;; Expected shape tree: - ;; - ;; [Page] - ;; Root Frame - ;; Rect 1 - ;; Rect 1 - ;; Rect 1 - ;; - ;; [Rect 1] - ;; page1 / Rect 1 - ;; - ;; [Rect 1] - ;; page1 / Rect 1 - ;; + ;; Expected shape tree: + ;; + ;; [Page: Page] + ;; Root Frame + ;; Rect 1 # + ;; Rect 1 + ;; Rect 1 # + ;; Rect 1* @--> Rect 1 + ;; Rect 1 ---> Rect 1 + ;; (let [[[instance1 shape1] [c-instance1 c-shape1] component1] (thl/resolve-instance-and-main new-state - (thp/id :main1) + (thp/id :instance1) true) [[instance2 instance1' shape1'] @@ -225,6 +222,56 @@ (t/is (= (:name c-instance1') "Rect 1")) (t/is (= (:name c-instance2) "Rect 1")))))] + (ptk/emit! + store + (dw/select-shape (thp/id :instance1)) + (dwl/add-component) + :the/end)))) + + +(t/deftest test-add-component-from-component-main + (t/async + done + (let [state (-> thp/initial-state + (thp/sample-page) + (thp/sample-shape :shape1 :rect + {:name "Rect 1"}) + (thp/make-component :main1 :component1 + [(thp/id :shape1)])) + + store (the/prepare-store state done + (fn [new-state] + ;; Expected shape tree: + ;; + ;; [Page] + ;; Root Frame + ;; Rect 1 + ;; Rect 1 + ;; + ;; [Rect 1] + ;; page1 / Rect 1 + ;; + (let [file (wsh/get-local-file new-state) + components (ctkl/components file) + page (thp/current-page new-state) + shape1 (thp/get-shape new-state :shape1) + parent1 (ctn/get-shape page (:parent-id shape1)) + main1 (thp/get-shape state :main1) + [[instance1 shape1] + [c-instance1 c-shape1] + component1] + (thl/resolve-instance-and-main + new-state + (:id main1))] + ;; Creating a component from a main doesn't generate a new component + (t/is (= (count components) 1)) + + (t/is (= (:name shape1) "Rect 1")) + (t/is (= (:name instance1) "Rect 1")) + (t/is (= (:name component1) "Rect 1")) + (t/is (= (:name c-shape1) "Rect 1")) + (t/is (= (:name c-instance1) "Rect 1")))))] + (ptk/emit! store (dw/select-shape (thp/id :main1)) @@ -580,7 +627,62 @@ (dwl/detach-component (:id instance1)) :the/end)))) -(t/deftest test-add-nested-component + + +(t/deftest test-add-nested-component-instance + (t/async + done + (let [state (-> thp/initial-state + (thp/sample-page) + (thp/sample-shape :shape1 :rect + {:name "Rect 1"}) + (thp/make-component :main1 :component1 + [(thp/id :shape1)]) + (thp/instantiate-component :instance1 (thp/id :component1))) + + store (the/prepare-store state done + (fn [new-state] + ;; Expected shape tree: + ;; + ;; [Page] + ;; Root Frame + ;; Rect 1 + ;; Rect 1 + ;; Board + ;; Rect 1 + ;; Rect 1 + ;; + ;; [Rect 1] + ;; page1 / Rect 1 + ;; + ;; [Board] + ;; page1 / Board + ;; + (let [instance1 (thp/get-shape new-state :instance1) + + [[group shape1 shape2] + [c-group c-shape1 c-shape2] + component] + (thl/resolve-instance-and-main + new-state + (:parent-id instance1))] + + (t/is (= (:name group) "Board")) + (t/is (= (:name shape1) "Rect 1")) + (t/is (= (:name shape2) "Rect 1")) + (t/is (= (:name component) "Board")) + (t/is (= (:name c-group) "Board")) + (t/is (= (:name c-shape1) "Rect 1")) + (t/is (= (:name c-shape2) "Rect 1")))))] + + (ptk/emit! + store + (dw/select-shape (thp/id :instance1)) + (dwsh/create-artboard-from-selection) + (dwl/add-component) + :the/end)))) + +(t/deftest test-add-nested-component-main (t/async done (let [state (-> thp/initial-state @@ -594,34 +696,36 @@ ;; ;; [Page] ;; Root Frame - ;; Group + ;; Board ;; Rect 1 ;; Rect 1 ;; ;; [Rect 1] ;; page1 / Rect 1 ;; - ;; [Group] - ;; page1 / Group ;; - (let [page (thp/current-page new-state) + (let [file (wsh/get-local-file new-state) + components (ctkl/components file) + page (thp/current-page new-state) + shape1 (thp/get-shape new-state :shape1) parent1 (ctn/get-shape page (:parent-id shape1)) - [[group shape1 shape2] - [c-group c-shape1 c-shape2] + [[group shape1] + [c-group c-shape1] component] (thl/resolve-instance-and-main new-state - (:parent-id parent1))] + (:parent-id shape1))] - (t/is (= (:name group) "Board")) + ;; Creating a component from something containing a main doesn't generate a new component + (t/is (= (count components) 1)) + + (t/is (= (:name group) "Rect 1")) (t/is (= (:name shape1) "Rect 1")) - (t/is (= (:name shape2) "Rect 1")) - (t/is (= (:name component) "Board")) - (t/is (= (:name c-group) "Board")) - (t/is (= (:name c-shape1) "Rect 1")) - (t/is (= (:name c-shape2) "Rect 1")))))] + (t/is (= (:name component) "Rect 1")) + (t/is (= (:name c-group) "Rect 1")) + (t/is (= (:name c-shape1) "Rect 1")))))] (ptk/emit! store