Merge pull request #3025 from penpot/alotor-bugfixes2

Bug fixes

CHANGES.md

@@ -1,6 +1,5 @@
 # CHANGELOG
 
-<<<<<<< HEAD
 ## :rocket: Next
 
 ### :boom: Breaking changes & Deprecations
@@ -22,6 +21,9 @@
 - 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)
+- Improve deeps selection of nested arboards [Taiga #4913](https://tree.taiga.io/project/penpot/issue/4913)
+- Fix problem on selection numeric inputs on Firefox [#2991](https://github.com/penpot/penpot/issues/2991)
+- Changed the text dominant-baseline to use ideographic [Taiga #4791](https://tree.taiga.io/project/penpot/issue/4791)
 
 ### :arrow_up: Deps updates
 
@@ -29,8 +31,6 @@
 - To @ondrejkonec: for contributing to the code with:
 - Refactor CSS variables [Github #2948](https://github.com/penpot/penpot/pull/2948)
 
-=======
->>>>>>> origin/staging
 ## 1.17.3
 
 ### :bug: Bugs fixed

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

@@ -24,9 +24,11 @@
   (and (= type :frame) (= id uuid/zero)))
 
 (defn root-frame?
-  [{:keys [frame-id type]}]
+  ([objects id]
-  (and (= type :frame)
+   (root-frame? (get objects id)))
-       (= frame-id uuid/zero)))
+  ([{:keys [frame-id type]}]
+   (and (= type :frame)
+        (= frame-id uuid/zero))))
 
 (defn frame-shape?
   ([objects id]

common/src/app/common/types/shape_tree.cljc

@@ -179,18 +179,22 @@
 
   ([objects ids {:keys [bottom-frames?] :as options}]
    (letfn [(comp [id-a id-b]
-             (let [type-a (dm/get-in objects [id-a :type])
+             (let [frame-a? (= :frame (dm/get-in objects [id-a :type]))
-                   type-b (dm/get-in objects [id-b :type])]
+                   frame-b? (= :frame (dm/get-in objects [id-b :type]))]
                (cond
-                 (and (not= :frame type-a) (= :frame type-b))
-                 (if bottom-frames? -1 1)
-
-                 (and (= :frame type-a) (not= :frame type-b))
-                 (if bottom-frames? 1 -1)
-
                  (= id-a id-b)
                  0
 
+                 (and (not frame-a?) frame-b?)
+                 (if bottom-frames? -1 1)
+
+                 (and frame-a? (not frame-b?))
+                 (if bottom-frames? 1 -1)
+
+                 ;; When comparing frames we invert the order if the flag `bottom-frames?` is on
+                 (and frame-a? frame-b? bottom-frames?)
+                 (if (is-shape-over-shape? objects id-b id-a) 1 -1)
+
                  (is-shape-over-shape? objects id-b id-a)
                  -1
 

frontend/src/app/main/fonts.cljs

@@ -194,47 +194,43 @@
 ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
 
 (defn ensure-loaded!
-  ([id]
+  [id]
-   (p/create (fn [resolve]
+  (log/debug :action "try-ensure-loaded!" :font-id id)
-               (ensure-loaded! id resolve))))
+  (if-not (exists? js/window)
-  ([id on-loaded]
+    ;; If we are in the worker environment, we just mark it as loaded
-   (log/debug :action "try-ensure-loaded!" :font-id id)
+    ;; without really loading it.
-   (if-not (exists? js/window)
+    (do
-     ;; If we are in the worker environment, we just mark it as loaded
+      (swap! loaded conj id)
-     ;; without really loading it.
+      (p/resolved id))
-     (do
-       (swap! loaded conj id)
-       (p/resolved id))
 
-     (when-let [font (get @fontsdb id)]
+    (let [font (get @fontsdb id)]
-       (cond
+      (cond
-         ;; Font already loaded, we just continue
+        (nil? font)
-         (contains? @loaded id)
+        (p/resolved id)
-         (p/do
-           (on-loaded id)
-           id)
 
-         ;; Font is currently downloading. We attach the caller to the promise
+        ;; Font already loaded, we just continue
-         (contains? @loading id)
+        (contains? @loaded id)
-         (-> (get @loading id)
+        (p/resolved id)
-             (p/then #(do (on-loaded id) id)))
 
-         ;; First caller, we create the promise and then wait
+        ;; Font is currently downloading. We attach the caller to the promise
-         :else
+        (contains? @loading id)
-         (let [on-load (fn [resolve]
+        (p/resolved (get @loading id))
-                         (swap! loaded conj id)
-                         (swap! loading dissoc id)
-                         (on-loaded id)
-                         (resolve id))
 
-               load-p (p/create
+        ;; First caller, we create the promise and then wait
-                       (fn [resolve _]
+        :else
-                         (-> font
+        (let [on-load (fn [resolve]
-                             (assoc ::on-loaded (partial on-load resolve))
+                        (swap! loaded conj id)
-                             (load-font))))]
+                        (swap! loading dissoc id)
+                        (resolve id))
 
-           (swap! loading assoc id load-p)
+              load-p (p/create
-           load-p))))))
+                      (fn [resolve _]
+                        (-> font
+                            (assoc ::on-loaded (partial on-load resolve))
+                            (load-font))))]
+
+          (swap! loading assoc id load-p)
+          load-p)))))
 
 (defn ready
   [cb]

frontend/src/app/main/ui/shapes/text/svg_text.cljs

@@ -69,16 +69,21 @@
 
      [:> :g group-props
       (for [[index data] (d/enumerate position-data)]
-        (let [alignment-bl (when (cf/check-browser? :safari) "text-before-edge")
+        (let [rtl? (= "rtl" (:direction data))
-              dominant-bl  (when-not (cf/check-browser? :safari) "text-before-edge")
+
-              rtl? (= "rtl" (:direction data))
+              browser-props
+              (cond
+                (cf/check-browser? :safari)
+                #js {:dominantBaseline "hanging"
+                     :dy "0.2em"
+                     :y (- (:y data) (:height data))})
+
               props (-> #js {:key (dm/str "text-" (:id shape) "-" index)
                              :x (if rtl? (+ (:x data) (:width data)) (:x data))
-                             :y (- (:y data) (:height data))
+                             :y (:y data)
+                             :dominantBaseline "ideographic"
                              :textLength (:width data)
                              :lengthAdjust "spacingAndGlyphs"
-                             :alignmentBaseline alignment-bl
-                             :dominantBaseline dominant-bl
                              :style (-> #js {:fontFamily (:font-family data)
                                              :fontSize (:font-size data)
                                              :fontWeight (:font-weight data)
@@ -88,7 +93,9 @@
                                              :fontStyle (:font-style data)
                                              :direction (:direction data)
                                              :whiteSpace "pre"}
-                                        (obj/set! "fill" (str "url(#fill-" index "-" render-id ")")))})
+                                        (obj/set! "fill" (str "url(#fill-" index "-" render-id ")")))}
+                        (cond-> browser-props
+                          (obj/merge! browser-props)))
               shape (assoc shape :fills (:fills data))]
 
           [:& (mf/provider muc/render-id) {:key index :value (str render-id "_" (:id shape) "_" index)}

frontend/src/app/main/ui/workspace/shapes/text.cljs

@@ -15,9 +15,54 @@
    [app.main.refs :as refs]
    [app.main.ui.shapes.shape :refer [shape-container]]
    [app.main.ui.shapes.text :as text]
+   [app.util.dom :as dom]
    [debug :refer [debug?]]
    [rumext.v2 :as mf]))
 
+(mf/defc debug-text-bounds
+  {::mf/wrap-props false}
+  [props]
+  (let [shape (unchecked-get props "shape")
+        zoom (mf/deref refs/selected-zoom)
+        bounding-box (gsht/position-data-selrect shape)
+        ctx (js* "document.createElement(\"canvas\").getContext(\"2d\")")]
+    [:g {:transform (gsh/transform-str shape)}
+     [:rect {:x (:x bounding-box)
+             :y (:y bounding-box)
+             :width (:width bounding-box)
+             :height (:height bounding-box)
+             :style {:fill "none"
+                     :stroke "orange"
+                     :stroke-width (/ 1 zoom)}}]
+
+     (for [[index data] (d/enumerate (:position-data shape))]
+       (let [{:keys [x y width height]} data
+             res (dom/measure-text ctx (:font-size data) (:font-family data) (:text data))]
+         [:g {:key (dm/str index)}
+          ;; Text fragment bounding box
+          [:rect {:x x
+                  :y (- y height)
+                  :width width
+                  :height height
+                  :style {:fill "none"
+                          :stroke "red"
+                          :stroke-width (/ 1 zoom)}}]
+
+          ;; Text baseline
+          [:line {:x1 (mth/round x)
+                  :y1 (mth/round (- (:y data) (:height data)))
+                  :x2 (mth/round (+ x width))
+                  :y2 (mth/round (- (:y data) (:height data)))
+                  :style {:stroke "blue"
+                          :stroke-width (/ 1 zoom)}}]
+
+          [:line {:x1 (:x data)
+                  :y1 (- (:y data) (:descent res))
+                  :x2 (+ (:x data) (:width data))
+                  :y2 (- (:y data) (:descent res))
+                  :style {:stroke "green"
+                          :stroke-width (/ 2 zoom)}}]]))]))
+
 ;; --- Text Wrapper for workspace
 (mf/defc text-wrapper
   {::mf/wrap-props false}
@@ -39,28 +84,4 @@
       [:& text/text-shape {:shape shape}]]
 
      (when (and (debug? :text-outline) (d/not-empty? (:position-data shape)))
-       [:g {:transform (gsh/transform-str shape)}
+       [:& debug-text-bounds {:shape shape}])]))
-        (let [bounding-box (gsht/position-data-selrect shape)]
-          [:rect {
-                  :x (:x bounding-box)
-                  :y (:y bounding-box)
-                  :width (:width bounding-box)
-                  :height (:height bounding-box)
-                  :style { :fill "none" :stroke "orange"}}])
-
-        (for [[index data] (d/enumerate (:position-data shape))]
-          (let [{:keys [x y width height]} data]
-            [:g {:key (dm/str index)}
-             ;; Text fragment bounding box
-             [:rect {:x x
-                     :y (- y height)
-                     :width width
-                     :height height
-                     :style {:fill "none" :stroke "red"}}]
-
-             ;; Text baseline
-             [:line {:x1 (mth/round x)
-                     :y1 (mth/round (- (:y data) (:height data)))
-                     :x2 (mth/round (+ x width))
-                     :y2 (mth/round (- (:y data) (:height data)))
-                     :style {:stroke "blue"}}]]))])]))

frontend/src/app/main/ui/workspace/sidebar/options/menus/interactions.cljs

@@ -336,7 +336,7 @@
             [:span.element-set-subtitle.wide (tr "workspace.options.interaction-delay")]
             [:div.input-element {:title (tr "workspace.options.interaction-ms")}
              [:> numeric-input {:ref ext-delay-ref
-                                :on-click (select-text ext-delay-ref)
+                                :on-focus (select-text ext-delay-ref)
                                 :on-change change-delay
                                 :value (:delay interaction)
                                 :title (tr "workspace.options.interaction-ms")}]
@@ -523,7 +523,7 @@
                 [:span.element-set-subtitle.wide (tr "workspace.options.interaction-duration")]
                 [:div.input-element {:title (tr "workspace.options.interaction-ms")}
                  [:> numeric-input {:ref ext-duration-ref
-                                    :on-click (select-text ext-duration-ref)
+                                    :on-focus (select-text ext-duration-ref)
                                     :on-change change-duration
                                     :value (-> interaction :animation :duration)
                                     :title (tr "workspace.options.interaction-ms")}]

frontend/src/app/main/ui/workspace/sidebar/options/menus/layer.cljs

@@ -113,7 +113,7 @@
        [:div.input-element {:title (tr "workspace.options.opacity") :class "percentail"}
         [:> numeric-input {:value (-> values :opacity opacity->string)
                            :placeholder (tr "settings.multiple")
-                           :on-click select-all
+                           :on-focus select-all
                            :on-change handle-opacity-change
                            :min 0
                            :max 100}]]

frontend/src/app/main/ui/workspace/sidebar/options/menus/layout_container.cljs

@@ -266,10 +266,11 @@
          [:span.icon.rotated i/auto-padding-both-sides]
          [:> numeric-input
           {:placeholder "--"
-           :on-click #(dom/select-target %)
            :on-change (partial on-change :simple :p1)
            :on-focus #(select-paddings true false true false)
-           :on-blur #(select-paddings false false false false)
+           :on-blur #(do
+                       (dom/select-target %)
+                       (select-paddings false false false false))
            :value p1}]]
 
         [:div.padding-item.tooltip.tooltip-bottom-left
@@ -277,9 +278,9 @@
          [:span.icon i/auto-padding-both-sides]
          [:> numeric-input
           {:placeholder "--"
-           :on-click #(dom/select-target %)
            :on-change (partial on-change :simple :p2)
-           :on-focus #(select-paddings false true false true)
+           :on-focus #(do (dom/select-target %)
+                          (select-paddings false true false true))
            :on-blur #(select-paddings false false false false)
            :value p2}]]]
 
@@ -296,9 +297,9 @@
            [:div.input-element.auto
             [:> numeric-input
              {:placeholder "--"
-              :on-click #(dom/select-target %)
               :on-change (partial on-change :multiple num)
-              :on-focus #(select-padding num)
+              :on-focus #(do (dom/select-target %)
+                             (select-padding num))
               :on-blur #(select-paddings false false false false)
               :value (num (:layout-padding values))}]]])])
 
