3pp-mirror/penpot
1
0
Fork 0
mirror of https://github.com/penpot/penpot.git synced 2025-08-06 05:28:28 +02:00
Code Issues Projects Releases Packages Wiki Activity

✨ Add removal of variant container when it becomes empty (#6311)

Browse source
This commit is contained in:
Pablo Alba 2025-04-22 09:22:18 +02:00 • committed by GitHub
parent fae1df7f4b
commit fe003d7496
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 185 additions and 123 deletions
Download patch file Download diff file Expand all files Collapse all files

8
common/src/app/common/files/changes_builder.cljc
View file

@ -1095,3 +1095,11 @@
(defn get-objects (defn get-objects
[changes] [changes]
(dm/get-in (::file-data (meta changes)) [:pages-index uuid/zero :objects])) (dm/get-in (::file-data (meta changes)) [:pages-index uuid/zero :objects]))
(defn get-page
[changes]
(::page (meta changes)))
(defn get-page-id
[changes]
(::page-id (meta changes)))

269
common/src/app/common/logic/shapes.cljc
View file

@ -13,6 +13,7 @@
[app.common.logic.variant-properties :as clvp] [app.common.logic.variant-properties :as clvp]
[app.common.types.component :as ctk] [app.common.types.component :as ctk]
[app.common.types.container :as ctn] [app.common.types.container :as ctn]
[app.common.types.pages-list :as ctpl]
[app.common.types.shape.interactions :as ctsi] [app.common.types.shape.interactions :as ctsi]
[app.common.types.shape.layout :as ctl] [app.common.types.shape.layout :as ctl]
[app.common.types.token :as cto] [app.common.types.token :as cto]
@ -80,161 +81,167 @@
(pcb/update-shapes ids update-fn {:attrs #{:blocked :hidden}})))) (pcb/update-shapes ids update-fn {:attrs #{:blocked :hidden}}))))
(defn generate-delete-shapes (defn generate-delete-shapes
[changes file page objects ids {:keys [ignore-touched component-swap]}] ([changes file page objects ids options]
(let [ids (cfh/clean-loops objects ids) (generate-delete-shapes (-> changes
(pcb/with-page page)
(pcb/with-objects objects)
(pcb/with-library-data file))
ids
options))
([changes ids {:keys [ignore-touched component-swap]}]
(let [objects (pcb/get-objects changes)
data (pcb/get-library-data changes)
page-id (pcb/get-page-id changes)
page (or (pcb/get-page changes)
(ctpl/get-page data page-id))
in-component-copy? ids (cfh/clean-loops objects ids)
(fn [shape-id] in-component-copy?
(fn [shape-id]
;; Look for shapes that are inside a component copy, but are ;; Look for shapes that are inside a component copy, but are
;; not the root. In this case, they must not be deleted, ;; not the root. In this case, they must not be deleted,
;; but hidden (to be able to recover them more easily). ;; but hidden (to be able to recover them more easily).
;; Unless we are doing a component swap, in which case we want ;; Unless we are doing a component swap, in which case we want
;; to delete the old shape ;; to delete the old shape
(let [shape (get objects shape-id)] (let [shape (get objects shape-id)]
(and (ctn/has-any-copy-parent? objects shape) (and (ctn/has-any-copy-parent? objects shape)
(not component-swap)))) (not component-swap))))
[ids-to-delete ids-to-hide] [ids-to-delete ids-to-hide]
(loop [ids-seq (seq ids) (loop [ids-seq (seq ids)
ids-to-delete [] ids-to-delete []
ids-to-hide []] ids-to-hide []]
(let [id (first ids-seq)] (let [id (first ids-seq)]
(if (nil? id) (if (nil? id)
[ids-to-delete ids-to-hide] [ids-to-delete ids-to-hide]
(if (in-component-copy? id) (if (in-component-copy? id)
(recur (rest ids-seq) (recur (rest ids-seq)
ids-to-delete ids-to-delete
(conj ids-to-hide id)) (conj ids-to-hide id))
(recur (rest ids-seq) (recur (rest ids-seq)
(conj ids-to-delete id) (conj ids-to-delete id)
ids-to-hide))))) ids-to-hide)))))
lookup (d/getf objects)
changes (-> changes groups-to-unmask
(pcb/with-page page) (reduce (fn [group-ids id]
(pcb/with-objects objects)
(pcb/with-library-data file))
lookup (d/getf objects)
groups-to-unmask
(reduce (fn [group-ids id]
;; When the shape to delete is the mask of a masked group, ;; When the shape to delete is the mask of a masked group,
;; the mask condition must be removed, and it must be ;; the mask condition must be removed, and it must be
;; converted to a normal group. ;; converted to a normal group.
(let [obj (lookup id) (let [obj (lookup id)
parent (lookup (:parent-id obj))] parent (lookup (:parent-id obj))]
(if (and (:masked-group parent) (if (and (:masked-group parent)
(= id (first (:shapes parent)))) (= id (first (:shapes parent))))
(conj group-ids (:id parent)) (conj group-ids (:id parent))
group-ids))) group-ids)))
#{} #{}
ids-to-delete) ids-to-delete)
interacting-shapes interacting-shapes
(filter (fn [shape] (filter (fn [shape]
;; If any of the deleted shapes is the destination of ;; If any of the deleted shapes is the destination of
;; some interaction, this must be deleted, too. ;; some interaction, this must be deleted, too.
(let [interactions (:interactions shape)] (let [interactions (:interactions shape)]
(some #(and (ctsi/has-destination %) (some #(and (ctsi/has-destination %)
(contains? ids-to-delete (:destination %))) (contains? ids-to-delete (:destination %)))
interactions))) interactions)))
(vals objects)) (vals objects))
changes changes
(reduce (fn [changes {:keys [id] :as flow}] (reduce (fn [changes {:keys [id] :as flow}]
(if (contains? ids-to-delete (:starting-frame flow)) (if (contains? ids-to-delete (:starting-frame flow))
(pcb/set-flow changes id nil) (pcb/set-flow changes id nil)
changes)) changes))
changes changes
(:flows page)) (:flows page))
all-parents all-parents
(reduce (fn [res id] (reduce (fn [res id]
;; All parents of any deleted shape must be resized. ;; All parents of any deleted shape must be resized.
(into res (cfh/get-parent-ids objects id))) (into res (cfh/get-parent-ids objects id)))
(d/ordered-set) (d/ordered-set)
(concat ids-to-delete ids-to-hide)) (concat ids-to-delete ids-to-hide))
all-children all-children
(->> ids-to-delete ;; Children of deleted shapes must be also deleted. (->> ids-to-delete ;; Children of deleted shapes must be also deleted.
(reduce (fn [res id] (reduce (fn [res id]
(into res (cfh/get-children-ids objects id))) (into res (cfh/get-children-ids objects id)))
[]) [])
(reverse) (reverse)
(into (d/ordered-set))) (into (d/ordered-set)))
find-all-empty-parents find-all-empty-parents
(fn recursive-find-empty-parents [empty-parents] (fn recursive-find-empty-parents [empty-parents]
(let [all-ids (into empty-parents ids-to-delete) (let [all-ids (into empty-parents ids-to-delete)
contains? (partial contains? all-ids) contains? (partial contains? all-ids)
xform (comp (map lookup) xform (comp (map lookup)
(filter #(or (cfh/group-shape? %) (cfh/bool-shape? %))) (filter #(or (cfh/group-shape? %) (cfh/bool-shape? %) (ctk/is-variant-container? %)))
(remove #(->> (:shapes %) (remove contains?) seq)) (remove #(->> (:shapes %) (remove contains?) seq))
(map :id)) (map :id))
parents (into #{} xform all-parents)] parents (into #{} xform all-parents)]
(if (= empty-parents parents) (if (= empty-parents parents)
empty-parents empty-parents
(recursive-find-empty-parents parents)))) (recursive-find-empty-parents parents))))
empty-parents empty-parents
;; Any parent whose children are all deleted, must be deleted too. ;; Any parent whose children are all deleted, must be deleted too.
;; Unless we are during a component swap: in this case we are replacing a shape by ;; Unless we are during a component swap: in this case we are replacing a shape by
;; other one, so must not delete empty parents. ;; other one, so must not delete empty parents.
(if-not component-swap (if-not component-swap
(into (d/ordered-set) (find-all-empty-parents #{})) (into (d/ordered-set) (find-all-empty-parents #{}))
#{}) #{})
components-to-delete components-to-delete
(reduce (fn [components id] (reduce (fn [components id]
(let [shape (get objects id)] (let [shape (get objects id)]
(if (and (= (:component-file shape) (:id file)) ;; Main instances should exist only in local file (if (and (= (:component-file shape) (:id data)) ;; Main instances should exist only in local file
(:main-instance shape)) ;; but check anyway (:main-instance shape)) ;; but check anyway
(conj components (:component-id shape)) (conj components (:component-id shape))
components))) components)))
[] []
(into ids-to-delete all-children)) (into ids-to-delete all-children))
ids-set (set ids-to-delete) ids-set (set ids-to-delete)
guides-to-delete guides-to-delete
(->> (:guides page) (->> (:guides page)
(vals) (vals)
(filter #(contains? ids-set (:frame-id %))) (filter #(contains? ids-set (:frame-id %)))
(map :id)) (map :id))
changes (reduce (fn [changes guide-id] changes (reduce (fn [changes guide-id]
(pcb/set-flow changes guide-id nil)) (pcb/set-flow changes guide-id nil))
changes changes
guides-to-delete) guides-to-delete)
changes (reduce (fn [changes component-id] changes (reduce (fn [changes component-id]
;; It's important to delete the component before the main instance, because we ;; It's important to delete the component before the main instance, because we
;; need to store the instance position if we want to restore it later. ;; need to store the instance position if we want to restore it later.
(pcb/delete-component changes component-id (:id page))) (pcb/delete-component changes component-id (:id page)))
changes changes
components-to-delete) components-to-delete)
changes (-> changes changes (-> changes
(generate-update-shape-flags ids-to-hide objects {:hidden true}) (generate-update-shape-flags ids-to-hide objects {:hidden true})
(pcb/remove-objects all-children {:ignore-touched true}) (pcb/remove-objects all-children {:ignore-touched true})
(pcb/remove-objects ids-to-delete {:ignore-touched ignore-touched}) (pcb/remove-objects ids-to-delete {:ignore-touched ignore-touched})
(pcb/remove-objects empty-parents) (pcb/remove-objects empty-parents)
(pcb/resize-parents all-parents) (pcb/resize-parents all-parents)
(pcb/update-shapes groups-to-unmask (pcb/update-shapes groups-to-unmask
(fn [shape] (fn [shape]
(assoc shape :masked-group false))) (assoc shape :masked-group false)))
(pcb/update-shapes (map :id interacting-shapes) (pcb/update-shapes (map :id interacting-shapes)
(fn [shape] (fn [shape]
(d/update-when shape :interactions (d/update-when shape :interactions
(fn [interactions] (fn [interactions]
(into [] (into []
(remove #(and (ctsi/has-destination %) (remove #(and (ctsi/has-destination %)
(contains? ids-to-delete (:destination %)))) (contains? ids-to-delete (:destination %))))
interactions))))))] interactions))))))]
[all-parents changes])) [all-parents changes])))
(defn generate-relocate (defn generate-relocate
@ -336,7 +343,19 @@
(map :id))) (map :id)))
index-cell-data (when to-index (ctl/get-cell-by-index parent to-index)) index-cell-data (when to-index (ctl/get-cell-by-index parent to-index))
cell (or cell (and index-cell-data [(:row index-cell-data) (:column index-cell-data)]))] cell (or cell (and index-cell-data [(:row index-cell-data) (:column index-cell-data)]))
;; Parents that are a variant-container that becomes empty
empty-variant-cont (reduce
(fn [to-delete parent-id]
(let [parent (get objects parent-id)]
(if (and (ctk/is-variant-container? parent)
(empty? (remove (set ids) (:shapes parent))))
(conj to-delete (:id parent))
to-delete)))
#{}
(remove #(= % parent-id) all-parents))]
(-> changes (-> changes
;; Remove layout-item properties when moving a shape outside a layout ;; Remove layout-item properties when moving a shape outside a layout
@ -444,7 +463,11 @@
(pcb/update-shapes ids #(assoc % :blocked true))) (pcb/update-shapes ids #(assoc % :blocked true)))
;; Resize parent containers that need to ;; Resize parent containers that need to
(pcb/resize-parents parents)))) (pcb/resize-parents parents)
;; Remove parents when are a variant-container that becomes empty
(cond-> (seq empty-variant-cont)
(#(second (generate-delete-shapes % empty-variant-cont {})))))))
(defn change-show-in-viewer (defn change-show-in-viewer
[shape hide?] [shape hide?]

31
common/test/common_tests/logic/variants_test.cljc
View file

@ -9,6 +9,7 @@
[app.common.files.changes-builder :as pcb] [app.common.files.changes-builder :as pcb]
[app.common.geom.point :as gpt] [app.common.geom.point :as gpt]
[app.common.logic.libraries :as cll] [app.common.logic.libraries :as cll]
[app.common.logic.shapes :as cls]
[app.common.logic.variant-properties :as clvp] [app.common.logic.variant-properties :as clvp]
[app.common.test-helpers.components :as thc] [app.common.test-helpers.components :as thc]
[app.common.test-helpers.files :as thf] [app.common.test-helpers.files :as thf]
@ -234,3 +235,33 @@
(t/is (= (count (:components data')) 4)) (t/is (= (count (:components data')) 4))
(t/is (= (count objects) 4)) (t/is (= (count objects) 4))
(t/is (= (count objects') 7)))) (t/is (= (count objects') 7))))
(t/deftest test-delete-variant
;; When a variant container becomes empty, it id automatically deleted
(let [;; ==== Setup
file (-> (thf/sample-file :file1)
(thv/add-variant-two-properties :v01 :c01 :m01 :c02 :m02))
container (ths/get-shape file :v01)
m01-id (-> (ths/get-shape file :m01) :id)
m02-id (-> (ths/get-shape file :m02) :id)
page (thf/current-page file)
;; ==== Action
changes (-> (pcb/empty-changes nil)
(pcb/with-page-id (:id page))
(pcb/with-library-data (:data file))
(pcb/with-objects (:objects page))
(#(second (cls/generate-delete-shapes % #{m01-id m02-id} {}))))
file' (thf/apply-changes file changes)
;; ==== Get
container' (ths/get-shape file' :v01)]
;; ==== Check
;; The variant containew was not nil before the deletion
(t/is (not (nil? container)))
;; The variant containew is nil after the deletion
(t/is (nil? container'))))