diff --git a/CHANGES.md b/CHANGES.md index 003089362..9c7c76520 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -40,6 +40,7 @@ - Fix wrong update of text in components [Taiga #4646](https://tree.taiga.io/project/penpot/issue/4646) - Fix problem with SVG imports with style [#2605](https://github.com/penpot/penpot/issues/2605) - Fix ghost shapes after sync groups in components [Taiga #4649](https://tree.taiga.io/project/penpot/issue/4649) +- Fix layer orders messed up on move, group, reparent and undo [Github #2672](https://github.com/penpot/penpot/issues/2672) ## 1.16.2-beta diff --git a/common/src/app/common/pages/changes.cljc b/common/src/app/common/pages/changes.cljc index cee62de19..b7a81a136 100644 --- a/common/src/app/common/pages/changes.cljc +++ b/common/src/app/common/pages/changes.cljc @@ -182,7 +182,7 @@ (d/update-in-when data [:components component-id :objects] reg-objects)))) (defmethod process-change :mov-objects - [data {:keys [parent-id shapes index page-id component-id ignore-touched]}] + [data {:keys [parent-id shapes index page-id component-id ignore-touched after-shape]}] (letfn [(calculate-invalid-targets [objects shape-id] (let [reduce-fn #(into %1 (calculate-invalid-targets objects %2))] (->> (get-in objects [shape-id :shapes]) @@ -260,9 +260,12 @@ (not= :frame (:type obj)) (as-> $$ (reduce (partial assign-frame-id frame-id) $$ (:shapes obj)))))) + + (move-objects [objects] (let [valid? (every? (partial is-valid-move? objects) shapes) parent (get objects parent-id) + index (if (nil? after-shape) index (inc (d/index-of (:shapes parent) after-shape))) frame-id (if (= :frame (:type parent)) (:id parent) (:frame-id parent))] @@ -283,7 +286,7 @@ ;; Ensure that all shapes of the new parent has a ;; correct link to the topside frame. (reduce (partial assign-frame-id frame-id) $ shapes)) - objects)))] + objects)))] (if page-id (d/update-in-when data [:pages-index page-id :objects] move-objects) diff --git a/common/src/app/common/pages/changes_builder.cljc b/common/src/app/common/pages/changes_builder.cljc index bb669eeb1..e85445cc4 100644 --- a/common/src/app/common/pages/changes_builder.cljc +++ b/common/src/app/common/pages/changes_builder.cljc @@ -244,32 +244,26 @@ (assert-page-id changes) (assert-objects changes) (let [objects (lookup-objects changes) - set-parent-change (cond-> {:type :mov-objects :parent-id parent-id :page-id (::page-id (meta changes)) - :shapes (->> shapes (mapv :id))} + :shapes (->> shapes reverse (mapv :id))} (some? index) (assoc :index index)) mk-undo-change (fn [change-set shape] - (let [idx (or (cph/get-position-on-parent objects (:id shape)) 0) - ;; Different index if the movement was from top to bottom or the other way - ;; Similar that on frontend/src/app/main/ui/workspace/sidebar/layers.cljs - ;; with the 'side' property of the on-drop - idx (if (< index idx) - (inc idx) - idx)] + (let [prev-sibling (cph/get-prev-sibling objects (:id shape))] (d/preconj change-set {:type :mov-objects :page-id (::page-id (meta changes)) :parent-id (:parent-id shape) :shapes [(:id shape)] - :index idx})))] + :after-shape prev-sibling + :index 0})))] ; index is used in case there is no after-shape (moving bottom shapes) (-> changes (update :redo-changes conj set-parent-change) diff --git a/common/src/app/common/pages/helpers.cljc b/common/src/app/common/pages/helpers.cljc index 8ada915fc..bdc5a781f 100644 --- a/common/src/app/common/pages/helpers.cljc +++ b/common/src/app/common/pages/helpers.cljc @@ -147,6 +147,16 @@ prt (get objects pid)] (d/index-of (:shapes prt) id))) +(defn get-prev-sibling + [objects id] + (let [obj (get objects id) + pid (:parent-id obj) + prt (get objects pid) + shapes (:shapes prt) + pos (d/index-of shapes id)] + (if (= 0 pos) nil (nth shapes (dec pos))))) + + (defn get-immediate-children "Retrieve resolved shape objects that are immediate children of the specified shape-id" @@ -336,6 +346,13 @@ (map second) (into #{})))) +(defn order-by-indexed-shapes + [objects ids] + (->> (indexed-shapes objects) + (sort-by first) + (filter (comp (into #{} ids) second)) + (map second))) + ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; ;; SHAPES ORGANIZATION (PATH MANAGEMENT) ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; diff --git a/frontend/src/app/main/data/workspace.cljs b/frontend/src/app/main/data/workspace.cljs index 2dca29d37..e01bc88f9 100644 --- a/frontend/src/app/main/data/workspace.cljs +++ b/frontend/src/app/main/data/workspace.cljs @@ -639,14 +639,15 @@ (defn relocate-shapes-changes [it objects parents parent-id page-id to-index ids groups-to-delete groups-to-unmask shapes-to-detach shapes-to-reroot shapes-to-deroot shapes-to-unconstraint] - (let [shapes (map (d/getf objects) ids)] + (let [ordered-indexes (cph/order-by-indexed-shapes objects ids) + shapes (map (d/getf objects) ordered-indexes)] (-> (pcb/empty-changes it page-id) (pcb/with-objects objects) ; Move the shapes (pcb/change-parent parent-id - (reverse shapes) + shapes to-index) ; Remove empty groups diff --git a/frontend/src/app/main/data/workspace/bool.cljs b/frontend/src/app/main/data/workspace/bool.cljs index 88f9dd1cc..bfaec41ac 100644 --- a/frontend/src/app/main/data/workspace/bool.cljs +++ b/frontend/src/app/main/data/workspace/bool.cljs @@ -20,15 +20,11 @@ [cuerdas.core :as str] [potok.core :as ptk])) -(defn selected-shapes +(defn selected-shapes-idx [state] (let [objects (wsh/lookup-page-objects state)] (->> (wsh/lookup-selected state) - (cph/clean-loops objects) - (map (d/getf objects)) - (remove cph/frame-shape?) - (map #(assoc % ::index (cph/get-position-on-parent objects (:id %)))) - (sort-by ::index)))) + (cph/clean-loops objects)))) (defn create-bool-data [bool-type name shapes objects] @@ -92,10 +88,15 @@ base-name (-> bool-type d/name str/capital (str "-1")) name (-> (ctt/retrieve-used-names objects) (ctt/generate-unique-name base-name)) - shapes (selected-shapes state)] + ids (selected-shapes-idx state) + ordered-indexes (cph/order-by-indexed-shapes objects ids) + shapes (->> ordered-indexes + (map (d/getf objects)) + (remove cph/frame-shape?))] (when-not (empty? shapes) - (let [[boolean-data index] (create-bool-data bool-type name shapes objects) + (let [[boolean-data index] (create-bool-data bool-type name (reverse shapes) objects) + index (inc index) shape-id (:id boolean-data) changes (-> (pcb/empty-changes it page-id) (pcb/with-objects objects) diff --git a/frontend/src/app/main/data/workspace/groups.cljs b/frontend/src/app/main/data/workspace/groups.cljs index aec9cc0db..fde89eb0c 100644 --- a/frontend/src/app/main/data/workspace/groups.cljs +++ b/frontend/src/app/main/data/workspace/groups.cljs @@ -22,6 +22,7 @@ (defn shapes-for-grouping [objects selected] (->> selected + (cph/order-by-indexed-shapes objects) (map #(get objects %)) (map #(assoc % ::index (cph/get-position-on-parent objects (:id %)))) (sort-by ::index))) @@ -76,12 +77,13 @@ (ctst/generate-unique-name base-name))) selrect (gsh/selection-rect shapes) + group-idx (inc (::index (last shapes))) group (-> (cts/make-minimal-group frame-id selrect gname) (cts/setup-shape selrect) (assoc :shapes (mapv :id shapes) :parent-id parent-id :frame-id frame-id - :index (::index (first shapes)))) + :index group-idx)) ;; Shapes that are in a component, but are not root, must be detached, ;; because they will be now children of a non instance group. @@ -93,8 +95,8 @@ changes (-> (pcb/empty-changes it page-id) (pcb/with-objects objects) - (pcb/add-object group {:index (::index (first shapes))}) - (pcb/change-parent (:id group) shapes) + (pcb/add-object group {:index group-idx}) + (pcb/change-parent (:id group) (reverse shapes)) (pcb/update-shapes (map :id shapes-to-detach) ctk/detach-shape) (pcb/remove-objects ids-to-delete))] @@ -102,7 +104,9 @@ (defn remove-group-changes [it page-id group objects] - (let [children (mapv #(get objects %) (:shapes group)) + (let [children (->> (:shapes group) + (cph/order-by-indexed-shapes objects) + (mapv #(get objects %))) parent-id (cph/get-parent-id objects (:id group)) parent (get objects parent-id) @@ -114,7 +118,7 @@ ;; Shapes that are in a component (including root) must be detached, ;; because cannot be easyly synchronized back to the main component. - shapes-to-detach (filter ctk/in-component-instance? + shapes-to-detach (filter ctk/in-component-instance? (cph/get-children-with-self objects (:id group)))] (-> (pcb/empty-changes it page-id) @@ -125,8 +129,9 @@ (defn remove-frame-changes [it page-id frame objects] - - (let [children (mapv #(get objects %) (:shapes frame)) + (let [children (->> (:shapes frame) + (cph/order-by-indexed-shapes objects) + (mapv #(get objects %))) parent-id (cph/get-parent-id objects (:id frame)) idx-in-parent (cph/get-position-on-parent objects (:id frame))] diff --git a/frontend/src/app/main/data/workspace/shapes.cljs b/frontend/src/app/main/data/workspace/shapes.cljs index 95adb579a..de4af3244 100644 --- a/frontend/src/app/main/data/workspace/shapes.cljs +++ b/frontend/src/app/main/data/workspace/shapes.cljs @@ -118,10 +118,8 @@ (let [page-id (:current-page-id state) objects (wsh/lookup-page-objects state page-id) - to-move-shapes - (into [] - (map (d/getf objects)) - (reverse (ctst/sort-z-index objects shapes))) + ordered-indexes (cph/order-by-indexed-shapes objects shapes) + to-move-shapes (map (d/getf objects) ordered-indexes) changes (when (d/not-empty? to-move-shapes) diff --git a/frontend/src/app/main/data/workspace/transforms.cljs b/frontend/src/app/main/data/workspace/transforms.cljs index b61f63749..371ddd731 100644 --- a/frontend/src/app/main/data/workspace/transforms.cljs +++ b/frontend/src/app/main/data/workspace/transforms.cljs @@ -685,6 +685,7 @@ shapes (->> ids (cph/clean-loops objects) (keep lookup)) + moving-shapes (cond->> shapes (not layout?) @@ -694,6 +695,9 @@ (remove #(and (= (:frame-id %) frame-id) (not= (:parent-id %) frame-id)))) + ordered-indexes (cph/order-by-indexed-shapes objects (map :id moving-shapes)) + moving-shapes (map (d/getf objects) ordered-indexes) + all-parents (reduce (fn [res id] (into res (cph/get-parent-ids objects id)))