@@ -320,7 +321,7 @@
       i/auto-gap]
      [:> numeric-input {:no-validate true
                         :placeholder "--"
-                        :on-click (fn [event]
+                        :on-focus (fn [event]
                                     (reset! gap-selected? :column-gap)
                                     (dom/select-target event))
                         :on-change (partial set-gap (= :nowrap wrap-type) :column-gap)
@@ -334,7 +335,7 @@
       i/auto-gap]
      [:> numeric-input {:no-validate true
                         :placeholder "--"
-                        :on-click (fn [event]
+                        :on-focus (fn [event]
                                     (reset! gap-selected? :row-gap)
                                     (dom/select-target event))
                         :on-change (partial set-gap (= :nowrap wrap-type) :row-gap)

frontend/src/app/main/ui/workspace/sidebar/options/menus/layout_item.cljs

@@ -58,7 +58,7 @@
          [:span.icon i/auto-margin-both-sides]
          [:> numeric-input
           {:placeholder "--"
-           :on-click #(dom/select-target %)
+           :on-focus #(dom/select-target %)
            :on-change (partial on-margin-change :simple :m1)
            :value m1}]]
 
@@ -67,7 +67,7 @@
          [:span.icon.rotated i/auto-margin-both-sides]
          [:> numeric-input
           {:placeholder "--"
-           :on-click #(dom/select-target %)
+           :on-focus #(dom/select-target %)
            :on-change (partial on-margin-change :simple :m2)
            :value m2}]]]
 
@@ -84,7 +84,7 @@
            [:div.input-element.auto
             [:> numeric-input
              {:placeholder "--"
-              :on-click #(dom/select-target %)
+              :on-focus #(dom/select-target %)
               :on-change (partial on-margin-change :multiple num)
               :value (num (:layout-item-margin values))}]]])])
 
