🐛 Fix unexpected behavior of grid options on right sidebar.

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

@@ -8,7 +8,7 @@
   (:require
    [app.common.uuid :as uuid]))
 
-(def file-version 11)
+(def file-version 12)
 (def default-color "#b1b2b5") ;; $color-gray-20
 (def root uuid/zero)
 

common/src/app/common/pages/migrations.cljc

@@ -268,3 +268,16 @@
             (update page :objects #(d/mapm (partial update-object %) %)))]
 
     (update data :pages-index #(d/mapm update-page %))))
+
+
+(defmethod migrate 12
+  [data]
+  (letfn [(update-grid [_key grid]
+            (cond-> grid
+              (= :auto (:size grid))
+              (assoc :size nil)))
+
+          (update-page [_id page]
+            (d/update-in-when page [:options :saved-grids] #(d/mapm update-grid %)))]
+
+    (update data :pages-index #(d/mapm update-page %))))

common/src/app/common/types/page_options.cljc

@@ -15,11 +15,12 @@
 (s/def :artboard-grid.color/color ::us/string)
 (s/def :artboard-grid.color/opacity ::us/safe-number)
 
-(s/def :artboard-grid/size ::us/safe-integer)
+(s/def :artboard-grid/size (s/nilable ::us/safe-integer))
+(s/def :artboard-grid/item-length (s/nilable ::us/safe-number))
+
 (s/def :artboard-grid/color (s/keys :req-un [:artboard-grid.color/color
                                              :artboard-grid.color/opacity]))
 (s/def :artboard-grid/type #{:stretch :left :center :right})
-(s/def :artboard-grid/item-length (s/nilable ::us/safe-integer))
 (s/def :artboard-grid/gutter (s/nilable ::us/safe-integer))
 (s/def :artboard-grid/margin (s/nilable ::us/safe-integer))
 

frontend/resources/styles/main/partials/sidebar-element-options.scss

@@ -406,7 +406,7 @@
     border: 1px solid transparent;
     position: relative;
     height: 38px;
-    margin-right: $size-2;
+    // margin-left: $size-2;
     max-height: 30px;
     position: relative;
     width: 60%;
@@ -494,12 +494,10 @@
   .editable-select svg {
     fill: $color-gray-40;
   }
-  .dropdown-button {
-    top: 4px;
-  }
   .editable-select.input-option .input-text {
     padding: 0;
     padding-top: 0.18rem;
+    padding-left: 0.25rem;
   }
 
   .input-element {

frontend/src/app/main/ui/components/editable_select.cljs

@@ -16,7 +16,8 @@
    [app.util.timers :as timers]
    [rumext.alpha :as mf]))
 
-(mf/defc editable-select [{:keys [value type options class on-change placeholder on-blur] :as params}]
+(mf/defc editable-select
+  [{:keys [value type options class on-change placeholder on-blur] :as params}]
   (let [state (mf/use-state {:id (uuid/next)
                              :is-open? false
                              :current-value value
@@ -42,20 +43,23 @@
                         (when on-blur (on-blur))))
 
         as-key-value (fn [item] (if (map? item) [(:value item) (:label item)] [item item]))
-
-        labels-map (into {} (->> options (map as-key-value)))
-
+        labels-map   (into {} (map as-key-value) options)
         value->label (fn [value] (get labels-map value value))
 
-        set-value (fn [value]
-                    (swap! state assoc :current-value value)
-                    (when on-change (on-change value)))
+        set-value
+        (fn [value]
+          (swap! state assoc :current-value value)
+          (when on-change (on-change value)))
 
-        handle-change-input (fn [event]
-                              (let [value (-> event dom/get-target dom/get-value)
-                                    value (-> (or (d/parse-double value) value)
-                                              (math/precision 2))]
-                                (set-value value)))
+        ;; TODO: why this method supposes that all editable select
+        ;; works with numbers?
+
+        handle-change-input
+        (fn [event]
+          (let [value (-> event dom/get-target dom/get-value)
+                value (-> (or (d/parse-double value) value)
+                          (math/precision 2))]
+            (set-value value)))
 
         on-node-load
         (fn [node]
@@ -133,7 +137,7 @@
 
     [:div.editable-select {:class class
                            :ref on-node-load}
-     [:input.input-text {:value (or (-> @state :current-value value->label) "")
+     [:input.input-text {:value (or (some-> @state :current-value value->label) "")
                          :on-change handle-change-input
                          :on-key-down handle-key-down
                          :on-focus handle-focus
@@ -149,12 +153,12 @@
                                            :left (:left @state)
                                            :bottom (:bottom @state)}}
        (for [[index item] (map-indexed vector options)]
-         (cond
-           (= :separator item) [:hr {:key (str (:id @state) "-" index)}]
-           :else (let [[value label] (as-key-value item)]
-                   [:li.checked-element
-                    {:key (str (:id @state) "-" index)
-                     :class (when (= value (-> @state :current-value)) "is-selected")
-                     :on-click (select-item value)}
-                    [:span.check-icon i/tick]
-                    [:span label]])))]]]))
+         (if (= :separator item)
+           [:hr {:key (str (:id @state) "-" index)}]
+           (let [[value label] (as-key-value item)]
+             [:li.checked-element
+              {:key (str (:id @state) "-" index)
+               :class (when (= value (-> @state :current-value)) "is-selected")
+               :on-click (select-item value)}
+              [:span.check-icon i/tick]
+              [:span label]])))]]]))

frontend/src/app/main/ui/components/numeric_input.cljs

@@ -15,6 +15,11 @@
    [app.util.simple-math :as sm]
    [rumext.alpha :as mf]))
 
+(defn num? [val]
+  (and (number? val)
+       (not (math/nan? val))
+       (math/finite? val)))
+
 (mf/defc numeric-input
   {::mf/wrap-props false
    ::mf/forward-ref true}
@@ -25,32 +30,31 @@
         wrap-value? (obj/get props "data-wrap")
         on-change   (obj/get props "onChange")
         title       (obj/get props "title")
+        default-val (obj/get props "default" 0)
 
         ;; We need a ref pointing to the input dom element, but the user
         ;; of this component may provide one (that is forwarded here).
         ;; So we use the external ref if provided, and the local one if not.
-        local-ref (mf/use-ref)
-        ref (or external-ref local-ref)
+        local-ref  (mf/use-ref)
+        ref        (or external-ref local-ref)
 
-        value (d/parse-integer value-str 0)
+        ;; This `value` represents the previous value and is used as
+        ;; initil value for the simple math expression evaluation.
+        value      (d/parse-integer value-str default-val)
 
-        min-val (cond
-                  (number? min-val-str)
-                  min-val-str
+        min-val    (cond
+                     (number? min-val-str)
+                     min-val-str
 
-                  (string? min-val-str)
-                  (d/parse-integer min-val-str))
+                     (string? min-val-str)
+                     (d/parse-integer min-val-str))
 
-        max-val (cond
-                  (number? max-val-str)
-                  max-val-str
+        max-val    (cond
+                     (number? max-val-str)
+                     max-val-str
 
-                  (string? max-val-str)
-                  (d/parse-integer max-val-str))
-
-        num? (fn [val] (and (number? val)
-                            (not (math/nan? val))
-                            (math/finite? val)))
+                     (string? max-val-str)
+                     (d/parse-integer max-val-str))
 
         parse-value
         (mf/use-callback
@@ -82,10 +86,9 @@
         (mf/use-callback
           (mf/deps on-change update-input value)
           (fn [new-value]
-            (when (and (some? new-value) (not= new-value value) (some? on-change))
+            (when (and (not= new-value value) (some? on-change))
               (on-change new-value))
-            (when (some? new-value)
-              (update-input new-value))))
+            (update-input new-value)))
 
         set-delta
         (mf/use-callback
@@ -143,10 +146,10 @@
         (mf/use-callback
           (mf/deps parse-value apply-value update-input)
           (fn [_]
-            (let [new-value (parse-value)]
+            (let [new-value (or (parse-value) default-val)]
               (if new-value
                 (apply-value new-value)
-                (update-input value-str)))))
+                (update-input new-value)))))
 
         props (-> props
                   (obj/without ["value" "onChange"])
@@ -159,12 +162,5 @@
                   (obj/set! "onKeyDown" handle-key-down)
                   (obj/set! "onBlur" handle-blur))]
 
-    (mf/use-effect
-     (mf/deps value-str)
-     (fn []
-       (when-let [input-node (mf/ref-val ref)]
-         (when-not (dom/active? input-node)
-           (dom/set-value! input-node value-str)))))
-
     [:> :input props]))
 

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

