Merge pull request #3019 from penpot/alotor-bugfixes

Alotor bugfixes

CHANGES.md

@@ -16,6 +16,11 @@
 ### :bug: Bugs fixed
 
 - Fix problem with rules position on changing pages [Taiga #4847](https://tree.taiga.io/project/penpot/issue/4847)
+- Fix error streen when uploading wrong SVG [#2995](https://github.com/penpot/penpot/issues/2995)
+- Fix selecting children from hidden parent layers [Taiga #4934](https://tree.taiga.io/project/penpot/issue/4934)
+- Fix problem when undoing multiple selected colors [Taiga #4920](https://tree.taiga.io/project/penpot/issue/4920)
+- Allow selection of empty board by partial rect [Taiga #4806](https://tree.taiga.io/project/penpot/issue/4806)
+- Improve behavior for undo on text edition [Taiga #4693](https://tree.taiga.io/project/penpot/issue/4693)
 
 ### :arrow_up: Deps updates
 

common/src/app/common/pages/changes_builder.cljc

@@ -35,6 +35,10 @@
   [changes save-undo?]
   (assoc changes :save-undo? save-undo?))
 
+(defn set-stack-undo?
+  [changes stack-undo?]
+  (assoc changes :stack-undo? stack-undo?))
+
 (defn with-page
   [changes page]
   (vary-meta changes assoc

common/src/app/common/pages/helpers.cljc

@@ -120,6 +120,16 @@
         (recur (conj result parent-id) parent-id)
         result))))
 
+(defn hidden-parent?
+  "Checks the parent for the hidden property"
+  [objects shape-id]
+  (let [parent-id (dm/get-in objects [shape-id :parent-id])]
+    (cond
+      (or (nil? parent-id) (nil? shape-id) (= shape-id uuid/zero) (= parent-id uuid/zero)) false
+      (dm/get-in objects [parent-id :hidden]) true
+      :else
+      (recur objects parent-id))))
+
 (defn get-parent-ids-with-index
   "Returns a tuple with the list of parents and a map with the position within each parent"
   [objects shape-id]

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

@@ -55,9 +55,8 @@
 
 (defn update-shapes
   ([ids update-fn] (update-shapes ids update-fn nil))
-  ([ids update-fn {:keys [reg-objects? save-undo? attrs ignore-tree page-id]
+  ([ids update-fn {:keys [reg-objects? save-undo? stack-undo? attrs ignore-tree page-id]
-                   :or {reg-objects? false save-undo? true}}]
+                   :or {reg-objects? false save-undo? true stack-undo? false}}]
-
    (us/assert ::coll-of-uuid ids)
    (us/assert fn? update-fn)
 
@@ -80,6 +79,7 @@
                             (pcb/update-shapes changes [id] update-fn opts)))
                         (-> (pcb/empty-changes it page-id)
                             (pcb/set-save-undo? save-undo?)
+                            (pcb/set-stack-undo? stack-undo?)
                             (pcb/with-objects objects))
                         ids)
              changes (add-group-id changes state)]
@@ -165,8 +165,8 @@
 
 (defn commit-changes
   [{:keys [redo-changes undo-changes
-           origin save-undo? file-id group-id]
+           origin save-undo? file-id group-id stack-undo?]
-    :or {save-undo? true}}]
+    :or {save-undo? true stack-undo? false}}]
   (log/debug :msg "commit-changes"
              :js/redo-changes redo-changes
              :js/undo-changes undo-changes)
@@ -183,6 +183,7 @@
          :page-id page-id
          :frames frames
          :save-undo? save-undo?
+         :stack-undo? stack-undo?
          :group-id group-id})
 
       ptk/UpdateEvent
@@ -233,4 +234,4 @@
                (let [entry {:undo-changes undo-changes
                             :redo-changes redo-changes
                             :group-id group-id}]
-                 (rx/of (dwu/append-undo entry)))))))))))
+                 (rx/of (dwu/append-undo entry stack-undo?)))))))))))

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