@@ -231,7 +231,7 @@
            i/layers
            [:> numeric-input
             {:placeholder "--"
-             :on-click #(dom/select-target %)
+             :on-focus #(dom/select-target %)
              :on-change #(on-change-z-index %)
              :value (:layout-item-z-index values)}]]]])
 
@@ -281,7 +281,7 @@
                  :min 0
                  :data-wrap true
                  :placeholder "--"
-                 :on-click #(dom/select-target %)
+                 :on-focus #(dom/select-target %)
                  :on-change (partial on-size-change item)
                  :value (get values item)
                  :nillable true}]]])]]])]]))

frontend/src/app/main/ui/workspace/sidebar/options/menus/measures.cljs

@@ -316,7 +316,7 @@
            [:> numeric-input {:min 0.01
                               :no-validate true
                               :placeholder "--"
-                              :on-click select-all
+                              :on-focus select-all
                               :on-change on-width-change
                               :disabled disabled-width-sizing?
                               :value (:width values)}]]
@@ -325,7 +325,7 @@
            [:> numeric-input {:min 0.01
                               :no-validate true
                               :placeholder "--"
-                              :on-click select-all
+                              :on-focus select-all
                               :on-change on-height-change
                               :disabled disabled-height-sizing?
                               :value (:height values)}]]
