github/penpot
1
0
Fork 0
mirror of https://github.com/penpot/penpot.git synced 2026-05-13 12:04:06 +00:00
Code Issues Packages Projects Releases Wiki Activity

🐛 Fix loss of swap slot in some cases of variant switch

Browse Source
This commit is contained in:
Andrés Moya 2026-04-24 14:32:24 +02:00
parent 08bd53b6a1
commit 4aa0f9754e
4 changed files with 206 additions and 22 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
CHANGES.md
View File

@ -144,6 +144,7 @@
- Fix plugin parse-point returning plain map instead of Point record (by @FairyPigDev) [Github #9129](https://github.com/penpot/penpot/pull/9129)
- Fix `:heigth` typo in clipboard frame-same-size? (by @iot2edge) [Github #9250](https://github.com/penpot/penpot/pull/9250)
- Fix Settings and Notifications "Update Settings" button enabled state when form has no changes (by @moorsecopers99) [Github #9090](https://github.com/penpot/penpot/issues/9090)
- Fix file crashing when switching a variant [Taiga #14014](https://tree.taiga.io/project/penpot/issue/14014)
## 2.15.0 (Unreleased)

4
common/src/app/common/files/changes_builder.cljc
View File

@ -46,8 +46,8 @@
(with-meta changes
{::page-id page-id})))
([]
{:redo-changes []
:undo-changes '()})
{:redo-changes [] ;; redo-changes is a vector so that conj adds things at the end, in order of execution
:undo-changes '()}) ;; undo-changes is a list to conj things at the beginning, so they execute in the reverse order when undoing several changes
([origin]
{:redo-changes []
:undo-changes '()

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

@ -2119,8 +2119,8 @@
(contains? #{:auto-height :auto-width} (:grow-type current-shape)))]
(loop [attrs updatable-attrs
roperations [{:type :set-touched :touched (:touched previous-shape)}]
uoperations (list {:type :set-touched :touched (:touched current-shape)})]
roperations []
uoperations '()]
(if-let [attr (first attrs)]
(let [sync-group
(ctk/resolve-sync-group (:type previous-shape) attr)
@ -2262,7 +2262,13 @@
(let [updated-attrs (into #{} (comp (filter #(= :set (:type %)))
(map :attr))
roperations)]
roperations)
updated-sync-groups (into #{}
(keep #(ctk/resolve-sync-group (:type previous-shape) %))
updated-attrs)
new-touched (set/union (or (:touched current-shape) #{}) updated-sync-groups)
roperations (into [{:type :set-touched :touched new-touched}] roperations)
uoperations (into (list {:type :set-touched :touched (:touched current-shape)}) uoperations)]
(cond-> changes
(> (count roperations) 1)
(-> (add-update-attr-changes current-shape container roperations uoperations)

211
common/test/common_tests/logic/variants_switch_test.cljc
View File

@ -14,6 +14,7 @@
[app.common.test-helpers.ids-map :as thi]
[app.common.test-helpers.shapes :as ths]
[app.common.test-helpers.variants :as thv]
[app.common.types.component :as ctk]
[clojure.test :as t]))
(t/use-fixtures :each thi/test-fixture)
@ -35,13 +36,13 @@
copy01 (ths/get-shape file :copy01)
;; ==== Action
file' (tho/swap-component-in-shape file :copy01 :c02 {:new-shape-label :copy02 :keep-touched? true})
file' (tho/swap-component-in-shape file :copy01 :c02 {:keep-touched? true})
copy01' (ths/get-shape file' :copy02)]
copy01' (ths/get-shape file' :copy01)]
(thf/dump-file file :keys [:width])
;; The copy had width 5 before the switch
(t/is (= (:width copy01) 5))
;; The rect has width 15 after the switch
;; The copy has width 15 after the switch
(t/is (= (:width copy01') 15))))
(t/deftest test-simple-switch
@ -61,15 +62,15 @@
rect01 (get-in page [:objects (-> copy01 :shapes first)])
;; ==== Action
file' (tho/swap-component-in-shape file :copy01 :c02 {:new-shape-label :copy02 :keep-touched? true})
file' (tho/swap-component-in-shape file :copy01 :c02 {:keep-touched? true})
page' (thf/current-page file')
copy02' (ths/get-shape file' :copy02)
rect02' (get-in page' [:objects (-> copy02' :shapes first)])]
copy01' (ths/get-shape file' :copy01)
rect01' (get-in page' [:objects (-> copy01' :shapes first)])]
;; The rect had width 5 before the switch
(t/is (= (:width rect01) 5))
;; The rect has width 15 after the switch
(t/is (= (:width rect02') 15))))
(t/is (= (:width rect01') 15))))
;; ============================================================
;; SIMPLE ATTRIBUTE OVERRIDES (identical variants)
@ -100,9 +101,9 @@
copy01 (ths/get-shape file :copy01)
;; ==== Action
file' (tho/swap-component-in-shape file :copy01 :c02 {:new-shape-label :copy02 :keep-touched? true})
file' (tho/swap-component-in-shape file :copy01 :c02 {:keep-touched? true})
copy01' (ths/get-shape file' :copy02)]
copy01' (ths/get-shape file' :copy01)]
(thf/dump-file file :keys [:width])
;; The copy had width 25 before the switch
(t/is (= (:width copy01) 25))
@ -137,16 +138,16 @@
rect01 (get-in page [:objects (:id rect01)])
;; ==== Action
file' (tho/swap-component-in-shape file :copy01 :c02 {:new-shape-label :copy02 :keep-touched? true})
file' (tho/swap-component-in-shape file :copy01 :c02 {:keep-touched? true})
page' (thf/current-page file')
copy02' (ths/get-shape file' :copy02)
rect02' (get-in page' [:objects (-> copy02' :shapes first)])]
copy01' (ths/get-shape file' :copy01)
rect01' (get-in page' [:objects (-> copy01' :shapes first)])]
;; The rect had width 25 before the switch
(t/is (= (:width rect01) 25))
;; The override is keept: The rect still has width 25 after the switch
(t/is (= (:width rect02') 25))))
(t/is (= (:width rect01') 25))))
;; ============================================================
;; SIMPLE ATTRIBUTE OVERRIDES (different variants)
@ -180,17 +181,193 @@
rect01 (get-in page [:objects (:id rect01)])
;; ==== Action
file' (tho/swap-component-in-shape file :copy01 :c02 {:new-shape-label :copy02 :keep-touched? true})
file' (tho/swap-component-in-shape file :copy01 :c02 {:keep-touched? true})
page' (thf/current-page file')
copy02' (ths/get-shape file' :copy02)
rect02' (get-in page' [:objects (-> copy02' :shapes first)])]
copy01' (ths/get-shape file' :copy01)
rect01' (get-in page' [:objects (-> copy01' :shapes first)])]
;; The rect had width 25 before the switch
(t/is (= (:width rect01) 25))
;; The override isn't keept, because the property is different in the mains
;; The rect has width 15 after the switch
(t/is (= (:width rect02') 15))))
(t/is (= (:width rect01') 15))))
;; ============================================================
;; NESTED COPY SWITCH (no overrides)
;; ============================================================
(t/deftest test-nested-switch-in-main
(let [;; ==== Setup
file (-> (thf/sample-file :file1)
(thv/add-variant
:v01 :c01 :m01 :c02 :m02
{:variant1-params {:width 5}
:variant2-params {:width 15}})
(tho/add-frame :m03)
(thc/instantiate-component :c01
:copy01
:parent-label :m03)
(thc/make-component :c03 :m03))
copy01 (ths/get-shape file :copy01)
;; ==== Action
file' (tho/swap-component-in-shape file :copy01 :c02 {:keep-touched? true})
copy01' (ths/get-shape file' :copy01)]
(thf/dump-file file :keys [:width])
;; The copy had width 5 before the switch
(t/is (= (:width copy01) 5))
;; The copy has width 15 after the switch
(t/is (= (:width copy01') 15))
;; The copy is not touched but has swap slot
(t/is (= (count (:touched copy01')) 1))
(t/is (= (ctk/get-swap-slot copy01') (thi/id :copy01)))))
(t/deftest test-nested-switch-in-copy
(let [;; ==== Setup
file (-> (thf/sample-file :file1)
(thv/add-variant
:v01 :c01 :m01 :c02 :m02
{:variant1-params {:width 5}
:variant2-params {:width 15}})
(tho/add-frame :m03)
(thc/instantiate-component :c01
:nested01
:parent-label :m03)
(thc/make-component :c03 :m03)
(thc/instantiate-component :c03
:nested02
:children-labels [:child01]))
child01 (ths/get-shape file :child01)
;; ==== Action
file' (tho/swap-component-in-shape file :child01 :c02 {:keep-touched? true})
child01' (ths/get-shape file' :child01)]
(thf/dump-file file :keys [:width])
;; The copy had width 5 before the switch
(t/is (= (:width child01) 5))
;; The copy has width 15 after the switch
(t/is (= (:width child01') 15))
;; The copy is not touched but has swap slot
(t/is (= (count (:touched child01')) 1))
(t/is (= (ctk/get-swap-slot child01') (thi/id :nested01)))))
;; ============================================================
;; NESTED COPY SWITCH (with overrides)
;; ============================================================
(t/deftest test-nested-switch-in-main-with-override
(let [;; ==== Setup
file (-> (thf/sample-file :file1)
(thv/add-variant
:v01 :c01 :m01 :c02 :m02
{:variant1-params {:width 5}
:variant2-params {:width 15}})
(tho/add-frame :m03)
(thc/instantiate-component :c01
:copy01
:parent-label :m03)
(thc/make-component :c03 :m03))
page (thf/current-page file)
fills (ths/sample-fills-color :fill-color "#fabada")
changes (cls/generate-update-shapes (pcb/empty-changes nil (:id page))
#{(thi/id :copy01)}
(fn [shape]
(assoc shape
:width 25
:fills fills))
(:objects page)
{})
file (thf/apply-changes file changes)
copy01 (ths/get-shape file :copy01)
;; ==== Action
file' (tho/swap-component-in-shape file :copy01 :c02 {:keep-touched? true})
copy01' (ths/get-shape file' :copy01)]
(thf/dump-file file :keys [:width :touched])
;; The copy had fill color before the switch
(t/is (= (:fills copy01) fills))
;; The copy still has fill color after the switch
(t/is (= (:fills copy01') fills))
;; The copy had width 25 before the switch
(t/is (= (:width copy01) 25))
;; The copy gets the switched variant width 15, because this is the value changed in the variant
(t/is (= (:width copy01') 15))
;; The copy is fills touched and has swap slot
(t/is (= (count (:touched copy01')) 2))
(t/is (= (ctk/get-swap-slot copy01') (thi/id :copy01)))
(t/is (contains? (:touched copy01') :fill-group))))
(t/deftest test-nested-switch-in-copy-with-override
(let [;; ==== Setup
file (-> (thf/sample-file :file1)
(thv/add-variant
:v01 :c01 :m01 :c02 :m02
{:variant1-params {:width 5}
:variant2-params {:width 15}})
(tho/add-frame :m03)
(thc/instantiate-component :c01
:nested01
:parent-label :m03)
(thc/make-component :c03 :m03)
(thc/instantiate-component :c03
:copy02
:children-labels [:nested02]))
page (thf/current-page file)
fills (ths/sample-fills-color :fill-color "#fabada")
changes (cls/generate-update-shapes (pcb/empty-changes nil (:id page))
#{(thi/id :nested02)}
(fn [shape]
(assoc shape
:width 25
:fills fills))
(:objects page)
{})
file (thf/apply-changes file changes)
nested02 (ths/get-shape file :nested02)
;; ==== Action
file' (tho/swap-component-in-shape file :nested02 :c02 {:keep-touched? true})
nested02' (ths/get-shape file' :nested02)]
(thf/dump-file file :keys [:width])
;; The copy had fill color before the switch
(t/is (= (:fills nested02) fills))
;; The copy still has fill color after the switch
(t/is (= (:fills nested02') fills))
;; The copy had width 5 before the switch
(t/is (not= (:width nested02) 5))
;; The copy gets the switched variant width 15, because this is the value changed in the variant
(t/is (= (:width nested02') 15))
;; The copy is fills touched and has swap slot
(t/is (= (count (:touched nested02')) 2))
(t/is (= (ctk/get-swap-slot nested02') (thi/id :nested01)))
(t/is (contains? (:touched nested02') :fill-group))))
;; ============================================================
;; TEXT OVERRIDES (identical variants)