@@ -16,8 +16,7 @@
    [app.main.ui.icons :as i]
    [app.main.ui.workspace.sidebar.options.common :refer [advanced-options]]
    [app.main.ui.workspace.sidebar.options.rows.color-row :refer [color-row]]
-   [app.main.ui.workspace.sidebar.options.rows.input-row :refer [input-row]]
-   [app.util.data :as d]
+   [app.main.ui.workspace.sidebar.options.rows.input-row :refer [input-row input-row-v2]]
    [app.util.geom.grid :as gg]
    [app.util.i18n :as i18n :refer [tr]]
    [okulary.core :as l]
@@ -27,90 +26,101 @@
   (l/derived :saved-grids refs/workspace-page-options))
 
 (defn- get-size-options []
-  [{:value :auto :label (tr "workspace.options.grid.auto")}
+  [{:value nil :label (tr "workspace.options.grid.auto")}
    :separator
    18 12 10 8 6 4 3 2])
 
 (mf/defc grid-options
-  [{:keys [grid frame default-grid-params on-change on-remove on-save-grid]}]
-  (let [size-options (get-size-options)
-        state (mf/use-state {:show-advanced-options false})
+  {::mf/wrap [mf/memo]}
+  [{:keys [shape-id index grid frame-width frame-height default-grid-params]}]
+  (let [on-change       (mf/use-fn (mf/deps shape-id index) #(st/emit! (dw/set-frame-grid shape-id index %)))
+        on-remove       (mf/use-fn (mf/deps shape-id index) #(st/emit! (dw/remove-frame-grid shape-id index)))
+        on-save-default (mf/use-fn #(st/emit! (dw/set-default-grid (:type %) (:params %))))
+
+        size-options    (mf/use-memo get-size-options)
+        state           (mf/use-state {:show-advanced-options false})
+
         {:keys [type display params]} grid
 
         toggle-advanced-options
-        #(swap! state update :show-advanced-options not)
+        (mf/use-fn #(swap! state update :show-advanced-options not))
 
         handle-toggle-visibility
-        (fn [_]
-          (when on-change
-            (on-change (update grid :display #(if (nil? %) false (not %))))))
-
-        handle-remove-grid
-        (fn [_]
-          (when on-remove (on-remove)))
+        (mf/use-fn
+         (mf/deps grid)
+         (fn [_]
+           (on-change (update grid :display #(if (nil? %) false (not %))))))
 
         handle-change-type
-        (fn [grid-type]
-          (let [defaults (grid-type default-grid-params)]
-            (when on-change
-              (on-change (assoc grid
-                                :type grid-type
-                                :params defaults)))))
-
+        (mf/use-fn
+         (mf/deps grid)
+         (fn [grid-type]
+           (let [defaults (grid-type default-grid-params)]
+             (on-change (assoc grid
+                               :type grid-type
+                               :params defaults)))))
         handle-change
-        (fn [& keys]
+        (fn [& keys-path]
           (fn [value]
-            (when on-change
-              (on-change (assoc-in grid keys value)))))
+            (on-change (assoc-in grid keys-path value))))
 
+        ;; TODO: remove references to :auto
         handle-change-size
-        (fn [size]
-          (let [{:keys [margin gutter item-length]} (:params grid)
-                frame-length (if (= :column (:type grid)) (:width frame) (:height frame))
-                item-length (if (or (nil? size) (= :auto size))
-                              (-> (gg/calculate-default-item-length frame-length margin gutter)
-                                  (mth/precision 2))
-                              item-length)]
-            (when on-change
-              (on-change (-> grid
-                             (assoc-in [:params :size] size)
-                             (assoc-in [:params :item-length] item-length))))))
+        (mf/use-fn
+         (mf/deps grid)
+         (fn [size]
+           (let [{:keys [margin gutter item-length]} (:params grid)
+                 frame-length (if (= :column (:type grid)) frame-width frame-height)
+                 item-length  (if (nil? size)
+                                (-> (gg/calculate-default-item-length frame-length margin gutter)
+                                    (mth/precision 2))
+                                item-length)]
+
+             (-> grid
+                 (update :params assoc :size size :item-length item-length)
+                 (on-change)))))
 
         handle-change-item-length
-        (fn [item-length]
-          (let [size (get-in grid [:params :size])
-                size (if (and (nil? item-length) (or (nil? size) (= :auto size))) 12 size)]
-            (when on-change
-              (on-change (-> grid
-                             (assoc-in [:params :size] size)
-                             (assoc-in [:params :item-length] item-length))))))
+        (mf/use-fn
+         (mf/deps grid)
+         (fn [item-length]
+           (let [item-length (if (zero? item-length) nil item-length)
+                 size (get-in grid [:params :size])
+                 size (if (and (nil? item-length) (nil? size)) 12 size)]
+             (-> grid
+                 (update :params assoc :size size :item-length item-length)
+                 (on-change)))))
 
         handle-change-color
-        (fn [color]
-          (when on-change
-            (on-change (assoc-in grid [:params :color] color))))
+        (mf/use-fn
+         (mf/deps grid)
+         (fn [color]
+           (-> grid
+               (update :params assoc :color color)
+               (on-change))))
 
         handle-detach-color
-        (fn []
-          (when on-change
-            (on-change (-> grid
-                           (d/dissoc-in [:params :color :id])
-                           (d/dissoc-in [:params :color :file-id])))))
+        (mf/use-fn
+         (mf/deps grid)
+         (fn []
+           (-> grid
+               (update-in [:params :color] dissoc :id :file-id)
+               (on-change))))
 
         handle-use-default
-        (fn []
-          (let [params ((:type grid) default-grid-params)
-                color (or (get-in params [:color :value]) (get-in params [:color :color]))
-                params (-> params
-                           (assoc-in [:color :color] color)
-                           (update :color dissoc :value))]
-            (when on-change
-              (on-change (assoc grid :params params)))))
+        (mf/use-fn
+         (mf/deps grid)
+         (fn []
+           (let [params ((:type grid) default-grid-params)
+                 color (or (get-in params [:color :value]) (get-in params [:color :color]))
+                 params (-> params
+                            (assoc-in [:color :color] color)
+                            (update :color dissoc :value))]
+             (when on-change
+               (on-change (assoc grid :params params))))))
 
         handle-set-as-default
-        (fn []
-          (when on-save-grid
-            (on-save-grid grid)))
+        (mf/use-fn (mf/deps grid) #(on-save-default grid))
 
         is-default (= (->> grid :params)
                       (->> grid :type default-grid-params))
@@ -122,21 +132,23 @@
       [:button.custom-button {:class (when open? "is-active")
                               :on-click toggle-advanced-options} i/actions]
 
-      [:& select {:class "flex-grow"
-                  :default-value type
-                  :options [{:value :square :label (tr "workspace.options.grid.square")}
-                            {:value :column :label (tr "workspace.options.grid.column")}
-                            {:value :row :label (tr "workspace.options.grid.row")}]
-                  :on-change handle-change-type}]
+      [:& select
+       {:class "flex-grow"
+        :default-value type
+        :options [{:value :square :label (tr "workspace.options.grid.square")}
+                  {:value :column :label (tr "workspace.options.grid.column")}
+                  {:value :row :label (tr "workspace.options.grid.row")}]
+        :on-change handle-change-type}]
 
       (if (= type :square)
         [:div.input-element.pixels {:title (tr "workspace.options.size")}
          [:> numeric-input {:min 1
+                            :value (or (:size params) "")
                             :no-validate true
-                            :value (:size params)
                             :on-change (handle-change :params :size)}]]
+
         [:& editable-select {:value (:size params)
-                             :type (when (number? (:size params)) "number" )
+                             :type  "number"
                              :class "input-option"
                              :min 1
                              :options size-options
@@ -145,10 +157,9 @@
 
       [:div.grid-option-main-actions
        [:button.custom-button {:on-click handle-toggle-visibility} (if display i/eye i/eye-closed)]
-       [:button.custom-button {:on-click handle-remove-grid} i/minus]]]
+       [:button.custom-button {:on-click on-remove} i/minus]]]
 
-     [:& advanced-options {:visible? open?
-                           :on-close toggle-advanced-options}
+     [:& advanced-options {:visible? open? :on-close toggle-advanced-options}
       [:button.custom-button {:on-click toggle-advanced-options} i/actions]
       (when (= :square type)
         [:& input-row {:label (tr "workspace.options.grid.params.size")
@@ -190,13 +201,16 @@
                        :on-change (handle-change :params :type)}])
 
       (when (#{:row :column} type)
-        [:& input-row {:label (if (= :row type)
-                                (tr "workspace.options.grid.params.height")
-                                (tr "workspace.options.grid.params.width"))
-                       :class "pixels"
-                       :placeholder "Auto"
-                       :value (or (:item-length params) "")
-                       :on-change handle-change-item-length}])
+        [:& input-row-v2
+         {:class "pixels"
+          :label (if (= :row type)
+                   (tr "workspace.options.grid.params.height")
+                   (tr "workspace.options.grid.params.width"))}
+         [:> numeric-input
+          {:placeholder "Auto"
+           :value (or (:item-length params) "")
+           :default nil
+           :on-change handle-change-item-length}]])
 
       (when (#{:row :column} type)
         [:*
@@ -226,11 +240,9 @@
 
 (mf/defc frame-grid [{:keys [shape]}]
   (let [id (:id shape)
-        default-grid-params (merge dw/default-grid-params (mf/deref workspace-saved-grids))
-        handle-create-grid #(st/emit! (dw/add-frame-grid id))
-        handle-remove-grid (fn [index] #(st/emit! (dw/remove-frame-grid id index)))
-        handle-edit-grid (fn [index] #(st/emit! (dw/set-frame-grid id index %)))
-        handle-save-grid (fn [grid] (st/emit! (dw/set-default-grid (:type grid) (:params grid))))]
+        saved-grids         (mf/deref workspace-saved-grids)
+        default-grid-params (mf/use-memo (mf/deps saved-grids) #(merge dw/default-grid-params saved-grids))
+        handle-create-grid  (mf/use-fn (mf/deps id) #(st/emit! (dw/add-frame-grid id)))]
     [:div.element-set
      [:div.element-set-title
       [:span (tr "workspace.options.grid.title")]
@@ -239,12 +251,13 @@
      (when (seq (:grids shape))
        [:div.element-set-content
         (for [[index grid] (map-indexed vector (:grids shape))]
-          [:& grid-options {:key (str (:id shape) "-" index)
-                              :grid grid
-                              :default-grid-params default-grid-params
-                              :frame shape
-                              :on-change (handle-edit-grid index)
-                              :on-remove (handle-remove-grid index)
-                              :on-save-grid handle-save-grid}])])]))
+          [:& grid-options {:key (str id "-" index)
+                            :shape-id id
+                            :grid grid
+                            :index index
+                            :frame-width (:width shape)
+                            :frame-height (:height shape)
+                            :default-grid-params default-grid-params
+                            }])])]))
 
 

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

@@ -9,6 +9,7 @@
    [app.main.ui.components.editable-select :refer [editable-select]]
    [app.main.ui.components.numeric-input :refer [numeric-input]]
    [app.main.ui.components.select :refer [select]]
+   [app.util.object :as obj]
    [rumext.alpha :as mf]))
 
 (mf/defc input-row [{:keys [label options value class min max on-change type placeholder]}]
@@ -27,7 +28,7 @@
       [:& editable-select {:value value
                            :class "input-option"
                            :options options
-                           :type (when (number? value) "number")
+                           :type "number"
                            :min min
                            :max max
                            :placeholder placeholder
@@ -38,9 +39,28 @@
                :class "input-text"
                :on-change on-change} ]
 
-      [:> numeric-input {:placeholder placeholder
-                         :min min
-                         :max max
-                         :on-change on-change
-                         :value (or value "")}])]])
+      [:> numeric-input
+       {:placeholder placeholder
+        :min min
+        :max max
+        :on-change on-change
+        :value (or value "")}])]])
+
+
+;; NOTE: (by niwinz) this is a new version of input-row, I didn't
+;; touched the original one because it is used in many sites and I
+;; don't have intention to refactor all the code right now. We should
+;; consider to use the new one and once we have migrated all to the
+;; new component, we can proceed to rename it and delete the old one.
+
+(mf/defc input-row-v2
+  {::mf/wrap-props false}
+  [props]
+  (let [label    (obj/get props "label")
+        class    (obj/get props "class")
+        children (obj/get props "children")]
+    [:div.row-flex.input-row
+     [:span.element-set-subtitle label]
+     [:div.input-element {:class class}
+      children]]))
 

frontend/src/app/util/simple_math.cljs

@@ -28,9 +28,8 @@
   (let [token (first tree)
         args  (rest tree)]
     (case token
-
       :opt-expr
-      (if (empty? args) 0 (interpret (first args) init-value))
+      (if (empty? args) nil (interpret (first args) init-value))
 
       :expr
       (if (index-of "+-*/" (first args))
@@ -87,8 +86,9 @@
 (defn expr-eval
   [expr init-value]
   (s/assert string? expr)
-  (s/assert number? init-value)
-  (let [result (parser expr)]
+  (let [result     (parser expr)
+        init-value (or init-value 0)]
+    (s/assert number? init-value)
     (if-not (insta/failure? result)
       (interpret result init-value)
       (let [text (:text result)