@@ -16,6 +16,7 @@
    [app.main.data.workspace.libraries :as dwl]
    [app.main.data.workspace.state-helpers :as wsh]
    [app.main.data.workspace.texts :as dwt]
+   [app.main.data.workspace.undo :as dwu]
    [app.util.color :as uc]
    [beicon.core :as rx]
    [potok.core :as ptk]))
@@ -350,6 +351,9 @@
   (ptk/reify ::change-color-in-selected
     ptk/WatchEvent
     (watch [_ _ _]
+      (let [undo-id (js/Symbol)]
+        (rx/concat
+         (rx/of (dwu/start-undo-transaction undo-id))
          (->> (rx/from shapes-by-color)
               (rx/map (fn [shape] (case (:prop shape)
                                     :fill (change-fill [(:shape-id shape)] new-color (:index shape))
@@ -357,7 +361,8 @@
                                     :shadow (change-shadow [(:shape-id shape)] new-color (:index shape))
                                     :content (dwt/update-text-with-function
                                               (:shape-id shape)
-                                           (partial change-text-color old-color new-color (:index shape))))))))))
+                                              (partial change-text-color old-color new-color (:index shape)))))))
+         (rx/of (dwu/commit-undo-transaction undo-id)))))))
 
 (defn apply-color-from-palette
   [color is-alt?]

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

@@ -374,7 +374,7 @@
   ([]
    (apply-modifiers nil))
 
-  ([{:keys [undo-transation? modifiers] :or {undo-transation? true}}]
+  ([{:keys [modifiers undo-transation? stack-undo?] :or {undo-transation? true stack-undo? false}}]
    (ptk/reify ::apply-modifiers
      ptk/WatchEvent
      (watch [_ state _]
@@ -412,6 +412,7 @@
                           (cond-> text-shape?
                             (update-grow-type shape)))))
                   {:reg-objects? true
+                   :stack-undo? stack-undo?
                    :ignore-tree ignore-tree
                    ;; Attributes that can change in the transform. This way we don't have to check
                    ;; all the attributes

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

@@ -202,7 +202,8 @@
             ids (->> ids (filter #(contains? objects %)))]
         (if (d/not-empty? ids)
           (let [modif-tree (dwm/create-modif-tree ids (ctm/reflow-modifiers))]
-            (rx/of (dwm/apply-modifiers {:modifiers modif-tree})))
+            (rx/of (dwm/apply-modifiers {:modifiers modif-tree
+                                         :stack-undo? true})))
           (rx/empty))))))
 
 (defn initialize

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

@@ -62,7 +62,9 @@
         height (get-in data [:attrs :height] 100)
         viewbox (get-in data [:attrs :viewBox] (str "0 0 " width " " height))
         [x y width height] (->> (str/split viewbox #"\s+")
-                                (map d/parse-double))]
+                                (map d/parse-double))
+        width (if (= width 0) 1 width)
+        height (if (= height 0) 1 height)]
     [(assert-valid-num :x x)
      (assert-valid-num :y y)
      (assert-valid-pos-num :width width)
@@ -483,7 +485,6 @@
 
 (defn create-svg-shapes
   [svg-data {:keys [x y]} objects frame-id parent-id selected center?]
-  (try
   (let [[vb-x vb-y vb-width vb-height] (svg-dimensions svg-data)
         x (mth/round
            (if center?
@@ -544,18 +545,14 @@
                 (d/enumerate (->> (:content svg-data)
                                   (mapv #(usvg/inherit-attributes root-attrs %)))))]
 
-      [new-shape new-children])
+    [new-shape new-children]))
-
-    (catch :default e
-      (.error js/console "Error SVG" e)
-      (rx/throw {:type :svg-parser
-                 :data e}))))
 
 (defn add-svg-shapes
   [svg-data position]
   (ptk/reify ::add-svg-shapes
     ptk/WatchEvent
     (watch [it state _]
+      (try
         (let [page-id  (:current-page-id state)
               objects  (wsh/lookup-page-objects state page-id)
               frame-id (ctst/top-nested-frame objects position)
@@ -565,8 +562,9 @@
               selected-frame? (and (= 1 (count selected))
                                    (= :frame (get-in objects [(first selected) :type])))
 
-            parent-id (if
+              parent-id
-                       (or selected-frame? (empty? selected)) frame-id
+              (if (or selected-frame? (empty? selected))
+                frame-id
                 (:parent-id base))
 
               [new-shape new-children]
@@ -596,4 +594,9 @@
                  (dch/commit-changes changes)
                  (dws/select-shapes (d/ordered-set (:id new-shape)))
                  (ptk/data-event :layout/update [(:id new-shape)])
-               (dwu/commit-undo-transaction undo-id))))))
+                 (dwu/commit-undo-transaction undo-id)))
+
+        (catch :default e
+          (.error js/console "Error SVG" e)
+          (rx/throw {:type :svg-parser
+                     :data e}))))))

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

@@ -414,7 +414,7 @@
 
           (let [ids (->> (keys props) (filter changed-text?))]
             (rx/of (dwu/start-undo-transaction undo-id)
-                   (dch/update-shapes ids update-fn {:reg-objects? true :save-undo? true})
+                   (dch/update-shapes ids update-fn {:reg-objects? true :stack-undo? true})
                    (ptk/data-event :layout/update ids)
                    (dwu/commit-undo-transaction undo-id))))))))
 
