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 the variant after adding a new property

Browse Source 
generate-add-new-property changes all the components of the variant, so
it now validates the variant container.

generate-make-shapes-variant calls it while some shapes of the container
may not be variants yet, e.g. when a component is restored into the
container, which gave a false :main-instance-not-a-variant. Reordering
the calls changes the resulting property names, so add a
:skip-validation? option instead. generate-make-shapes-variant uses it
and does not validate on its own: its callers validate the container
once the shapes are inside 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 16c381ccef
commit 31248ac115
2 changed files with 59 additions and 6 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

22
common/src/app/common/logic/variant_properties.cljc
View File

@ -174,8 +174,11 @@
The value may also be given, or else it will be an empty value, unless fill-values? is true, in
which case the value will be set to a default value based on the property number.
The editing? flag, if set, will be added to the metadata of the properties, for later use."
[changes variant-id & {:keys [fill-values? editing? property-name property-value]}]
The editing? flag, if set, will be added to the metadata of the properties, for later use.
The skip-validation? flag is for callers that add the property while some shapes of the
container are not variants yet. They must validate the container themselves when done."
[changes variant-id & {:keys [fill-values? editing? property-name property-value skip-validation?]}]
(let [data (pcb/get-library-data changes)
objects (pcb/get-objects changes)
related-components (cfv/find-variant-components data objects variant-id)
@ -215,7 +218,13 @@
(pcb/update-shapes [main-id] #(update % :variant-name update-name)))]))
[1 changes]
related-components)]
changes))
;; All components of the variant change, so validate the container
(cond-> changes
(and (seq related-components) (not skip-validation?))
(pcb/validate-shapes (pcb/get-page-id changes)
[variant-id]
(str "generate-add-new-property: " variant-id
" name: " property-name)))))
(defn- generate-make-shape-no-variant
[changes shape]
@ -268,7 +277,10 @@
(defn generate-make-shapes-variant
"Introduce some components into a variant, adding the variant-id and variant-name to the
main instances and the variant-id and variant-properties to the components."
main instances and the variant-id and variant-properties to the components.
It does not validate the shapes, as callers may run it before the shapes are inside the
container. The callers must validate the container when done."
[changes shapes variant-container]
(let [data (pcb/get-library-data changes)
objects (pcb/get-objects changes)
@ -310,7 +322,7 @@
(- total-props num-base-props))
changes (nth
(iterate #(generate-add-new-property % variant-id) changes)
(iterate #(generate-add-new-property % variant-id :skip-validation? true) changes)
num-new-props)
changes (pcb/update-shapes changes (map :id shapes)

43
common/test/common_tests/logic/variant_properties_test.cljc
View File

@ -527,7 +527,23 @@
(t/is (= (count (:variant-properties comp01')) 2))
(t/is (= (count (:variant-properties comp02)) 1))
(t/is (= (count (:variant-properties comp02')) 2))
(t/is (= (-> comp01' :variant-properties last :value) ""))))
(t/is (= (-> comp01' :variant-properties last :value) ""))
(t/is (thf/validates-shapes-last? changes (:id page) [v-id]))))
(t/deftest test-add-new-property-skip-validation
(let [file (-> (thf/sample-file :file1)
(thv/add-variant :v01 :c01 :m01 :c02 :m02))
v-id (-> (ths/get-shape file :v01) :id)
page (thf/current-page file)
changes (-> (pcb/empty-changes nil)
(pcb/with-page-id (:id page))
(pcb/with-library-data (:data file))
(pcb/with-objects (:objects page))
(clvp/generate-add-new-property v-id :skip-validation? true))]
(t/is (seq (:redo-changes changes)))
(t/is (not-any? #(= :validate-shapes (:type %)) (:redo-changes changes)))))
(t/deftest test-add-new-property-with-values
(let [;; ==== Setup
@ -730,3 +746,28 @@
(t/is (= "Property 1" (-> c03' :variant-properties first :name)))
(t/is (= "Frame1" (-> c03' :variant-properties first :value)))
(t/is (= "Frame1" (-> m03' :variant-name)))))
(t/deftest test-make-shapes-variant-with-new-properties-does-not-validate
;; The shape is already inside the container but it is not a variant yet,
;; as when a component is restored into a variant. The function adds a new
;; property to the siblings while the shape is not a variant, so it must
;; not validate: the caller validates the container when done
(let [file (-> (thf/sample-file :file1)
(thv/add-variant :v01 :c01 :m01 :c02 :m02)
(tho/add-simple-component :c03 :m03 :sh03
:root-params {:parent-label :v01
:name "Big / Red"}))
page (thf/current-page file)
changes (-> (pcb/empty-changes nil)
(pcb/with-page-id (:id page))
(pcb/with-library-data (:data file))
(pcb/with-objects (:objects page))
(clvp/generate-make-shapes-variant [(ths/get-shape file :m03)]
(ths/get-shape file :v01)))
file' (thf/apply-changes file changes)]
(t/is (not-any? #(= :validate-shapes (:type %)) (:redo-changes changes)))
(t/is (= 2 (count (:variant-properties (thc/get-component file' :c01)))))
(t/is (= 2 (count (:variant-properties (thc/get-component file' :c03)))))))