@@ -345,14 +345,14 @@
           [:div.input-element.Xaxis {:title (tr "workspace.options.x")}
            [:> numeric-input {:no-validate true
                               :placeholder "--"
-                              :on-click select-all
+                              :on-focus select-all
                               :on-change on-pos-x-change
                               :disabled disabled-position-x?
                               :value (:x values)}]]
           [:div.input-element.Yaxis {:title (tr "workspace.options.y")}
            [:> numeric-input {:no-validate true
                               :placeholder "--"
-                              :on-click select-all
+                              :on-focus select-all
                               :disabled disabled-position-y?
                               :on-change on-pos-y-change
                               :value (:y values)}]]])
@@ -368,7 +368,7 @@
              :max 359
              :data-wrap true
              :placeholder "--"
-             :on-click select-all
+             :on-focus select-all
              :on-change on-rotation-change
              :value (:rotation values)}]]])
 
@@ -396,7 +396,7 @@
               {:placeholder "--"
                :ref radius-input-ref
                :min 0
-               :on-click select-all
+               :on-focus select-all
                :on-change on-radius-1-change
                :value (:rx values)}]]
 
@@ -406,7 +406,7 @@
               {:type "number"
                :placeholder "--"
                :min 0
-               :on-click select-all
+               :on-focus select-all
                :on-change on-radius-multi-change
                :value ""}]]
 
