Merge pull request #3967 from penpot/alotor-grid-fix-auto

Fixes some issues with auto cells

common/src/app/common/files/shapes_helpers.cljc

@@ -50,8 +50,8 @@
           (pcb/update-shapes ordered-indexes #(cond-> % (cfh/frame-shape? %) (assoc :hide-in-viewer true)))
           (pcb/change-parent frame-id to-move-shapes 0)
           (cond-> (ctl/grid-layout? objects frame-id)
-            (pcb/update-shapes [frame-id] ctl/assign-cells {:with-objects? true}))
-          (pcb/reorder-grid-children [frame-id]))
+            (-> (pcb/update-shapes [frame-id] ctl/assign-cells {:with-objects? true})
+                (pcb/reorder-grid-children [frame-id]))))
       changes)))
 
 (defn prepare-create-artboard-from-selection

common/src/app/common/types/shape/layout.cljc

@@ -628,23 +628,36 @@
          (filter #(> (get % prop-span) 1))
          (reduce
           (fn [parent cell]
-            (let [changed-cells
+            (let [area? (= :area (:position cell))
+                  changed-cells
                   (cond
                     ;; New track at the beginning
                     (= (get cell prop) (inc index))
                     [(assoc cell prop-span 1)
-                     (assoc cell :id (uuid/next) :shapes [] prop (inc (get cell prop)) prop-span (dec (get cell prop-span)))]
+                     (-> cell
+                         (assoc :id (uuid/next) :shapes [] prop (inc (get cell prop)) prop-span (dec (get cell prop-span)))
+                         (dissoc :area-name)
+                         (cond-> area? (assoc :position :manual)))]
 
                     ;; New track at the middle
                     (< (get cell prop) (inc index) (+ (get cell prop) (dec (get cell prop-span))))
                     [(assoc cell prop-span (- (inc index) (get cell prop)))
-                     (assoc cell :id (uuid/next) :shapes [] prop (inc index) prop-span 1)
-                     (assoc cell :id (uuid/next) :shapes [] prop (+ index 2) prop-span (- (+ (get cell prop) (dec (get cell prop-span))) (inc index)))]
+                     (-> cell
+                         (assoc :id (uuid/next) :shapes [] prop (inc index) prop-span 1)
+                         (dissoc :area-name)
+                         (cond-> area? (assoc :position :manual)))
+                     (-> cell
+                         (assoc :id (uuid/next) :shapes [] prop (+ index 2) prop-span (- (+ (get cell prop) (dec (get cell prop-span))) (inc index)))
+                         (dissoc :area-name)
+                         (cond-> area? (assoc :position :manual)))]
 
                     ;; New track at the end
                     (= (+ (get cell prop) (dec (get cell prop-span))) (inc index))
                     [(assoc cell prop-span (- (inc index) (get cell prop)))
-                     (assoc cell :id (uuid/next) :shapes [] prop (inc index) prop-span 1)])]
+                     (-> cell
+                         (assoc :id (uuid/next) :shapes [] prop (inc index) prop-span 1)
+                         (dissoc :area-name)
+                         (cond-> area? (assoc :position :manual)))])]
 
               (->> changed-cells
                    (reduce #(update %1 :layout-grid-cells assoc (:id %2) %2) parent))))
@@ -659,17 +672,24 @@
          (filter #(> (get % prop-span) 1))
          (reduce
           (fn [parent cell]
-            (let [changed-cells
+            (let [area? (= :area (:position cell))
+                  changed-cells
                   (cond
                     ;; New track at the beginning
                     (= (get cell prop) (inc index))
                     [(assoc cell prop-span 1)
-                     (assoc cell :id (uuid/next) :shapes [] prop (inc (get cell prop)) prop-span (dec (get cell prop-span)))]
+                     (-> cell
+                         (assoc :id (uuid/next) :shapes [] prop (inc (get cell prop)) prop-span (dec (get cell prop-span)))
+                         (dissoc :area-name)
+                         (cond-> area? (assoc :position :manual)))]
 
                     ;; New track at the middle
                     (< (get cell prop) (inc index) (+ (get cell prop) (dec (get cell prop-span))))
                     [(assoc cell prop-span (- (+ index 2) (get cell prop)))
-                     (assoc cell :id (uuid/next) :shapes [] prop (+ index 2) prop-span (- (+ (get cell prop) (dec (get cell prop-span))) (inc index)))])]
+                     (-> cell
+                         (assoc :id (uuid/next) :shapes [] prop (+ index 2) prop-span (- (+ (get cell prop) (dec (get cell prop-span))) (inc index)))
+                         (dissoc :area-name)
+                         (cond-> area? (assoc :position :manual)))])]
               (->> changed-cells
                    (reduce #(update %1 :layout-grid-cells assoc (:id %2) %2) parent))))
           parent))))
@@ -776,6 +796,8 @@
             (let [match-cell
                   (-> (get to-cells-idx (get cell prop-other))
                       (d/patch-object (select-keys cell [prop-other-span :position :align-self :justify-self]))
+                      (cond-> (= :area (:position cell))
+                        (assoc :position :manual))
                       (cond-> (= (get cell prop-span) 1)
                         (assoc :shapes (mapv ids-map (:shapes cell)))))]
               (recur (rest from-cells)
@@ -796,20 +818,22 @@
 
 
 (defn duplicate-row
-  [shape index ids-map]
+  [shape objects index ids-map]
   (let [value (dm/get-in shape [:layout-grid-rows index])]
     (-> shape
         (remove-cell-areas-after :row index)
         (add-grid-row value (inc index))
-        (duplicate-cells :row index (inc index) ids-map))))
+        (duplicate-cells :row index (inc index) ids-map)
+        (assign-cells objects))))
 
 (defn duplicate-column
-  [shape index ids-map]
+  [shape objects index ids-map]
   (let [value (dm/get-in shape [:layout-grid-columns index])]
     (-> shape
         (remove-cell-areas-after :column index)
         (add-grid-column value (inc index))
-        (duplicate-cells :column index (inc index) ids-map))))
+        (duplicate-cells :column index (inc index) ids-map)
+        (assign-cells objects))))
 
 (defn make-remove-cell
   [attr span-attr track-num]
@@ -1024,6 +1048,41 @@
           parent
           overlaps))
 
