Penpot/penpot
0
Fork 0
mirror of https://github.com/penpot/penpot.git synced 2025-02-12 18:18:24 -05:00
Code Issues Activity

✨ Add nesting constraints for components

Browse source
This commit is contained in:
Alejandro Alonso 2023-12-13 16:59:01 +01:00 • committed by Andrés Moya
parent 3c07416c48
commit a78eb226e2
9 changed files with 277 additions and 131 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

3
common/src/app/common/files/repair.cljc
View file

@ -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 _]

3
common/src/app/common/files/validate.cljc
View file

@ -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)

46
common/src/app/common/types/container.cljc
View file

@ -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))])))

107
frontend/src/app/main/data/workspace.cljs
View file

@ -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)

43
frontend/src/app/main/data/workspace/libraries.cljs
View file

@ -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."

27
frontend/src/app/main/data/workspace/transforms.cljs
View file

@ -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)

4
frontend/src/app/main/ui/workspace/context_menu.cljs
View file

@ -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]

9
frontend/src/app/main/ui/workspace/sidebar/layer_item.cljs
View file

@ -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

166
frontend/test/frontend_tests/state_components_test.cljs
View file

@ -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