github/penpot
1
0
Fork 0
mirror of https://github.com/penpot/penpot.git synced 2026-10-29 14:00:37 +00:00
Code Issues Packages Projects Releases Wiki Activity

🔧 Validate variant containers after moving shapes into them

Browse Source 
generate-make-shapes-variant does not validate, as its callers may run
it before the shapes are inside the container. Validate in the callers
instead, once the action is complete:

- generate-relocate validates the variant containers the variants come
  from (unless they are deleted for being empty) and the one they go
  into.
- prepare-restore-component validates the container when it restores a
  component into a variant.
- duplicate-variant validates the container after moving the new
  variant into it.

AI-assisted-by: claude-opus-5-5
This commit is contained in:
Eva Marco 2026-10-01 13:33:23 +02:00 committed by Andrés Moya
parent 31248ac115
commit e702b44121
4 changed files with 157 additions and 7 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

23
common/src/app/common/logic/libraries.cljc
View File

@ -494,6 +494,7 @@
(assoc :component-root true))
restoring-into-parent (get objects (:parent-id first-shape))
into-variant? (ctk/is-variant-container? restoring-into-parent)
changes (-> changes
(pcb/with-page page)
@ -512,9 +513,16 @@
(nil? restoring-into-parent)))
(clvp/generate-make-shapes-no-variant [first-shape])
;; Add variant info and rename when restoring into a variant-container
(ctk/is-variant-container? restoring-into-parent)
(clvp/generate-make-shapes-variant [first-shape] restoring-into-parent))]
{:changes (pcb/restore-component changes component-id (:id page) minusdelta)
into-variant?
(clvp/generate-make-shapes-variant [first-shape] restoring-into-parent))
changes (cond-> (pcb/restore-component changes component-id (:id page) minusdelta)
;; generate-make-shapes-variant does not validate, so validate the
;; container with the restored component inside
into-variant?
(pcb/validate-shapes (:id page)
[(:id restoring-into-parent)]
(str "prepare-restore-component: " component-id)))]
{:changes changes
:shape (first moved-shapes)})))
;; ---- General library synchronization functions ----
@ -2945,7 +2953,14 @@
(clvp/generate-update-property-value new-component-id (-> component :variant-properties count dec) value)
:always
(pcb/change-parent (:id parent) [shape] 0))]))
(pcb/change-parent (:id parent) [shape] 0)
;; generate-make-shapes-variant does not validate, so validate the
;; container once the shape is inside it
into-new-variant?
(pcb/validate-shapes page-id
[(:id parent)]
(str "duplicate-variant: " (:id component) " into " (:id parent))))]))
(defn generate-duplicate-component-change

21
common/src/app/common/logic/shapes.cljc
View File

@ -588,7 +588,19 @@
(conj to-delete (:id parent))
to-delete)))
#{}
(remove #(= % parent-id) all-parents))]
(remove #(= % parent-id) all-parents))
;; Variant containers changed by the move: the ones the variants come
;; from (unless they are deleted for being empty) and the one they go
;; into. The shapes taken out of a variant are validated by
;; generate-make-shapes-no-variant
variant-cont-to-validate
(cond-> (into #{}
(comp (keep :variant-id)
(remove empty-variant-cont))
variant-shapes)
(ctk/is-variant-container? parent)
(conj parent-id))]
(-> changes
;; Remove layout-item properties and tokens when moving a shape outside a layout
@ -701,7 +713,12 @@
;; Remove parents when are a variant-container that becomes empty
(cond-> (seq empty-variant-cont)
(#(second (generate-delete-shapes % empty-variant-cont {})))))))
(#(second (generate-delete-shapes % empty-variant-cont {}))))
(cond-> (seq variant-cont-to-validate)
(pcb/validate-shapes (pcb/get-page-id changes)
variant-cont-to-validate
(str "generate-relocate: " (vec ids) " to parent " parent-id))))))
(defn change-show-in-viewer
[shape hide?]

51
common/test/common_tests/logic/comp_creation_test.cljc
View File

@ -768,3 +768,54 @@
(t/is (some? m01'))
(t/is (nil? (:variant-id c01')))
(t/is (nil? (:variant-id m01')))))
(defn- validates-shapes?
"Check that some :validate-shapes change validates exactly `shape-ids`."
[changes shape-ids]
(some #(and (= :validate-shapes (:type %))
(= (set shape-ids) (set (:shape-ids %))))
(:redo-changes changes)))
(t/deftest test-restore-variant-into-its-container-validates-container
(let [;; ==== Setup
file (-> (thf/sample-file :file1)
(thv/add-variant :v01 :c01 :m01 :c02 :m02)
(delete-shapes [:m02]))
;; ==== Action
changes (restore-changes file :c02)
file' (thf/apply-changes file changes)]
;; ==== Check
(t/is (= (thi/id :v01) (:parent-id (ths/get-shape file' :m02))))
(t/is (validates-shapes? changes [(thi/id :v01)]))))
(t/deftest test-paste-variant-into-other-variant-validates-container
(let [;; ==== Setup
file (-> (thf/sample-file :file1)
(thv/add-variant :v01 :c01 :m01 :c02 :m02)
(thv/add-variant :v02 :c03 :m03 :c04 :m04)
(ths/update-shape :v02 :name "Other")
(ths/update-shape :m03 :name "Other")
(ths/update-shape :m04 :name "Other")
(thc/update-component :c03 {:name "Other"})
(thc/update-component :c04 {:name "Other"}))
page (thf/current-page file)
shape-id (thi/id :m01)
v02-id (thi/id :v02)
;; ==== Action
;; Paste a copy of m01 into v02, as the workspace does
objects (update (:objects page) shape-id assoc :parent-id v02-id :frame-id v02-id)
changes (-> (pcb/empty-changes nil)
(pcb/with-page-id (:id page))
(pcb/with-library-data (:data file))
(pcb/with-objects (:objects page))
(cll/generate-duplicate-changes objects page #{shape-id} (gpt/point 0 0)
{(:id file) file} (:data file) (:id file)))
file' (thf/apply-changes file changes)
v02' (ths/get-shape file' :v02)]
;; ==== Check
(t/is (= 3 (count (:shapes v02'))))
(t/is (validates-shapes? changes [v02-id]))))

69
common/test/common_tests/logic/move_shapes_test.cljc
View File

@ -8,11 +8,13 @@
(: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.compositions :as tho]
[app.common.test-helpers.files :as thf]
[app.common.test-helpers.ids-map :as thi]
[app.common.test-helpers.shapes :as ths]
[app.common.test-helpers.tokens :as tht]
[app.common.test-helpers.variants :as thv]
[app.common.types.tokens-lib :as ctob]
[app.common.types.tokens-status :as ctos]
[app.common.uuid :as uuid]
@ -216,4 +218,69 @@
(t/is (nil? (:layout-item-max-w circle')))
(t/is (nil? (:layout-item-min-w circle')))
(t/is (nil? (:layout-item-absolute circle')))
(t/is (nil? (:layout-item-z-index circle')))))
(t/is (nil? (:layout-item-z-index circle')))))
(defn- relocate-changes
[file shape-label parent-label]
(let [page (thf/current-page file)]
(cls/generate-relocate (-> (pcb/empty-changes nil)
(pcb/with-page-id (:id page))
(pcb/with-library-data (:data file))
(pcb/with-objects (:objects page)))
(thi/id parent-label)
0
#{(thi/id shape-label)})))
(defn- add-two-variants
[file]
(-> file
(thv/add-variant :v01 :c01 :m01 :c02 :m02)
(thv/add-variant :v02 :c03 :m03 :c04 :m04)
(ths/update-shape :v02 :name "Other")
(ths/update-shape :m03 :name "Other")
(ths/update-shape :m04 :name "Other")
(thc/update-component :c03 {:name "Other"})
(thc/update-component :c04 {:name "Other"})))
(t/deftest test-relocate-component-into-variant-validates-container
(let [;; ==== Setup
file (-> (thf/sample-file :file1)
(thv/add-variant :v01 :c01 :m01 :c02 :m02)
(tho/add-simple-component :c03 :m03 :r03))
page (thf/current-page file)
;; ==== Action
changes (relocate-changes file :m03 :v01)
file' (thf/apply-changes file changes)]
;; ==== Check
(t/is (= (thi/id :v01) (:variant-id (ths/get-shape file' :m03))))
(t/is (thf/validates-shapes-last? changes (:id page) [(thi/id :v01)]))))
(t/deftest test-relocate-variant-into-other-variant-validates-both-containers
(let [;; ==== Setup
file (add-two-variants (thf/sample-file :file1))
page (thf/current-page file)
;; ==== Action
changes (relocate-changes file :m01 :v02)
file' (thf/apply-changes file changes)]
;; ==== Check
(t/is (= (thi/id :v02) (:variant-id (ths/get-shape file' :m01))))
(t/is (thf/validates-shapes-last? changes (:id page) [(thi/id :v01) (thi/id :v02)]))))
(t/deftest test-relocate-variant-out-validates-old-container
(let [;; ==== Setup
file (-> (thf/sample-file :file1)
(thv/add-variant :v01 :c01 :m01 :c02 :m02)
(tho/add-frame :frame1))
page (thf/current-page file)
;; ==== Action
changes (relocate-changes file :m01 :frame1)
file' (thf/apply-changes file changes)]
;; ==== Check
(t/is (nil? (:variant-id (ths/get-shape file' :m01))))
(t/is (thf/validates-shapes-last? changes (:id page) [(thi/id :v01)]))))