+(defn reassign-positions
+  "Propagate the manual positioning to the following cells"
+  [parent]
+  (->> (cells-seq parent :sort? true)
+       (reduce
+        (fn [[parent auto?] cell]
+          
+          (let [[cell auto?]
+                (cond
+                  (and (empty? (:shapes cell))
+                       (= :manual (:position cell))
+                       (= (:row-span cell) 1)
+                       (= (:column-span cell) 1))
+                  [(assoc cell :position :auto) false]
+
+                  (and (or (not= (:row-span cell) 1)
+                           (not= (:column-span cell) 1))
+                       (= :auto (:position cell)))
+                  [(assoc cell :position :manual) false]
+                  
+                  (empty? (:shapes cell))
+                  [cell false]
+
+                  (and (not auto?) (= :auto (:position cell)))
+                  [(assoc cell :position :manual) false]
+
+                  (= :manual (:position cell))
+                  [cell false]
+
+                  :else
+                  [cell auto?])]
+            [(assoc-in parent [:layout-grid-cells (:id cell)] cell) auto?]))
+        [parent true])
+       (first)))
+
 (defn position-auto-shapes
   [parent]
   ;; Iterate through the cells. While auto and contains shape no changes.
@@ -1050,6 +1109,14 @@
                      (rest shapes)))))]
     parent))
 
+(defn assign-cell-positions
+  [parent objects]
+  (prn ">>>>assign-cell-positions" (:name parent))
+  (-> parent
+      (check-deassigned-cells objects)
+      (reassign-positions)
+      (position-auto-shapes)))
+
 ;; Assign cells takes the children and move them into the allotted cells. If there are not enough cells it creates
 ;; not-tracked rows/columns and put the shapes there
 ;;   Non-tracked tracks need to be deleted when they are empty and there are no more shapes unallocated
