mirror of
https://github.com/penpot/penpot.git
synced 2026-09-20 10:56:15 +00:00
🐛 Preserve source order when changing grid flow (#11662)
* 🐛 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 <junsoo1172@gmail.com> * 🐛 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 <junsoo1172@gmail.com> * 🐛 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 <junsoo1172@gmail.com> * 🐛 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 <junsoo1172@gmail.com> * 🐛 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 <junsoo1172@gmail.com> * 🐛 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 <junsoo1172@gmail.com> * 🐛 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 <junsoo1172@gmail.com> * 🐛 Fix grid plugin dir setter syntax Signed-off-by: makesomethingshit <junsoo1172@gmail.com> AI-assisted-by: opencode-go/muse-spark-1.3-contributor * 🐛 Fix comments and tests --------- Signed-off-by: makesomethingshit <junsoo1172@gmail.com> Co-authored-by: alonso.torres <alonso.torres@kaleidos.net>
This commit is contained in:
parent
2cdfc912d8
commit
edf146db7b
@ -75,7 +75,7 @@
|
|||||||
(reduce check-shape changes mod-obj-changes)))
|
(reduce check-shape changes mod-obj-changes)))
|
||||||
|
|
||||||
(defn generate-update-shapes
|
(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
|
(let [changes (reduce
|
||||||
(fn [changes id]
|
(fn [changes id]
|
||||||
(let [opts {:attrs attrs
|
(let [opts {:attrs attrs
|
||||||
@ -86,9 +86,9 @@
|
|||||||
(cond-> changes
|
(cond-> changes
|
||||||
(some? objects) (pcb/with-objects objects))
|
(some? objects) (pcb/with-objects objects))
|
||||||
ids)
|
ids)
|
||||||
;; Translation doesn't shift children between grid cells, so
|
;; Translation keeps cell assignments; direction changes reflow them
|
||||||
;; cell reassignment + child reorder are no-ops.
|
;; explicitly.
|
||||||
grid-ids (when-not translation?
|
grid-ids (when-not (or translation? skip-grid-reassignment?)
|
||||||
(->> ids (filter (partial ctl/grid-layout? objects))))
|
(->> ids (filter (partial ctl/grid-layout? objects))))
|
||||||
changes (cond-> changes
|
changes (cond-> changes
|
||||||
(seq grid-ids)
|
(seq grid-ids)
|
||||||
|
|||||||
@ -1557,6 +1557,40 @@
|
|||||||
(set in-cell-ids)
|
(set in-cell-ids)
|
||||||
(reverse 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
|
(defn cells-by-row
|
||||||
([parent index]
|
([parent index]
|
||||||
(cells-by-row parent index true))
|
(cells-by-row parent index true))
|
||||||
|
|||||||
@ -1491,3 +1491,93 @@
|
|||||||
(layout/add-grid-row {:type :fixed :value 300}))
|
(layout/add-grid-row {:type :fixed :value 300}))
|
||||||
result (layout/reorder-grid-row parent 0 2 false)]
|
result (layout/reorder-grid-row parent 0 2 false)]
|
||||||
(t/is (= 3 (count (:layout-grid-rows result)))))))
|
(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)))))))
|
||||||
|
|||||||
@ -319,6 +319,36 @@
|
|||||||
(create-layout type))]
|
(create-layout type))]
|
||||||
(rx/of (with-meta event (meta it)))))))))
|
(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
|
(defn update-layout
|
||||||
([ids changes] (update-layout ids changes nil))
|
([ids changes] (update-layout ids changes nil))
|
||||||
([ids changes options]
|
([ids changes options]
|
||||||
|
|||||||
@ -110,7 +110,7 @@
|
|||||||
([ids update-fn
|
([ids update-fn
|
||||||
{:keys [reg-objects? save-undo? stack-undo? attrs ignore-tree page-id
|
{:keys [reg-objects? save-undo? stack-undo? attrs ignore-tree page-id
|
||||||
ignore-touched undo-group with-objects? changed-sub-attr
|
ignore-touched undo-group with-objects? changed-sub-attr
|
||||||
translation? skip-component-sync?]
|
translation? skip-grid-reassignment? skip-component-sync?]
|
||||||
:or {reg-objects? false
|
:or {reg-objects? false
|
||||||
save-undo? true
|
save-undo? true
|
||||||
stack-undo? false
|
stack-undo? false
|
||||||
@ -150,7 +150,8 @@
|
|||||||
:changed-sub-attr changed-sub-attr
|
:changed-sub-attr changed-sub-attr
|
||||||
:ignore-tree ignore-tree
|
:ignore-tree ignore-tree
|
||||||
:ignore-touched ignore-touched
|
: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))
|
(cond-> reg-objects? (pcb/resize-parents ids))
|
||||||
(pcb/set-translation? translation?)
|
(pcb/set-translation? translation?)
|
||||||
(pcb/set-skip-component-sync? skip-component-sync?))))]
|
(pcb/set-skip-component-sync? skip-component-sync?))))]
|
||||||
@ -189,7 +190,7 @@
|
|||||||
{:as props
|
{:as props
|
||||||
:keys [reg-objects? save-undo? stack-undo? attrs ignore-tree page-id
|
:keys [reg-objects? save-undo? stack-undo? attrs ignore-tree page-id
|
||||||
ignore-touched undo-group with-objects? changed-sub-attr translation?
|
ignore-touched undo-group with-objects? changed-sub-attr translation?
|
||||||
skip-component-sync?]
|
skip-grid-reassignment? skip-component-sync?]
|
||||||
:or {reg-objects? false
|
:or {reg-objects? false
|
||||||
save-undo? true
|
save-undo? true
|
||||||
stack-undo? false
|
stack-undo? false
|
||||||
@ -222,7 +223,8 @@
|
|||||||
:ignore-tree ignore-tree
|
:ignore-tree ignore-tree
|
||||||
:ignore-touched ignore-touched
|
:ignore-touched ignore-touched
|
||||||
:with-objects? with-objects?
|
:with-objects? with-objects?
|
||||||
:translation? translation?})
|
:translation? translation?
|
||||||
|
:skip-grid-reassignment? skip-grid-reassignment?})
|
||||||
(cond-> undo-group
|
(cond-> undo-group
|
||||||
(pcb/set-undo-group undo-group))
|
(pcb/set-undo-group undo-group))
|
||||||
(pcb/set-translation? translation?)
|
(pcb/set-translation? translation?)
|
||||||
|
|||||||
@ -1098,6 +1098,8 @@
|
|||||||
(get n-values :layout-justify-content))
|
(get n-values :layout-justify-content))
|
||||||
(identical? (get o-values :layout-align-content)
|
(identical? (get o-values :layout-align-content)
|
||||||
(get n-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)
|
(identical? (get o-values :layout)
|
||||||
(get n-values :layout)))))
|
(get n-values :layout)))))
|
||||||
|
|
||||||
@ -1231,7 +1233,7 @@
|
|||||||
(fn [dir]
|
(fn [dir]
|
||||||
(if (= :flex layout-type)
|
(if (= :flex layout-type)
|
||||||
(st/emit! (dwsl/update-layout ids {:layout-flex-dir dir}))
|
(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 grid
|
||||||
align-items-row (:layout-align-items values)
|
align-items-row (:layout-align-items values)
|
||||||
@ -1426,7 +1428,7 @@
|
|||||||
(mf/use-fn
|
(mf/use-fn
|
||||||
(mf/deps ids)
|
(mf/deps ids)
|
||||||
(fn [dir]
|
(fn [dir]
|
||||||
(st/emit! (dwsl/update-layout ids {:layout-grid-dir dir}))))
|
(st/emit! (dwsl/change-grid-direction ids dir))))
|
||||||
|
|
||||||
on-gap-change
|
on-gap-change
|
||||||
(mf/use-fn
|
(mf/use-fn
|
||||||
|
|||||||
@ -56,7 +56,7 @@
|
|||||||
(u/not-valid plugin-id :dir "Cannot modify a page that is not currently active")
|
(u/not-valid plugin-id :dir "Cannot modify a page that is not currently active")
|
||||||
|
|
||||||
:else
|
:else
|
||||||
(st/emit! (dwsl/update-layout #{id} {:layout-grid-dir value})))))}
|
(st/emit! (dwsl/change-grid-direction #{id} value)))))}
|
||||||
|
|
||||||
:rows
|
:rows
|
||||||
{:this true
|
{:this true
|
||||||
|
|||||||
@ -9,6 +9,7 @@
|
|||||||
[app.common.test-helpers.files :as cthf]
|
[app.common.test-helpers.files :as cthf]
|
||||||
[app.main.store :as st]
|
[app.main.store :as st]
|
||||||
[app.plugins.api :as api]
|
[app.plugins.api :as api]
|
||||||
|
[app.plugins.utils :as u]
|
||||||
[cljs.test :as t :include-macros true]
|
[cljs.test :as t :include-macros true]
|
||||||
[frontend-tests.helpers.state :as ths]
|
[frontend-tests.helpers.state :as ths]
|
||||||
[frontend-tests.helpers.wasm :as thw]
|
[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 (.setColumn grid 1 "fixed" 10)))
|
||||||
(t/is (thrown? js/Error (.removeRow grid 1)))
|
(t/is (thrown? js/Error (.removeRow grid 1)))
|
||||||
(t/is (thrown? js/Error (.removeColumn 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)))))))
|
||||||
|
|||||||
Loading…
x
Reference in New Issue
Block a user