@@ -539,7 +539,7 @@
 
     ptk/WatchEvent
     (watch [_ _ _]
-      (rx/of (dwm/apply-modifiers)))))
+      (rx/of (dwm/apply-modifiers {:stack-undo? true})))))
 
 (defn commit-position-data
   []
@@ -558,7 +558,7 @@
                  (fn [shape]
                    (-> shape
                        (assoc :position-data (get position-data (:id shape)))))
-                 {:save-undo? false :reg-objects? false}))
+                 {:stack-undo? true :reg-objects? false}))
          (rx/of (fn [state]
                   (dissoc state ::update-position-data-debounce ::update-position-data))))))))
 

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

@@ -6,6 +6,7 @@
 
 (ns app.main.data.workspace.undo
   (:require
+   [app.common.data :as d]
    [app.common.pages.changes-spec :as pcs]
    [app.common.spec :as us]
    [cljs.spec.alpha :as s]
@@ -54,6 +55,17 @@
                                                     (dec MAX-UNDO-SIZE)))))
     state))
 
+(defn- stack-undo-entry
+  [state {:keys [undo-changes redo-changes] :as entry}]
+    (let [index (get-in state [:workspace-undo :index] -1)]
+    (if (>= index 0)
+      (update-in state [:workspace-undo :items index]
+                 (fn [item]
+                   (-> item
+                       (update :undo-changes #(into undo-changes %))
+                       (update :redo-changes #(into % redo-changes)))))
+      (add-undo-entry state entry))))
+
 (defn- accumulate-undo-entry
   [state {:keys [undo-changes redo-changes group-id]}]
   (-> state
@@ -62,13 +74,21 @@
       (assoc-in [:workspace-undo :transaction :group-id] group-id)))
 
 (defn append-undo
-  [entry]
+  [entry stack?]
   (us/assert ::undo-entry entry)
   (ptk/reify ::append-undo
     ptk/UpdateEvent
     (update [_ state]
-      (if (get-in state [:workspace-undo :transaction])
+      (cond
+        (and (get-in state [:workspace-undo :transaction])
+             (or (d/not-empty? (get-in state [:workspace-undo :transaction :undo-changes]))
+                 (d/not-empty? (get-in state [:workspace-undo :transaction :redo-changes]))))
         (accumulate-undo-entry state entry)
+
+        stack?
+        (stack-undo-entry state entry)
+
+        :else
         (add-undo-entry state entry)))))
 
 (def empty-tx
@@ -103,16 +123,6 @@
               (update :workspace-undo dissoc :transaction))
           state)))))
 
-(def pop-undo-into-transaction
-  (ptk/reify ::last-undo-into-transaction
-    ptk/UpdateEvent
-    (update [_ state]
-      (let [index (get-in state [:workspace-undo :index] -1)]
-
-        (cond-> state
-          (>= index 0) (accumulate-undo-entry (get-in state [:workspace-undo :items index]))
-          (>= index 0) (update-in [:workspace-undo :index] dec))))))
-
 (def reinitialize-undo
   (ptk/reify ::reset-undo
     ptk/UpdateEvent

frontend/src/app/main/store.cljs

@@ -46,7 +46,9 @@
 (defonce state
   (ptk/store {:resolve ptk/resolve
               :on-event on-event
-              :on-error (fn [e] (@on-error e))}))
+              :on-error (fn [e]
+                          (.log js/console "ERROR!!" e)
+                          (@on-error e))}))
 
 (defonce stream
   (ptk/input-stream state))

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