@@ -1061,13 +1128,10 @@
 ;;  - (maybe) create group/frames. This case will assigna a cell that had one of its children
 (defn assign-cells
   [parent objects]
-  (let [;; TODO: Remove this, shouldn't be happening
-        ;;overlaps (overlapping-cells parent)
-        ;;_ (when (not (empty? overlaps))
-        ;;    (.warn js/console "OVERLAPS" overlaps))
-        parent (cond-> (check-deassigned-cells parent objects)
-                 #_(d/not-empty? overlaps)
-                 #_(fix-overlaps overlaps))
+  (prn ">assign-cells")
+  (let [
+
+        parent (assign-cell-positions parent objects)
 
         shape-has-cell?
         (into #{} (mapcat (comp :shapes second)) (:layout-grid-cells parent))
@@ -1075,9 +1139,7 @@
         no-cell-shapes
         (->> (:shapes parent)
              (remove shape-has-cell?)
-             (remove (partial position-absolute? objects)))
-
-        parent (position-auto-shapes parent)]
+             (remove (partial position-absolute? objects)))]
 
     (if (empty? no-cell-shapes)
       ;; All shapes are within a cell. No need to assign
@@ -1415,13 +1477,16 @@
                (<= first-column (+ column column-span -1) last-column)))))))
 
 (defn shapes-by-row
+  "Find all the shapes for a given row"
   ([parent index]
    (shapes-by-row parent index true))
+  ;; check-span? if false will only see if there is a coincidence in file&row
   ([parent index check-span?]
    (->> (cells-by-row parent index check-span?)
         (mapcat :shapes))))
 
 (defn shapes-by-column
+  "Find all the shapes for a given column"
   ([parent index]
    (shapes-by-column parent index true))
   ([parent index check-span?]
@@ -1487,7 +1552,7 @@
        [r c]))))
 
 (defn remap-grid-cells
-  "Remaps the shapes inside the cells"
+  "Remaps the shapes ids inside the cells"
   [shape ids-map]
   (let [do-remap-cells
         (fn [cell]

frontend/src/app/main/data/workspace/changes.cljs

@@ -15,6 +15,7 @@
    [app.common.logging :as log]
    [app.common.schema :as sm]
    [app.common.types.shape-tree :as ctst]
+   [app.common.types.shape.layout :as ctl]
    [app.common.uuid :as uuid]
    [app.main.data.workspace.state-helpers :as wsh]
    [app.main.data.workspace.undo :as dwu]
@@ -87,6 +88,8 @@
                             (cond-> undo-group
                               (pcb/set-undo-group undo-group)))
                         ids)
+             grid-ids (->> ids (filter (partial ctl/grid-layout? objects)))
+             changes (pcb/update-shapes changes grid-ids ctl/assign-cell-positions {:with-objects? true})
              changes (pcb/reorder-grid-children changes ids)
              changes (add-undo-group changes state)]
          (rx/concat

frontend/src/app/main/data/workspace/selection.cljs

@@ -500,7 +500,6 @@
                        (pcb/amend-last-change #(assoc % :old-id (:id obj)))
                        (cond-> (ctl/grid-layout? objects (:parent-id obj))
                          (-> (pcb/update-shapes [(:parent-id obj)] ctl/assign-cells {:with-objects? true})
-                             (pcb/update-shapes [(:parent-id obj)] ctl/check-deassigned-cells {:with-objects? true})
                              (pcb/reorder-grid-children [(:parent-id obj)]))))
 
            changes (cond-> changes

frontend/src/app/main/data/workspace/shape_layout.cljs

@@ -342,12 +342,13 @@
             (-> changes
                 (pcb/update-shapes
                  ids
-                 (fn [shape]
+                 (fn [shape objects]
                    ;; The duplication could have altered the grid so we restore the values, we'll calculate the good ones now
                    (let [shape (merge shape (select-keys base-shape [:layout-grid-cells :layout-grid-columns :layout-grid-rows]))]
                      (case type
-                       :row    (ctl/duplicate-row shape index ids-map)
-                       :column (ctl/duplicate-column shape index ids-map))))))
+                       :row    (ctl/duplicate-row shape objects index ids-map)
+                       :column (ctl/duplicate-column shape objects index ids-map))))
+                 {:with-objects? true}))
 
             undo-id (js/Symbol)]
         (rx/of (dwu/start-undo-transaction undo-id)

frontend/src/app/main/data/workspace/transforms.cljs

@@ -559,8 +559,8 @@
                     (fn [[_ target-frame drop-index]]
                       (let [undo-id (js/Symbol)]
                         (rx/of (dwu/start-undo-transaction undo-id)
-                               (dwm/apply-modifiers {:undo-transation? false})
                                (move-shapes-to-frame ids target-frame drop-index)
+                               (dwm/apply-modifiers {:undo-transation? false})
                                (finish-transform)
                                (dwu/commit-undo-transaction undo-id))))))))))))))
 