@@ -416,7 +416,7 @@
               [:> numeric-input
                {:placeholder "--"
                 :min 0
-                :on-click select-all
+                :on-focus select-all
                 :on-change on-radius-r1-change
                 :value (:r1 values)}]]
 
@@ -424,7 +424,7 @@
               [:> numeric-input
                {:placeholder "--"
                 :min 0
-                :on-click select-all
+                :on-focus select-all
                 :on-change on-radius-r2-change
                 :value (:r2 values)}]]
 
@@ -432,7 +432,7 @@
               [:> numeric-input
                {:placeholder "--"
                 :min 0
-                :on-click select-all
+                :on-focus select-all
                 :on-change on-radius-r3-change
                 :value (:r3 values)}]]
 
@@ -440,7 +440,7 @@
               [:> numeric-input
                {:placeholder "--"
                 :min 0
-                :on-click select-all
+                :on-focus select-all
                 :on-change on-radius-r4-change
                 :value (:r4 values)}]]])])
 

frontend/src/app/main/ui/workspace/sidebar/options/menus/shadow.cljs

@@ -150,7 +150,7 @@
         [:> numeric-input {:ref adv-offset-x-ref
                            :no-validate true
                            :placeholder "--"
-                           :on-click (select-text adv-offset-x-ref)
+                           :on-focus (select-text adv-offset-x-ref)
                            :on-change (update-attr index :offset-x valid-number? basic-offset-x-ref)
                            :value (:offset-x value)}]
         [:span.after (tr "workspace.options.shadow-options.offsetx")]]
@@ -159,7 +159,7 @@
         [:> numeric-input {:ref adv-offset-y-ref
                            :no-validate true
                            :placeholder "--"
-                           :on-click (select-text adv-offset-y-ref)
+                           :on-focus (select-text adv-offset-y-ref)
                            :on-change (update-attr index :offset-y valid-number? basic-offset-y-ref)
                            :value (:offset-y value)}]
         [:span.after (tr "workspace.options.shadow-options.offsety")]]]