@@ -244,6 +244,7 @@
              hover-shape
              (->> ids
                   (remove remove-id?)
+                  (remove (partial cph/hidden-parent? objects))
                   (filter #(or (empty? focus) (cp/is-in-focus? objects focus %)))
                   (first)
                   (get objects))]

frontend/src/app/util/snap_data.cljs

@@ -82,7 +82,9 @@
         grid-y-data (get-grids-snap-points frame :y)]
 
     (cond-> page-data
-      (not (ctl/any-layout-descent? objects frame))
+      (and (not (ctl/any-layout-descent? objects frame))
+           (not (:hidden frame))
+           (not (cph/hidden-parent? objects frame-id)))
 
       (-> ;; Update root frame information
        (assoc-in [uuid/zero :objects-data frame-id] frame-data)
@@ -106,7 +108,9 @@
                                  :id (:id shape)
                                  :pt %)))]
     (cond-> page-data
-      (not (ctl/any-layout-descent? objects shape))
+      (and (not (ctl/any-layout-descent? objects shape))
+           (not (:hidden shape))
+           (not (cph/hidden-parent? objects (:id shape))))
       (-> (assoc-in [frame-id :objects-data (:id shape)] shape-data)
           (update-in [frame-id :x] (make-insert-tree-data shape-data :x))
           (update-in [frame-id :y] (make-insert-tree-data shape-data :y))))))
@@ -124,9 +128,11 @@
                                 :pt %)))]
     (if-let [frame-id (:frame-id guide)]
       ;; Guide inside frame, we add the information only on that frame
-      (-> page-data
+      (cond-> page-data
-          (assoc-in [frame-id :objects-data (:id guide)] guide-data)
+        (and (not (:hidden frame))
-          (update-in [frame-id (:axis guide)] (make-insert-tree-data guide-data (:axis guide))))
+             (not (cph/hidden-parent? objects frame-id)))
+        (-> (assoc-in [frame-id :objects-data (:id guide)] guide-data)
+            (update-in [frame-id (:axis guide)] (make-insert-tree-data guide-data (:axis guide)))))
 
       ;; Guide outside the frame. We add the information in the global guides data
       (-> page-data

frontend/src/app/util/svg.cljs

@@ -820,6 +820,10 @@
 (defn line->path [{:keys [attrs] :as node}]
   (let [tag :path
         {:keys [x1 y1 x2 y2]} attrs
+        x1 (or x1 0)
+        y1 (or y1 0)
+        x2 (or x2 0)
+        y2 (or y2 0)
         attrs (-> attrs
                   (dissoc :x1 :x2 :y1 :y2)
                   (assoc :d (str "M" x1 "," y1 " L" x2 "," y2)))]

frontend/src/app/worker/selection.cljs

@@ -131,7 +131,10 @@
 
                (or (not full-frame?)
                    (not= :frame (:type shape))
-                   (gsh/rect-contains-shape? rect shape))))
+                   (and (d/not-empty? (:shapes shape))
+                        (gsh/rect-contains-shape? rect shape))
+                   (and (empty? (:shapes shape))
+                        (gsh/overlaps? shape rect))1)))
 
         overlaps?
         (fn [shape]