@@ -872,7 +872,9 @@
                 (pcb/update-shapes moving-shapes-ids #(cond-> % (cfh/frame-shape? %) (assoc :hide-in-viewer true)))
                 (pcb/update-shapes shape-ids-to-detach ctk/detach-shape)
                 (pcb/change-parent frame-id moving-shapes drop-index)
-                (pcb/reorder-grid-children [frame-id])
+                (cond-> (ctl/grid-layout? objects frame-id)
+                  (-> (pcb/update-shapes [frame-id] ctl/assign-cell-positions {:with-objects? true})
+                      (pcb/reorder-grid-children [frame-id])))
                 (pcb/remove-objects empty-parents))]
 
         (when (and (some? frame-id) (d/not-empty? changes))

frontend/src/app/main/ui/workspace/sidebar/debug.cljs

@@ -5,6 +5,7 @@
 ;; Copyright (c) KALEIDOS INC
 
 (ns app.main.ui.workspace.sidebar.debug
+  (:require-macros [app.main.style :as stl])
   (:require
    [app.common.data :as d]
    [app.main.data.workspace :as dw]
@@ -24,26 +25,27 @@
            (dbg/toggle! option)
            (js* "app.main.reinit()")))
 
-        on-close
+        handle-close
         (mf/use-fn
          (fn []
            (st/emit! (dw/remove-layout-flag :debug-panel))))]
