From c88d9f568b983ef87724786700a2ebf8515af84a Mon Sep 17 00:00:00 2001 From: "Dr. Dominik Jain" Date: Mon, 6 Jul 2026 16:49:29 +0200 Subject: [PATCH] :bug: Preserve component order when combining components as variants (#10562) --- .../src/app/main/data/workspace/variants.cljs | 42 +++++++++++++++++-- frontend/src/app/plugins/api.cljs | 5 ++- frontend/src/app/plugins/shape.cljs | 11 ++++- .../plugins/shape_bugfixes_test.cljs | 20 +++++---- 4 files changed, 64 insertions(+), 14 deletions(-) diff --git a/frontend/src/app/main/data/workspace/variants.cljs b/frontend/src/app/main/data/workspace/variants.cljs index caacb88bc5..2d224facc2 100644 --- a/frontend/src/app/main/data/workspace/variants.cljs +++ b/frontend/src/app/main/data/workspace/variants.cljs @@ -612,7 +612,37 @@ (map first) vec)) +(defn- order-variant-children + "Reorders the children of a variant container so that the variant components + (as returned by cfv/find-variant-components, which reverses the children + vector) appear in the given order of ids. Ids that are not children of the + container are ignored; children not covered by ids keep their position at + the end." + [variant-id ordered-ids] + (ptk/reify ::order-variant-children + ptk/WatchEvent + (watch [it state _] + (let [page-id (:current-page-id state) + objects (dsh/lookup-page-objects state page-id) + children (set (dm/get-in objects [variant-id :shapes])) + ;; The desired :shapes vector is the reverse of ordered-ids; + ;; change-parent at index 0 inverts the order of the given shapes, + ;; so they are passed in ordered-ids order + shapes (->> ordered-ids + (filter children) + (mapv (d/getf objects)))] + (when (> (count shapes) 1) + (let [changes (-> (pcb/empty-changes it page-id) + (pcb/with-objects objects) + (pcb/change-parent variant-id shapes 0))] + (rx/of (dch/commit-changes changes) + (ptk/data-event :layout/update {:ids [variant-id]})))))))) + (defn combine-as-variants + "Combines the components identified by the main-instance ids `ids` into a + new variant container. If `ids` is a sequential collection, its order + determines the order of the resulting variant components; unordered + collections are normalized to the layer-tree order." [ids {:keys [page-id trigger variant-id]}] (ptk/reify ::combine-as-variants ptk/WatchEvent @@ -622,12 +652,15 @@ combine (fn [current-page] (let [objects (dsh/lookup-page-objects state current-page) - ids (->> ids + ids (->> (if (sequential? ids) + ids + (cfh/order-by-indexed-shapes objects ids)) (cfh/clean-loops objects) (remove (fn [id] (let [shape (get objects id)] (or (not (ctc/main-instance? shape)) - (ctc/is-variant? shape))))))] + (ctc/is-variant? shape))))) + (vec))] (when (> (count ids) 1) (let [shapes (mapv #(get objects %) ids) rect (bounding-rect shapes) @@ -657,7 +690,10 @@ (rx/of (dwu/start-undo-transaction undo-id) (transform-in-variant (first ids) variant-id delta prefix add-wrapper? false false) - (dwsh/relocate-shapes (into #{} (-> ids rest reverse)) variant-id 0) + (dwsh/relocate-shapes (into #{} (rest ids)) variant-id 0) + ;; relocate-shapes inserts the shapes in layer-tree + ;; order, so restore the intended order afterwards + (order-variant-children variant-id ids) (dwsh/update-shapes ids #(-> % (assoc :constraints-h :left) (assoc :constraints-v :top) diff --git a/frontend/src/app/plugins/api.cljs b/frontend/src/app/plugins/api.cljs index 3d3796a14e..7b8c5ef15a 100644 --- a/frontend/src/app/plugins/api.cljs +++ b/frontend/src/app/plugins/api.cljs @@ -687,9 +687,12 @@ :else (let [file-id (obj/get (first shapes) "$file") page-id (obj/get (first shapes) "$page") + ;; Keep the input order: it determines the order of the + ;; resulting variant components (see combine-as-variants) ids (->> shapes (map #(obj/get % "$id")) - (into #{})) + (distinct) + (vec)) ;; Check that every component is: ;; - in the same page diff --git a/frontend/src/app/plugins/shape.cljs b/frontend/src/app/plugins/shape.cljs index a535cad23b..2e9366941d 100644 --- a/frontend/src/app/plugins/shape.cljs +++ b/frontend/src/app/plugins/shape.cljs @@ -1644,8 +1644,15 @@ (u/not-valid plugin-id :ids ids) :else - (let [ids - (into #{id} (keep uuid/parse*) ids) + (let [;; Keep the input order (head shape first): it determines + ;; the order of the resulting variant components (see + ;; combine-as-variants) + ids + (into [id] + (comp (keep uuid/parse*) + (remove #{id}) + (distinct)) + ids) valid? (every? diff --git a/frontend/test/frontend_tests/plugins/shape_bugfixes_test.cljs b/frontend/test/frontend_tests/plugins/shape_bugfixes_test.cljs index bf9e77f325..c854e4942c 100644 --- a/frontend/test/frontend_tests/plugins/shape_bugfixes_test.cljs +++ b/frontend/test/frontend_tests/plugins/shape_bugfixes_test.cljs @@ -161,12 +161,13 @@ ;; `combine-as-variants` needs real main components and the variant pipeline, ;; so this stays at the proxy boundary and verifies the component ids that ;; the head proxy collects from its argument before delegating. - (let [file-id (uuid/next) - page-id (uuid/next) - head-id (uuid/next) - other-id (uuid/next) - proxy (shape/shape-proxy plugin-id file-id page-id head-id) - captured (atom nil)] + (let [file-id (uuid/next) + page-id (uuid/next) + head-id (uuid/next) + other-id (uuid/next) + third-id (uuid/next) + proxy (shape/shape-proxy plugin-id file-id page-id head-id) + captured (atom nil)] (with-redefs [u/locate-shape (fn [_file _page id] {:id id :component-id id}) u/locate-library-component (constantly {:id (uuid/next)}) ctk/is-variant? (constantly false) @@ -178,8 +179,11 @@ {:event :combine-as-variants}) st/emit! mock/noop shape/shape-proxy (mock/stub (fn [& _] #js {}))] - (.combineAsVariants proxy #js [(str other-id)]) - (t/is (= #{head-id other-id} (:ids @captured)))))) + ;; the collected ids must be ordered (head shape first, then the passed + ;; ids in their given order, deduplicated), since the order determines + ;; the order of the resulting variant components + (.combineAsVariants proxy #js [(str other-id) (str head-id) (str third-id) (str other-id)]) + (t/is (= [head-id other-id third-id] (:ids @captured)))))) (t/deftest remove-ruler-guide-deletes-the-guide-from-the-page ;; Adds a real ruler guide through the API and asserts it is gone from the