From ce62e11626c9483e22fff883cfc7599b1dc4bd27 Mon Sep 17 00:00:00 2001 From: Alejandro Alonso Date: Wed, 16 Jul 2025 08:52:18 +0200 Subject: [PATCH] :bug: Fix error on validating file referential integrity when duplicating a page --- CHANGES.md | 3 +- common/src/app/common/types/container.cljc | 7 +-- .../src/app/main/data/workspace/pages.cljs | 7 ++- .../logic/copying_and_duplicating_test.cljs | 58 +++++++++++++++++++ 4 files changed, 67 insertions(+), 8 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index ed2f153eb3..10549bc30b 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -32,10 +32,11 @@ - Fix toggle focus mode did not restore viewport and selection upon exit [GitHub #6280](https://github.com/penpot/penpot/issues/6820) - Fix problem when creating a layout from an existing layout [Taiga #11554](https://tree.taiga.io/project/penpot/issue/11554) - Fix title button from Title Case to Capitalize [Taiga #11476](https://tree.taiga.io/project/penpot/issue/11476) -- Fix Main component receives focus and is selected when using 'Show Main Component' [Taiga #11402](https://tree.taiga.io/project/penpot/issue/11402) - Fix touchpad swipe leading to navigating back/forth [GitHub #4246](https://github.com/penpot/penpot/issues/4246) - Keep color data when copying from info tab into CSS [Taiga #11144](https://tree.taiga.io/project/penpot/issue/11144) - Update HSL values to modern syntax as defined in W3C CSS Color Module Level 4 [Taiga #11144](https://tree.taiga.io/project/penpot/issue/11144) +- Fix main component receives focus and is selected when using 'Show Main Component' [Taiga #11402](https://tree.taiga.io/project/penpot/issue/11402) +- Fix duplicating pages with mainInstance shapes nested inside groups [Taiga #10774](https://tree.taiga.io/project/penpot/issue/10774) ## 2.8.0 diff --git a/common/src/app/common/types/container.cljc b/common/src/app/common/types/container.cljc index 6ca767f7cc..89e210d871 100644 --- a/common/src/app/common/types/container.cljc +++ b/common/src/app/common/types/container.cljc @@ -294,8 +294,8 @@ ([page component library-data position] (make-component-instance page component library-data position {})) ([page component library-data position - {:keys [main-instance? force-id force-frame-id keep-ids?] - :or {main-instance? false force-id nil force-frame-id nil keep-ids? false}}] + {:keys [main-instance? force-id force-frame-id keep-ids? force-parent-id] + :or {main-instance? false force-id nil force-frame-id nil keep-ids? false force-parent-id nil}}] (let [component-page (ctpl/get-page library-data (:main-instance-page component)) component-shape (-> (get-shape component-page (:main-instance-id component)) @@ -303,7 +303,6 @@ (assoc :frame-id uuid/zero) (remove-swap-keep-attrs)) - orig-pos (gpt/point (:x component-shape) (:y component-shape)) delta (gpt/subtract position orig-pos) @@ -368,7 +367,7 @@ [new-shape new-shapes _] (ctst/clone-shape component-shape - frame-id + (or force-parent-id frame-id) (:objects component-page) :update-new-shape update-new-shape :force-id force-id diff --git a/frontend/src/app/main/data/workspace/pages.cljs b/frontend/src/app/main/data/workspace/pages.cljs index 7cd29def67..73cbcdb1bb 100644 --- a/frontend/src/app/main/data/workspace/pages.cljs +++ b/frontend/src/app/main/data/workspace/pages.cljs @@ -174,12 +174,15 @@ add-component-copy (fn [objs id shape] (let [component (ctkl/get-component fdata (:component-id shape)) + parent-id (when (not= (:parent-id shape) uuid/zero) (:parent-id shape)) [new-shape new-shapes] (ctn/make-component-instance page component fdata (gpt/point (:x shape) (:y shape)) - {:keep-ids? true :force-frame-id (:frame-id shape)}) + {:keep-ids? true + :force-frame-id (:frame-id shape) + :force-parent-id parent-id}) children (into {} (map (fn [shape] [(:id shape) shape]) new-shapes)) objs (assoc objs id new-shape)] (merge objs children))) @@ -201,10 +204,8 @@ (assoc :id id) (assoc :objects objects)) - changes (-> (pcb/empty-changes it) (pcb/add-page id page))] - (rx/of (dch/commit-changes changes)))))) (s/def ::rename-page diff --git a/frontend/test/frontend_tests/logic/copying_and_duplicating_test.cljs b/frontend/test/frontend_tests/logic/copying_and_duplicating_test.cljs index a6a59047a3..1ea6f05ee3 100644 --- a/frontend/test/frontend_tests/logic/copying_and_duplicating_test.cljs +++ b/frontend/test/frontend_tests/logic/copying_and_duplicating_test.cljs @@ -11,6 +11,7 @@ [app.common.test-helpers.files :as cthf] [app.common.test-helpers.ids-map :as cthi] [app.common.test-helpers.shapes :as cths] + [app.common.types.component :as ctk] [app.common.uuid :as uuid] [app.main.data.workspace :as dw] [app.main.data.workspace.colors :as dc] @@ -394,3 +395,60 @@ (t/is (= (count-shapes file' "rect-simple-1" "#111111") 10)) (t/is (= (count-shapes file' "rect-simple-1" "#222222") 4)) (t/is (= (count-shapes file' "rect-simple-1" "#333333") 0))))))))) + +(t/deftest duplicate-page-integrity-frame-group-component + ;; This test covers the bug fixed in 2.9.0: duplicating a page with a mainInstance inside a group + ;; must preserve parent/child referential integrity (parent-id and :shapes). + ;; + ;; Structure created: + ;; Page + ;; └─ Group ("group-1") + ;; └─ Main component ("frame-1") + ;; └─ Shape ("shape-1") + ;; The frame is also promoted to a component (main). + ;; + ;; The test checks: + ;; - The group, frame, and shape exist in the duplicated page. + ;; - The parent/child relationships are correct (group:shapes contains frame, frame:shapes contains shape, etc). + ;; - The duplicated page contains an instance of the component whose main is in the original page. + (t/async done + (with-redefs [uuid/next cthi/next-uuid] + (let [file (-> (cthf/sample-file :file1 :page-label :page-1) + (ctho/add-group :group-1 {:name "group-1"}) + (ctho/add-frame :frame-1 :parent-label :group-1 {:name "frame-1"}) + (cths/add-sample-shape :shape-1 :parent-label :frame-1 {:name "shape-1"}) + (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)]] + (ths/run-store + store done events + (fn [new-state] + (let [file' (ths/get-file-from-state new-state) + pages-vec (get-in file' [:data :pages]) + pages-index (get-in file' [:data :pages-index]) + new-page-id (first (remove #(= page-id %) pages-vec)) + new-page (get pages-index new-page-id) + 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)))] + + (t/is group "Group exists in duplicated page") + (t/is frame "Frame exists in duplicated page") + (t/is shape "Shape exists in duplicated page") + (t/is (some #(= (:id frame) %) (:shapes group)) "Group's :shapes contains frame's id") + (t/is (some #(= (:id shape) %) (:shapes frame)) "Frame's :shapes contains shape's id") + (t/is (= (:parent-id frame) (:id group)) "Frame's parent is group") + (t/is (= (:parent-id shape) (:id frame)) "Shape's parent is frame") + + ;; Check the duplicated page must contain an instance of the component whose main is in the original page + (let [original-page (get pages-index page-id) + original-main (some #(when (:component-root %) %) (vals (:objects original-page))) + instance (some #(when (:component-root %) %) (vals (:objects new-page))) + component-id (:component-id original-main)] + (t/is (ctk/instance-of? instance (:id file) component-id) + (str "Duplicated page contains an instance of the original main component (component-id: " component-id ")"))) + + (done))))))))