-    [:div.debug-panel
-     [:div.debug-panel-header
-      [:div.debug-panel-close-button
-       {:on-click on-close} i/close]
-      [:div.debug-panel-title "Debugging tools"]]
 
-     [:div.debug-panel-inner
+    [:div {:class (stl/css :debug-panel)}
+     [:div {:class (stl/css :panel-title)}
+      [:span "Debugging tools"]
+      [:div {:class (stl/css :close-button) :on-click handle-close}
+       i/close-refactor]]
+
+     [:div {:class (stl/css :debug-panel-inner)}
       (for [option (sort-by d/name dbg/options)]
-        [:div.debug-option {:key (d/name option)
-                            :on-click #(on-toggle-enabled % option)}
+        [:div {:class (stl/css :checkbox-wrapper)}
+         [:span {:class (stl/css-case :checkbox-icon true :global/checked (dbg/enabled? option))
+                 :on-click #(on-toggle-enabled % option)}
+          (when (dbg/enabled? option) i/status-tick-refactor)]
+
          [:input {:type "checkbox"
                   :id (d/name option)
+                  :key (d/name option)
                   :on-change #(on-toggle-enabled % option)
                   :checked (dbg/enabled? option)}]
-         [:div.field.check
-          (if (dbg/enabled? option)
-            [:span.checked i/checkbox-checked]
-            [:span.unchecked i/checkbox-unchecked])]
          [:label {:for (d/name option)} (d/name option)]])]]))

frontend/src/app/main/ui/workspace/sidebar/debug.scss

@@ -0,0 +1,59 @@
+// This Source Code Form is subject to the terms of the Mozilla Public
+// License, v. 2.0. If a copy of the MPL was not distributed with this
+// file, You can obtain one at http://mozilla.org/MPL/2.0/.
+//
+// Copyright (c) KALEIDOS INC
+
+@use "common/refactor/common-refactor.scss" as *;
+
+.debug-panel {
+  display: flex;
+  flex-direction: column;
+  background-color: var(--panel-background-color);
+}
+
+.panel-title {
+  @include flexCenter;
+  @include tabTitleTipography;
+  position: relative;
+  height: $s-32;
+  min-height: $s-32;
+  margin: $s-8 $s-8 0 $s-8;
+  border-radius: $br-8;
+  background-color: var(--panel-title-background-color);
+
+  span {
+    @include flexCenter;
+    flex-grow: 1;
+    color: var(--title-foreground-color-hover);
+  }
+}
+
+.close-button {
+  @extend .button-tertiary;
+  position: absolute;
+  right: $s-2;
+  top: $s-2;
+  height: $s-28;
+  width: $s-28;
+  border-radius: $br-6;
+  svg {
+    @extend .button-icon;
+    stroke: var(--icon-foreground);
+  }
+}
+
+.checkbox-wrapper {
+  @extend .input-checkbox;
+  height: $s-32;
+  padding: 0;
+}
+
+.checkbox-icon {
+  @extend .checkbox-icon;
+  cursor: pointer;
+}
+
+.debug-panel-inner {
+  padding: $s-16 $s-8;
+}

frontend/src/app/main/ui/workspace/viewport/grid_layout_editor.cljs

@@ -30,6 +30,7 @@
    [app.main.ui.hooks :as hooks]
    [app.main.ui.icons :as i]
    [app.main.ui.workspace.viewport.viewport-ref :as uwvv]
+   [app.util.debug :as dbg]
    [app.util.dom :as dom]
    [app.util.i18n :as i18n :refer [tr]]
    [app.util.keyboard :as kbd]
@@ -347,6 +348,29 @@
              (st/emit! (dw/show-grid-cell-context-menu {:position position :grid-id (:id shape)})))))]
 
     [:g.cell-editor
+     ;; DEBUG OVERLAY
+     (when (dbg/enabled? :grid-cells)
+       [:g.debug-cell {:pointer-events "none"
+                       :transform (dm/str (gmt/transform-in cell-center (:transform shape)))}
+
+        [:rect
+         {:x (:x cell-origin)
+          :y (:y cell-origin)
+          :width cell-width
+          :height cell-height
+          :fill (cond
+                  (= (:position cell) :auto) "green"
+                  (= (:position cell) :manual) "red"
+                  (= (:position cell) :area) "yellow"
+                  :else "black")
+          :fill-opacity 0.2}]
+
+        (when (seq (:shapes cell))
+          [:circle
+           {:cx (+ (:x cell-origin) cell-width (- (/ 7 zoom)))
+            :cy (+ (:y cell-origin) (/ 7 zoom))
+            :r (/ 5 zoom)
+            :fill "red"}])])
      [:rect
       {:transform (dm/str (gmt/transform-in cell-center (:transform shape)))
        :class (dom/classnames (stl/css :grid-cell-outline) true

frontend/src/app/util/debug.cljs

@@ -75,6 +75,9 @@
 
     ;;
     :grid-layout
+
+    ;; Show an overlay to the grid cells to know its properties
+    :grid-cells
     })
 
 (defn enable!

frontend/translations/en.po

@@ -5080,7 +5080,7 @@ msgid "workspace.context-menu.grid-track.row.add-before"
 msgstr "Add 1 row above"
 
 msgid "workspace.context-menu.grid-track.row.add-after"
-msgstr "Add 1 row bellow"
+msgstr "Add 1 row below"
 
 msgid "workspace.context-menu.grid-track.row.delete"
 msgstr "Delete row"