@@ -169,7 +169,7 @@
         [:> numeric-input {:ref adv-blur-ref
                            :no-validate true
                            :placeholder "--"
-                           :on-click (select-text adv-blur-ref)
+                           :on-focus (select-text adv-blur-ref)
                            :on-change (update-attr index :blur valid-number? basic-blur-ref)
                            :min 0
                            :value (:blur value)}]
@@ -179,7 +179,7 @@
         [:> numeric-input {:ref adv-spread-ref
                            :no-validate true
                            :placeholder "--"
-                           :on-click (select-text adv-spread-ref)
+                           :on-focus (select-text adv-spread-ref)
                            :on-change (update-attr index :spread valid-number?)
                            :value (:spread value)}]
         [:span.after (tr "workspace.options.shadow-options.spread")]]]

frontend/src/app/main/ui/workspace/sidebar/options/rows/color_row.cljs

@@ -196,7 +196,7 @@
                                    ""
                                    (-> color :color uc/remove-hash))
                           :placeholder (tr "settings.multiple")
-                          :on-click select-all
+                          :on-focus select-all
                           :on-blur on-blur
                           :on-change handle-value-change}]]
 
@@ -206,7 +206,7 @@
            {:class (dom/classnames :percentail (not= (:opacity color) :multiple))}
            [:> numeric-input {:value (-> color :opacity opacity->string)
                               :placeholder (tr "settings.multiple")
-                              :on-click select-all
+                              :on-focus select-all
                               :on-blur on-blur
                               :on-change handle-opacity-change
                               :min 0

frontend/src/app/main/ui/workspace/sidebar/options/rows/stroke_row.cljs

@@ -100,7 +100,7 @@
          :value (-> (:stroke-width stroke) width->string)
          :placeholder (tr "settings.multiple")
          :on-change (on-stroke-width-change index)
-         :on-click select-all
+         :on-focus select-all
          :on-blur on-blur}]]
 
       [:select#style.input-select {:value (enum->string (:stroke-alignment stroke))

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

@@ -7,6 +7,7 @@
 (ns app.main.ui.workspace.viewport.hooks
   (:require
    [app.common.data :as d]
+   [app.common.data.macros :as dm]
    [app.common.geom.shapes :as gsh]
    [app.common.pages :as cp]
    [app.common.pages.helpers :as cph]
@@ -169,7 +170,8 @@
              ;; but the mouse has not been moved from its position.
              (->> mod-str
                   (rx/observe-on :async)
-                  (rx/map #(deref last-point-ref)))
+                  (rx/map #(deref last-point-ref))
+                  (rx/merge-map query-point))
 
              (->> move-stream
                   (rx/tap #(reset! last-point-ref %))
@@ -241,10 +243,17 @@
              remove-id?
              (into selected-with-parents remove-id-xf ids)
 
+             no-fill-nested-frames?
+             (fn [id]
+               (and (cph/frame-shape? objects id)
+                    (not (cph/root-frame? objects id))
+                    (empty? (dm/get-in objects [id :fills]))))
+
              hover-shape
              (->> ids
                   (remove remove-id?)
                   (remove (partial cph/hidden-parent? objects))
+                  (remove no-fill-nested-frames?)
                   (filter #(or (empty? focus) (cp/is-in-focus? objects focus %)))
                   (first)
                   (get objects))]

frontend/src/app/util/dom.cljs

@@ -662,3 +662,12 @@
 (defn has-children?
   [^js node]
   (> (-> node .-children .-length) 0))
+
+;; WARNING: Use only for debugging. It's to costly to use for real
+(defn measure-text
+  "Given a canvas' context 2d and the text info returns tis ascent/descent info"
+  [context-2d font-size font-family text]
+  (let [_ (set! (.-font context-2d) (str font-size " " font-family))
+        measures (.measureText context-2d text)]
+    {:descent (.-actualBoundingBoxDescent measures)
+     :ascent (.-actualBoundingBoxAscent measures)}))