From edf146db7b22ca14bb1789f38ae01e7a6300a297 Mon Sep 17 00:00:00 2001 From: makesomethingshit Date: Thu, 17 Sep 2026 21:09:32 +0900 Subject: [PATCH] :bug: Preserve source order when changing grid flow (#11662) * :bug: Reflow auto grid cells on flow direction change Remap only single-span auto cells to the new :layout-grid-dir traversal order, keeping source order, manual and area placements untouched. Update both grid direction controls to use the new change-grid-direction event and refresh the stale active button on persisted direction. Add a RED-to-GREEN model regression covering a 2x2 row-to-column transition and source-order. AI-assisted-by: muse-spark Signed-off-by: makesomethingshit * :bug: Keep source order on grid flow change with areas Skip the generic grid cell pass for the direction event, since reflowing already places every eligible auto item and a blind reorder rewrites shapes around pinned areas. Pin area/span grids with a regression test covering direction change and source order. AI-assisted-by: muse-spark Signed-off-by: makesomethingshit * :bug: Scope grid skip to direction changes only Replace the translation flag with a narrow skip-grid-reassignment option so component sync and reflow metadata stay intact while the generic grid cell pass is skipped. AI-assisted-by: muse-spark Signed-off-by: makesomethingshit * :bug: Clear leftover auto cells on grid flow change Write remapped shapes to every target auto cell and empty leftover cells so sparse grids cannot duplicate a child across target cells. Manual, area and spanned cells stay untouched; source order is kept. AI-assisted-by: muse-spark Signed-off-by: makesomethingshit * :bug: Pin grid flow invariants with span and manual regressions Keep the direction-change design unchanged and lock the claimed invariants with tests: a real 2x1 manual span cell and an occupied manual cell stay byte-identical, row->column->row round-trips to the original cells, and a mixed auto/manual/span/area grid shows no shape loss or duplication. Also drop the unused page-objects binding from the direction-change watcher. AI-assisted-by: muse-spark Signed-off-by: makesomethingshit * :bug: Unoverlap mixed grid fixture and assert movement Move auto C to (1,3) so it no longer overlaps the 2x1 manual span at (1,2). Row auto order A,C,B,E becomes column order A,B,E,C; assert the exact placement while keeping pinned, source-order and no-loss checks. Test-only change. AI-assisted-by: muse-spark Signed-off-by: makesomethingshit * :bug: Unify plugin dir setter and normalize missing direction Route GridLayoutProxy.dir through change-grid-direction so the plugin API shares the UI direction-change path with its reflow and source-order guarantees. Normalize a missing :layout-grid-dir to :row at the change-grid-direction entry point and cover it with a missing-direction regression plus a plugin setter routing regression. AI-assisted-by: muse-spark Signed-off-by: makesomethingshit * :bug: Fix grid plugin dir setter syntax Signed-off-by: makesomethingshit AI-assisted-by: opencode-go/muse-spark-1.3-contributor * :bug: Fix comments and tests --------- Signed-off-by: makesomethingshit Co-authored-by: alonso.torres --- common/src/app/common/logic/shapes.cljc | 8 +- common/src/app/common/types/shape/layout.cljc | 34 +++++++ .../common_tests/types/shape_layout_test.cljc | 90 +++++++++++++++++++ .../app/main/data/workspace/shape_layout.cljs | 30 +++++++ .../src/app/main/data/workspace/shapes.cljs | 10 ++- .../options/menus/layout_container.cljs | 6 +- frontend/src/app/plugins/grid.cljs | 2 +- .../frontend_tests/plugins/grid_test.cljs | 15 ++++ 8 files changed, 184 insertions(+), 11 deletions(-) diff --git a/common/src/app/common/logic/shapes.cljc b/common/src/app/common/logic/shapes.cljc index 638b5abd7c..43797c6e17 100644 --- a/common/src/app/common/logic/shapes.cljc +++ b/common/src/app/common/logic/shapes.cljc @@ -75,7 +75,7 @@ (reduce check-shape changes mod-obj-changes))) (defn generate-update-shapes - [changes ids update-fn objects {:keys [attrs changed-sub-attr ignore-tree ignore-touched with-objects? translation?]}] + [changes ids update-fn objects {:keys [attrs changed-sub-attr ignore-tree ignore-touched with-objects? translation? skip-grid-reassignment?]}] (let [changes (reduce (fn [changes id] (let [opts {:attrs attrs @@ -86,9 +86,9 @@ (cond-> changes (some? objects) (pcb/with-objects objects)) ids) - ;; Translation doesn't shift children between grid cells, so - ;; cell reassignment + child reorder are no-ops. - grid-ids (when-not translation? + ;; Translation keeps cell assignments; direction changes reflow them + ;; explicitly. + grid-ids (when-not (or translation? skip-grid-reassignment?) (->> ids (filter (partial ctl/grid-layout? objects)))) changes (cond-> changes (seq grid-ids) diff --git a/common/src/app/common/types/shape/layout.cljc b/common/src/app/common/types/shape/layout.cljc index b66aabc27d..1f3c215ab8 100644 --- a/common/src/app/common/types/shape/layout.cljc +++ b/common/src/app/common/types/shape/layout.cljc @@ -1557,6 +1557,40 @@ (set in-cell-ids) (reverse in-cell-ids))))) +(defn- reflow-eligible-cell? + [{:keys [position row-span column-span id]}] + (and (= position :auto) + (= row-span 1) + (= column-span 1) + (some? id))) + +(defn reflow-grid-auto-items-for-direction + "Reflow single-span auto cells for `to-dir` without changing explicit + placements or `:shapes`." + [parent from-dir to-dir] + (if (= from-dir to-dir) + parent + (let [old-auto-ids (->> (assoc parent :layout-grid-dir from-dir) + (#(cells-seq % :sort? true)) + (filter reflow-eligible-cell?) + (map :id)) + new-auto-ids (->> (assoc parent :layout-grid-dir to-dir) + (#(cells-seq % :sort? true)) + (filter reflow-eligible-cell?) + (map :id)) + shapes (vec (mapcat #(get-in parent [:layout-grid-cells % :shapes]) old-auto-ids))] + (-> parent + (assoc :layout-grid-dir to-dir) + (assoc :layout-grid-cells + (reduce + (fn [acc [idx cell-id]] + (let [shape (get shapes idx)] + (assoc acc cell-id + (assoc (get acc cell-id) + :shapes (if (some? shape) [shape] []))))) + (:layout-grid-cells parent) + (map-indexed vector new-auto-ids))))))) + (defn cells-by-row ([parent index] (cells-by-row parent index true)) diff --git a/common/test/common_tests/types/shape_layout_test.cljc b/common/test/common_tests/types/shape_layout_test.cljc index 308dd758a6..3aa0b08b95 100644 --- a/common/test/common_tests/types/shape_layout_test.cljc +++ b/common/test/common_tests/types/shape_layout_test.cljc @@ -1491,3 +1491,93 @@ (layout/add-grid-row {:type :fixed :value 300})) result (layout/reorder-grid-row parent 0 2 false)] (t/is (= 3 (count (:layout-grid-rows result))))))) + +;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; +;; reflow-grid-auto-items-for-direction +;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; + +(t/deftest reflow-grid-auto-items-for-direction-test + (t/testing "row->column redistributes auto cells and keeps :shapes" + (let [s1 (uuid/next) s2 (uuid/next) s3 (uuid/next) s4 (uuid/next) + c11 (make-cell :row 1 :column 1 :shapes [s1]) + c12 (make-cell :row 1 :column 2 :shapes [s2]) + c21 (make-cell :row 2 :column 1 :shapes [s3]) + c22 (make-cell :row 2 :column 2 :shapes [s4]) + parent {:layout :grid + :layout-grid-dir :row + :shapes [s4 s3 s2 s1] + :layout-grid-cells {(:id c11) c11 (:id c12) c12 (:id c21) c21 (:id c22) c22}} + holder (fn [p r c] (:shapes (layout/cell-by-row-column p r c))) + result (layout/reflow-grid-auto-items-for-direction parent :row :column)] + (t/is (= :column (:layout-grid-dir result))) + (t/is (= (:shapes parent) (:shapes result))) + (t/is (= [s1] (holder result 1 1))) + (t/is (= [s3] (holder result 1 2))) + (t/is (= [s2] (holder result 2 1))) + (t/is (= [s4] (holder result 2 2))) + (t/testing "row->column->row round-trips to the original state" + (let [roundtrip (layout/reflow-grid-auto-items-for-direction + result :column :row)] + (t/is (= :row (:layout-grid-dir roundtrip))) + (t/is (= (:shapes parent) (:shapes roundtrip))) + (t/is (= (:layout-grid-cells parent) (:layout-grid-cells roundtrip))) + (t/is (= (select-keys parent [:layout-grid-dir :layout-grid-cells :shapes]) + (select-keys roundtrip [:layout-grid-dir :layout-grid-cells :shapes])))))))) + +(t/deftest reflow-grid-sparse-auto-cells-clear-leftover-test + (t/testing "sparse grids: leftover target auto cells are emptied, no duplicates" + (let [s1 (uuid/next) s2 (uuid/next) + c11 (make-cell :row 1 :column 1 :shapes [s1]) + c12 (make-cell :row 1 :column 2 :shapes [s2]) + c21 (make-cell :row 2 :column 1 :shapes []) + c22 (make-cell :row 2 :column 2 :shapes []) + parent {:layout :grid + :layout-grid-dir :row + :shapes [s2 s1] + :layout-grid-cells {(:id c11) c11 (:id c12) c12 + (:id c21) c21 (:id c22) c22}} + result (layout/reflow-grid-auto-items-for-direction + parent :row :column) + actual (mapv :shapes (layout/cells-seq result :sort? true))] + (t/is (= :column (:layout-grid-dir result))) + (t/is (= (:shapes parent) (:shapes result))) + (t/is (= [[s1] [s2] [] []] actual))))) + +(t/deftest reflow-grid-mixed-keeps-invariants-test + (t/testing "mixed grids: only 1x1 auto cells move, no loss or duplicates" + (let [a (uuid/next) b (uuid/next) c (uuid/next) e (uuid/next) + x (uuid/next) d (uuid/next) m (uuid/next) + auto-a (make-cell :row 1 :column 1 :shapes [a]) + span-x (assoc (make-cell :row 1 :column 2 :position :manual + :shapes [x]) + :row-span 2 :column-span 1) + auto-b (make-cell :row 2 :column 1 :shapes [b]) + auto-c (make-cell :row 1 :column 3 :shapes [c]) + manual-m (make-cell :row 2 :column 3 :position :manual + :shapes [m]) + area-d (assoc (make-cell :row 3 :column 1 :position :area + :area-name "d" :shapes [d]) + :row-span 1 :column-span 1) + auto-e (make-cell :row 3 :column 2 :shapes [e]) + parent {:layout :grid + :layout-grid-dir :row + :shapes [x e d m c b a] + :layout-grid-cells {(:id auto-a) auto-a (:id span-x) span-x + (:id auto-b) auto-b (:id auto-c) auto-c + (:id manual-m) manual-m + (:id area-d) area-d (:id auto-e) auto-e}} + result (layout/reflow-grid-auto-items-for-direction + parent :row :column) + before (->> (:layout-grid-cells parent) vals (mapcat :shapes)) + after (->> (:layout-grid-cells result) vals (mapcat :shapes))] + (t/is (= :column (:layout-grid-dir result))) + (t/is (= (:shapes parent) (:shapes result))) + (t/is (= span-x (get-in result [:layout-grid-cells (:id span-x)]))) + (t/is (= manual-m (get-in result [:layout-grid-cells (:id manual-m)]))) + (t/is (= area-d (get-in result [:layout-grid-cells (:id area-d)]))) + (t/is (= [a] (:shapes (get-in result [:layout-grid-cells (:id auto-a)])))) + (t/is (= [c] (:shapes (get-in result [:layout-grid-cells (:id auto-b)])))) + (t/is (= [b] (:shapes (get-in result [:layout-grid-cells (:id auto-e)])))) + (t/is (= [e] (:shapes (get-in result [:layout-grid-cells (:id auto-c)])))) + (t/is (= (set before) (set after))) + (t/is (= (count after) (count (distinct after))))))) diff --git a/frontend/src/app/main/data/workspace/shape_layout.cljs b/frontend/src/app/main/data/workspace/shape_layout.cljs index 9d03bdbec1..df614dfe7a 100644 --- a/frontend/src/app/main/data/workspace/shape_layout.cljs +++ b/frontend/src/app/main/data/workspace/shape_layout.cljs @@ -319,6 +319,36 @@ (create-layout type))] (rx/of (with-meta event (meta it))))))))) +(defn change-grid-direction + "Set grid direction and reflow single-span auto items in one undo step." + [ids dir] + (ptk/reify ::change-grid-direction + ptk/WatchEvent + (watch [_ _ _] + (let [undo-id (js/Symbol)] + (rx/of (dwu/start-undo-transaction undo-id) + (dwsh/update-shapes + ids + (fn [shape] + (let [from-dir (d/nilv (:layout-grid-dir shape) :row)] + (cond + (not= :grid (:layout shape)) + shape + + (not (contains? #{:row :column} dir)) + shape + + (= from-dir dir) + (assoc shape :layout-grid-dir dir) + + :else + (ctl/reflow-grid-auto-items-for-direction shape from-dir dir)))) + ;; Auto cells are already reflowed, so skip generic reassignment + ;; and child reordering. + {:skip-grid-reassignment? true}) + (ptk/data-event :layout/update {:ids ids}) + (dwu/commit-undo-transaction undo-id)))))) + (defn update-layout ([ids changes] (update-layout ids changes nil)) ([ids changes options] diff --git a/frontend/src/app/main/data/workspace/shapes.cljs b/frontend/src/app/main/data/workspace/shapes.cljs index 2454b537a1..9c3878298a 100644 --- a/frontend/src/app/main/data/workspace/shapes.cljs +++ b/frontend/src/app/main/data/workspace/shapes.cljs @@ -110,7 +110,7 @@ ([ids update-fn {:keys [reg-objects? save-undo? stack-undo? attrs ignore-tree page-id ignore-touched undo-group with-objects? changed-sub-attr - translation? skip-component-sync?] + translation? skip-grid-reassignment? skip-component-sync?] :or {reg-objects? false save-undo? true stack-undo? false @@ -150,7 +150,8 @@ :changed-sub-attr changed-sub-attr :ignore-tree ignore-tree :ignore-touched ignore-touched - :with-objects? with-objects?}) + :with-objects? with-objects? + :skip-grid-reassignment? skip-grid-reassignment?}) (cond-> reg-objects? (pcb/resize-parents ids)) (pcb/set-translation? translation?) (pcb/set-skip-component-sync? skip-component-sync?))))] @@ -189,7 +190,7 @@ {:as props :keys [reg-objects? save-undo? stack-undo? attrs ignore-tree page-id ignore-touched undo-group with-objects? changed-sub-attr translation? - skip-component-sync?] + skip-grid-reassignment? skip-component-sync?] :or {reg-objects? false save-undo? true stack-undo? false @@ -222,7 +223,8 @@ :ignore-tree ignore-tree :ignore-touched ignore-touched :with-objects? with-objects? - :translation? translation?}) + :translation? translation? + :skip-grid-reassignment? skip-grid-reassignment?}) (cond-> undo-group (pcb/set-undo-group undo-group)) (pcb/set-translation? translation?) diff --git a/frontend/src/app/main/ui/workspace/sidebar/options/menus/layout_container.cljs b/frontend/src/app/main/ui/workspace/sidebar/options/menus/layout_container.cljs index 14f89f38b9..4fb0ebde64 100644 --- a/frontend/src/app/main/ui/workspace/sidebar/options/menus/layout_container.cljs +++ b/frontend/src/app/main/ui/workspace/sidebar/options/menus/layout_container.cljs @@ -1098,6 +1098,8 @@ (get n-values :layout-justify-content)) (identical? (get o-values :layout-align-content) (get n-values :layout-align-content)) + (identical? (get o-values :layout-grid-dir) + (get n-values :layout-grid-dir)) (identical? (get o-values :layout) (get n-values :layout))))) @@ -1231,7 +1233,7 @@ (fn [dir] (if (= :flex layout-type) (st/emit! (dwsl/update-layout ids {:layout-flex-dir dir})) - (st/emit! (dwsl/update-layout ids {:layout-grid-dir dir}))))) + (st/emit! (dwsl/change-grid-direction ids dir))))) ;; Align grid align-items-row (:layout-align-items values) @@ -1426,7 +1428,7 @@ (mf/use-fn (mf/deps ids) (fn [dir] - (st/emit! (dwsl/update-layout ids {:layout-grid-dir dir})))) + (st/emit! (dwsl/change-grid-direction ids dir)))) on-gap-change (mf/use-fn diff --git a/frontend/src/app/plugins/grid.cljs b/frontend/src/app/plugins/grid.cljs index 15771ff7b3..32cc2bf9ea 100644 --- a/frontend/src/app/plugins/grid.cljs +++ b/frontend/src/app/plugins/grid.cljs @@ -56,7 +56,7 @@ (u/not-valid plugin-id :dir "Cannot modify a page that is not currently active") :else - (st/emit! (dwsl/update-layout #{id} {:layout-grid-dir value})))))} + (st/emit! (dwsl/change-grid-direction #{id} value)))))} :rows {:this true diff --git a/frontend/test/frontend_tests/plugins/grid_test.cljs b/frontend/test/frontend_tests/plugins/grid_test.cljs index dc5c2ff8df..66e5f51a48 100644 --- a/frontend/test/frontend_tests/plugins/grid_test.cljs +++ b/frontend/test/frontend_tests/plugins/grid_test.cljs @@ -9,6 +9,7 @@ [app.common.test-helpers.files :as cthf] [app.main.store :as st] [app.plugins.api :as api] + [app.plugins.utils :as u] [cljs.test :as t :include-macros true] [frontend-tests.helpers.state :as ths] [frontend-tests.helpers.wasm :as thw] @@ -47,3 +48,17 @@ (t/is (thrown? js/Error (.setColumn grid 1 "fixed" 10))) (t/is (thrown? js/Error (.removeRow grid 1))) (t/is (thrown? js/Error (.removeColumn grid 1))))))) + +(t/deftest grid-dir-setter-persists-implicit-row + (thw/with-wasm-mocks* + (fn [] + (let [{:keys [store ^js grid]} (setup-grid) + file-id (:current-file-id @store) + page-id (:current-page-id @store) + grid-id (:id (u/proxy->shape grid))] + (swap! store update-in + [:files file-id :data :pages-index page-id :objects grid-id] + dissoc + :layout-grid-dir) + (set! (.-dir grid) "row") + (t/is (= "row" (.-dir grid)))))))