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

🐛 Fix error when copy & paste a swapped copy (#9934)

Browse Source
This commit is contained in:
Andrés Moya 2026-05-29 13:36:17 +02:00 committed by GitHub
parent 6e8d2b3708
commit 7e6884e330
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 141 additions and 22 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

11
common/src/app/common/test_helpers/files.cljc
View File

@ -53,9 +53,14 @@
(defn validate-file!
([file] (validate-file! file {}))
([file libraries]
(cfv/validate-file-schema! file)
(cfv/validate-file! file libraries)
file))
(try
(cfv/validate-file-schema! file)
(cfv/validate-file! file libraries)
file
(catch #?(:clj Exception :cljs :default) e
(println "File validation failed: " (ex-message e))
(pprint (ex-data e))
(throw e)))))
(defn apply-changes
[file changes & {:keys [validate?] :or {validate? true}}]

18
frontend/src/app/main/data/workspace/clipboard.cljs
View File

@ -21,7 +21,7 @@
[app.common.logic.shapes :as cls]
[app.common.schema :as sm]
[app.common.transit :as t]
[app.common.types.component :as ctc]
[app.common.types.component :as ctk]
[app.common.types.container :as ctn]
[app.common.types.file :as ctf]
[app.common.types.shape :as cts]
@ -137,7 +137,7 @@
(some? images)
(update :images into images)
(ctc/is-variant-container? item)
(ctk/is-variant-container? item)
(update :variant-properties merge (collect-variants state item))))
(maybe-translate [shape objects parent-frame-id]
@ -160,8 +160,10 @@
heads))))
(advance-copy [file libraries page objects shape]
(if (and (ctc/instance-head? shape) (not (ctc/main-instance? shape)))
(let [level-delta (ctn/get-nesting-level-delta (:objects page) shape uuid/zero)]
(if (and (ctk/instance-head? shape) (not (ctk/main-instance? shape)))
(let [level-delta (if (nil? (ctk/get-swap-slot shape))
(ctn/get-nesting-level-delta (:objects page) shape uuid/zero)
0)]
(if (pos? level-delta)
(reduce (partial advance-shape file libraries page level-delta)
objects
@ -948,10 +950,10 @@
add-component-to-variant? (and
;; Any of the shapes is a head
(some ctc/instance-head? orig-shapes)
(some ctk/instance-head? orig-shapes)
;; Any ancestor of the destination parent is a variant
(->> (cfh/get-parents-with-self page-objects parent-id)
(some ctc/is-variant?)))
(some ctk/is-variant?)))
undo-id (js/Symbol)]
(rx/concat
@ -965,13 +967,13 @@
;; NOTE: we don't emit the create-shape event all the time for
;; avoid send a lot of events (that are not necessary); this
;; decision is made explicitly by the responsible team.
(if (ctc/instance-head? shape)
(if (ctk/instance-head? shape)
(ev/event {::ev/name "use-library-component"
::ev/origin origin
:is-external-library external-lib?
:type (get shape :type)
:parent-type parent-type
:is-variant (ctc/is-variant? component)})
:is-variant (ctk/is-variant? component)})
(if (cfh/has-layout? objects (:parent-id shape))
(ev/event {::ev/name "layout-add-element"
::ev/origin origin

4
frontend/test/frontend_tests/helpers/pages.cljs
View File

@ -238,7 +238,9 @@
(advance-copy [file libraries page objects shape]
(if (and (ctk/instance-head? shape) (not (ctk/main-instance? shape)))
(let [level-delta (ctn/get-nesting-level-delta (:objects page) shape uuid/zero)]
(let [level-delta (if (nil? (ctk/get-swap-slot shape))
(ctn/get-nesting-level-delta (:objects page) shape uuid/zero)
0)]
(if (pos? level-delta)
(reduce (partial advance-shape file libraries page level-delta)
objects

130
frontend/test/frontend_tests/logic/copying_and_duplicating_test.cljs
View File

@ -16,6 +16,7 @@
[app.main.data.workspace :as dw]
[app.main.data.workspace.colors :as dc]
[app.main.data.workspace.libraries :as dwl]
[app.main.data.workspace.pages :as dwp]
[app.main.data.workspace.selection :as dws]
[cljs.test :as t :include-macros true]
[frontend-tests.helpers.pages :as thp]
@ -152,7 +153,8 @@
(ths/run-store
store done events
(fn [new-state]
(let [file' (ths/get-file-from-state new-state)]
(let [file' (ths/get-file-from-state new-state)]
(cthf/validate-file! file')
(t/is (= (count-shapes file' "rect-simple-1" "#111111") 18)))))))))
(t/deftest main-and-first-level-copy-2
@ -178,7 +180,8 @@
(ths/run-store
store done events
(fn [new-state]
(let [file' (ths/get-file-from-state new-state)]
(let [file' (ths/get-file-from-state new-state)]
(cthf/validate-file! file')
(t/is (= (count-shapes file' "rect-simple-1" "#222222") 15)))))))))
(t/deftest main-and-first-level-copy-3
@ -205,10 +208,10 @@
(ths/run-store
store done events
(fn [new-state]
(let [file' (ths/get-file-from-state new-state)]
(let [file' (ths/get-file-from-state new-state)]
(cthf/validate-file! file')
(t/is (= (count-shapes file' "rect-simple-1" "#333333") 12)))))))))
(t/deftest main-and-first-level-copy-4
(t/async
done
@ -234,7 +237,8 @@
(ths/run-store
store done events
(fn [new-state]
(let [file' (ths/get-file-from-state new-state)]
(let [file' (ths/get-file-from-state new-state)]
(cthf/validate-file! file')
(t/is (= (count-shapes file' "rect-simple-1" "#444444") 6)))))))))
(t/deftest copy-nested-in-main-1
@ -258,6 +262,7 @@
store done events
(fn [new-state]
(let [file' (ths/get-file-from-state new-state)]
(cthf/validate-file! file')
;; Check propagation to all copies.
(t/is (= (count-shapes file' "rect-simple-1" "#111111") 28)))))))))
@ -282,6 +287,7 @@
store done events
(fn [new-state]
(let [file' (ths/get-file-from-state new-state)]
(cthf/validate-file! file')
;; Check propagation to duplicated.
(t/is (= (count-shapes file' "rect-simple-1" "#222222") 9)))))))))
@ -306,6 +312,7 @@
store done events
(fn [new-state]
(let [file' (ths/get-file-from-state new-state)]
(cthf/validate-file! file')
;; Check that it's NOT PROPAGATED.
(t/is (= (count-shapes file' "rect-simple-1" "#333333") 2)))))))))
@ -331,10 +338,10 @@
store done events
(fn [new-state]
(let [file' (ths/get-file-from-state new-state)]
(cthf/validate-file! file')
;; Check propagation to all copies.
(t/is (= (count-shapes file' "rect-simple-1" "#111111") 20)))))))))
(t/deftest copy-nested-2
(t/async
done
@ -360,12 +367,12 @@
store done events
(fn [new-state]
(let [file' (ths/get-file-from-state new-state)]
(cthf/validate-file! file')
;; Check that it's NOT PROPAGATED.
(t/is (= (count-shapes file' "rect-simple-1" "#111111") 11))
(t/is (= (count-shapes file' "rect-simple-1" "#222222") 7))
(t/is (= (count-shapes file' "rect-simple-1" "#333333") 2)))))))))
(t/deftest copy-nested-3
(t/async done
(with-redefs [uuid/next cthi/next-uuid]
@ -391,6 +398,7 @@
(fn [new-state]
(let [file' (-> (ths/get-file-from-state new-state)
(cthf/switch-to-page :page-2))]
(cthf/validate-file! file')
;; Check that it's NOT PROPAGATED.
(t/is (= (count-shapes file' "rect-simple-1" "#111111") 10))
(t/is (= (count-shapes file' "rect-simple-1" "#222222") 4))
@ -420,7 +428,7 @@
(cthc/make-component :component-1 :frame-1))
page-id (cthf/current-page-id file)
store (ths/setup-store file)
events [(app.main.data.workspace.pages/duplicate-page page-id)]]
events [(dwp/duplicate-page page-id)]]
(ths/run-store
store done events
(fn [new-state]
@ -432,8 +440,7 @@
new-objects (:objects new-page)
group (some #(when (= (:name %) "group-1") %) (vals new-objects))
frame (some #(when (= (:name %) "frame-1") %) (vals new-objects))
shape (some #(when (= (:name %) "shape-1") %) (vals new-objects))
component-ids (map :component-id (filter :component-root (vals new-objects)))]
shape (some #(when (= (:name %) "shape-1") %) (vals new-objects))]
(t/is group "Group exists in duplicated page")
(t/is frame "Frame exists in duplicated page")
@ -452,3 +459,106 @@
(str "Duplicated page contains an instance of the original main component (component-id: " component-id ")")))
(done))))))))
(defn- setup-swapped-copies-file
"Creates a file with a component with two levels of nested copies inside. The component
has one copy, and inside it, the topmost nested copy is swapped with a second component,
also with one nested copy inside.
{:frame-simple-1} [:name Frame1] # [Component :simple-1]
:rect-simple-1 [:name Rect1]
{:frame-composed-1} [:name frame-composed-1] # [Component :composed-1]
:copy-simple-1-in-composed-1 [:name Frame1] @--> frame-simple-1
<no-label #000894> [:name Rect1] ---> rect-simple-1
{:frame-composed-2} [:name frame-composed-2] # [Component :composed-2]
:copy-composed-1-in-composed-2 [:name frame-composed-1] @--> frame-composed-1
<no-label #000899> [:name Frame1] @--> copy-simple-1-in-composed-1
<no-label #00089a> [:name Rect1] ---> <no-label #000894>
{:frame-simple-2} [:name Frame1] # [Component :simple-2]
:rect-simple-2 [:name Rect1]
{:frame-composed-3} [:name frame-composed-3] # [Component :composed-3]
:copy-simple-2-in-composed-3 [:name Frame1] @--> frame-simple-2
<no-label #0008a4> [:name Rect1] ---> rect-simple-2
:copy-composed-2 [:name frame-composed-2] #--> [Component :composed-2] frame-composed-2
:swapped-composed-3 [:name frame-composed-3, :swap-slot-label :copy-composed-1-in-composed-2] @--> frame-composed-3
:swapped-simple-2-frame [:name Frame1] @--> copy-simple-2-in-composed-3
:swapped-simple-2-rect [:name Rect1] ---> <no-label #0008a4>
"
[]
(-> (cthf/sample-file :file1 :page-label :page-1)
;; 1. Simple component :simple-1
(ctho/add-simple-component :simple-1 :frame-simple-1 :rect-simple-1)
;; 2. Composed component :composed-1 containing a copy of :simple-1
(ctho/add-frame :frame-composed-1 :name "frame-composed-1")
(cthc/instantiate-component :simple-1 :copy-simple-1-in-composed-1
:parent-label :frame-composed-1)
(cthc/make-component :composed-1 :frame-composed-1)
;; 3. Composed component :composed-2 containing a copy of :composed-1
(ctho/add-frame :frame-composed-2 :name "frame-composed-2")
(cthc/instantiate-component :composed-1 :copy-composed-1-in-composed-2
:parent-label :frame-composed-2)
(cthc/make-component :composed-2 :frame-composed-2)
;; 4. Simple component :simple-2
(ctho/add-simple-component :simple-2 :frame-simple-2 :rect-simple-2)
;; 5. Composed component :composed-3 containing a copy of :simple-2
(ctho/add-frame :frame-composed-3 :name "frame-composed-3")
(cthc/instantiate-component :simple-2 :copy-simple-2-in-composed-3
:parent-label :frame-composed-3)
(cthc/make-component :composed-3 :frame-composed-3)
;; 6. A copy of :composed-2
(cthc/instantiate-component :composed-2 :copy-composed-2
:children-labels [:nested-copy-composed-1])
;; 7. Swap the nested copy of :composed-1 in the copy of :composed-2 with :composed-3
(cthc/component-swap :nested-copy-composed-1 :composed-3 :swapped-composed-3
:children-labels [:swapped-simple-2-frame :swapped-simple-2-rect])))
(t/deftest duplicate-swapped-copies
(t/async done
(with-redefs [uuid/next cthi/next-uuid]
(let [;; ==== Setup
file (setup-swapped-copies-file)
store (ths/setup-store file)
;; ==== Action
;; Copy to the clipboard all the shapes in the swapped copy one by one,
;; and paste them outside the copy, under uuid/zero
events (concat (copy-paste-shape :copy-composed-2 file :target-container-id uuid/zero)
(copy-paste-shape :swapped-composed-3 file :target-container-id uuid/zero)
(copy-paste-shape :swapped-simple-2-frame file :target-container-id uuid/zero)
(copy-paste-shape :swapped-simple-2-rect file :target-container-id uuid/zero))]
(ths/run-store
store done events
(fn [new-state]
(let [file' (ths/get-file-from-state new-state)
page' (cthf/current-page file')]
(cthf/validate-file! file')
;; ==== Check
;; Shape count breakdown (including page root):
;; page root: 1
;; :simple-1 main: 2 shapes (frame + rect)
;; :composed-1 main: 3 shapes (frame + instance-of-simple-1 + its child)
;; :composed-2 main: 4 shapes (frame + instance-of-composed-1 + its descendants)
;; :simple-2 main: 2 shapes (frame + rect)
;; :composed-3 main: 3 shapes (frame + instance-of-simple-2 + its child)
;; copy of :composed-2 (with swapped child): 4 shapes
;; pasted copy of :copy-composed-2: 4 shapes
;; pasted copy of :composed-3: 3 shapes
;; pasted copy of :swapped-simple-2-frame: 2 shapes
;; pasted copy of :swapped-simple-2-rect: 1 shapes
;; Total = 1 + 2 + 3 + 4 + 2 + 3 + 4 + 4 + 3 + 2 + 1 = 29
(t/is (= (count (:objects page')) 29)))))))))