✨ Control malformed variant formulas (#6473)

* ✨ Control malformed variant strings

* 📎 PR changes

* 📎 PR changes

common/src/app/common/logic/variant_properties.cljc

@@ -60,6 +60,17 @@
         (pcb/update-shapes [main-id] #(assoc % :variant-name name)))))
 
 
+(defn generate-set-variant-error
+  [changes component-id value]
+  (let [data      (pcb/get-library-data changes)
+        component (ctcl/get-component data component-id true)
+        main-id   (:main-instance-id component)]
+    (-> changes
+        (pcb/update-shapes [main-id] (if (str/blank? value)
+                                       #(dissoc % :variant-error)
+                                       #(assoc % :variant-error value))))))
+
+
 (defn generate-add-new-property
   [changes variant-id & {:keys [fill-values? property-name]}]
   (let [data               (pcb/get-library-data changes)

common/src/app/common/types/variant.cljc

@@ -33,7 +33,8 @@
   ;; The root shape of the main instance of a variant component.
   [:map
    [:variant-id {:optional true} ::sm/uuid]
-   [:variant-name {:optional true} :string]])
+   [:variant-name {:optional true} :string]
+   [:variant-error {:optional true} :string]])
 
 (def schema:variant-container
   ;; is a board that contains all variant components of a variant set,
@@ -106,7 +107,7 @@
      (add-new-props assigned remaining))))
 
 
-(defn properties-map-to-string
+(defn properties-map->string
   "Transforms a map of properties to a string of properties omitting the empty ones"
   [properties]
   (->> properties
@@ -116,11 +117,12 @@
        (str/join ", ")))
 
 
-(defn properties-string-to-map
+(defn properties-string->map
   "Transforms a string of properties to a map of properties"
   [s]
   (->> (str/split s ",")
        (mapv #(str/split % "="))
+       (filter (fn [[_ v]] (not (str/blank? (str/trim v)))))
        (mapv (fn [[k v]]
                {:name (str/trim k)
                 :value (str/trim v)}))))
@@ -129,7 +131,7 @@
 (defn valid-properties-string?
   "Checks if a string of properties has a processable format or not"
   [s]
-  (let [pattern #"^([a-zA-Z0-9\s]+=[a-zA-Z0-9\s]+)(,\s*[a-zA-Z0-9\s]+=[a-zA-Z0-9\s]+)*$"]
+  (let [pattern #"^\s*([a-zA-Z0-9_ -]+=[^,]*)(,\s*[a-zA-Z0-9_ -]+=[^,]*)*\s*$"]
     (not (nil? (re-matches pattern s)))))
 
 

common/test/common_tests/variant_test.cljc

@@ -12,28 +12,37 @@
 (t/deftest convert-between-variant-properties-maps-and-strings
   (let [map-with-two-props           [{:name "border" :value "yes"} {:name "color" :value "gray"}]
         map-with-two-props-one-blank [{:name "border" :value "no"} {:name "color" :value ""}]
+        map-with-two-props-dashes    [{:name "border" :value "no"} {:name "color" :value "--"}]
         map-with-one-prop            [{:name "border" :value "no"}]
-        map-with-spaces              [{:name "border 1" :value "of course"} {:name "color 2" :value "dark gray"}]
+        map-with-spaces              [{:name "border 1" :value "of course"}
+                                      {:name "color 2" :value "dark gray"}
+                                      {:name "background 3" :value "anoth€r co-lor"}]
 
         string-valid-with-two-props  "border=yes, color=gray"
         string-valid-with-one-prop   "border=no"
-        string-valid-with-spaces     "border 1=of course, color 2=dark gray"
-        string-invalid               "border=yes, color="]
+        string-valid-with-spaces     "border 1=of course, color 2=dark gray, background 3=anoth€r co-lor"
+        string-valid-with-no-value   "border=no, color="
+        string-valid-with-dashes     "border=no, color=--"
+        string-invalid               "border=yes, color"]
 
     (t/testing "convert map to string"
-      (t/is (= (ctv/properties-map-to-string map-with-two-props) string-valid-with-two-props))
-      (t/is (= (ctv/properties-map-to-string map-with-two-props-one-blank) string-valid-with-one-prop))
-      (t/is (= (ctv/properties-map-to-string map-with-spaces) string-valid-with-spaces)))
+      (t/is (= (ctv/properties-map->string map-with-two-props) string-valid-with-two-props))
+      (t/is (= (ctv/properties-map->string map-with-two-props-one-blank) string-valid-with-one-prop))
+      (t/is (= (ctv/properties-map->string map-with-spaces) string-valid-with-spaces)))
 
     (t/testing "convert string to map"
-      (t/is (= (ctv/properties-string-to-map string-valid-with-two-props) map-with-two-props))
-      (t/is (= (ctv/properties-string-to-map string-valid-with-one-prop) map-with-one-prop))
-      (t/is (= (ctv/properties-string-to-map string-valid-with-spaces) map-with-spaces)))
+      (t/is (= (ctv/properties-string->map string-valid-with-two-props) map-with-two-props))
+      (t/is (= (ctv/properties-string->map string-valid-with-one-prop) map-with-one-prop))
+      (t/is (= (ctv/properties-string->map string-valid-with-no-value) map-with-one-prop))
+      (t/is (= (ctv/properties-string->map string-valid-with-dashes) map-with-two-props-dashes))
+      (t/is (= (ctv/properties-string->map string-valid-with-spaces) map-with-spaces)))
 
     (t/testing "check if a string is valid"
       (t/is (= (ctv/valid-properties-string? string-valid-with-two-props) true))
       (t/is (= (ctv/valid-properties-string? string-valid-with-one-prop) true))
       (t/is (= (ctv/valid-properties-string? string-valid-with-spaces) true))
+      (t/is (= (ctv/valid-properties-string? string-valid-with-no-value) true))
+      (t/is (= (ctv/valid-properties-string? string-valid-with-dashes) true))
       (t/is (= (ctv/valid-properties-string? string-invalid) false)))))
 
 

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

@@ -130,6 +130,28 @@
          (dwu/commit-undo-transaction undo-id))))))
 
 
+(defn update-error
+  "Updates the error in a component"
+  [component-id value]
+  (ptk/reify ::update-error
+    ptk/WatchEvent
+    (watch [it state _]
+      (let [page-id    (:current-page-id state)
+            data       (dsh/lookup-file-data state)
+            objects    (-> (dsh/get-page data page-id)
+                           (get :objects))
+
+            changes    (-> (pcb/empty-changes it page-id)
+                           (pcb/with-library-data data)
+                           (pcb/with-objects objects)
+                           (clvp/generate-set-variant-error component-id value))
+            undo-id    (js/Symbol)]
+        (rx/of
+         (dwu/start-undo-transaction undo-id)
+         (dch/commit-changes changes)
+         (dwu/commit-undo-transaction undo-id))))))
+
+
 (defn remove-property
   "Remove the variant property on the position pos
    in all the components with this variant-id"

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

@@ -61,6 +61,7 @@
         is-variant-container? (when variants? (ctk/is-variant-container? item))
         variant-id            (when is-variant? (:variant-id item))
         variant-name          (when is-variant? (:variant-name item))
+        variant-error         (when is-variant? (:variant-error item))
 
         data                  (deref refs/workspace-data)
         component             (ctkl/get-component data (:component-id item))
@@ -144,6 +145,7 @@
                         :variant-id variant-id
                         :variant-name variant-name
                         :variant-properties variant-properties
+                        :variant-error variant-error
                         :component-id (:id component)
                         :is-hidden hidden?}]
 

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

@@ -31,8 +31,8 @@
    ::mf/forward-ref true}
   [{:keys [shape-id shape-name is-shape-touched disabled-double-click
            on-start-edit on-stop-edit depth parent-size is-selected
-           type-comp type-frame variant-id variant-name variant-properties
-           component-id is-hidden is-blocked]} external-ref]
+           type-comp type-frame component-id is-hidden is-blocked
+           variant-id variant-name variant-properties variant-error]} external-ref]
   (let [edition*         (mf/use-state false)
         edition?         (deref edition*)
 
@@ -41,9 +41,12 @@
 
         shape-for-rename (mf/deref lens:shape-for-rename)
 
-        shape-name       (d/nilv variant-name shape-name)
+        shape-name       (if variant-id
+                           (d/nilv variant-error variant-name)
+                           shape-name)
+
         default-value    (if variant-id
-                           (ctv/properties-map-to-string variant-properties)
+                           (or variant-error (ctv/properties-map->string variant-properties))
                            shape-name)
 
         has-path?        (str/includes? shape-name "/")
@@ -67,9 +70,11 @@
              (on-stop-edit)
              (reset! edition* false)
              (if variant-name
-               (let [valid? (ctv/valid-properties-string? name)
-                     props  (if valid? (ctv/properties-string-to-map name) {})]
-                 (st/emit! (dwv/update-properties-names-and-values component-id variant-id variant-properties props)))
+               (if (ctv/valid-properties-string? name)
+                 (st/emit! (dwv/update-properties-names-and-values component-id variant-id variant-properties (ctv/properties-string->map name))
+                           (dwv/update-error component-id nil))
+                 (st/emit! (dwv/update-properties-names-and-values component-id variant-id variant-properties {})
+                           (dwv/update-error component-id name)))
                (st/emit! (dw/end-rename-shape shape-id name))))))
 
         cancel-edit

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

@@ -10,6 +10,8 @@
   @include textEllipsis;
   @include bodySmallTypography;
   flex-grow: 1;
+  height: 100%;
+  align-content: center;
   color: var(--context-hover-color, var(--layer-row-foreground-color));
 
   &.selected {

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

@@ -7,6 +7,7 @@
 (ns app.main.ui.workspace.sidebar.options.menus.component
   (:require-macros [app.main.style :as stl])
   (:require
+   [app.common.data :as d]
    [app.common.data.macros :as dm]
    [app.common.files.helpers :as cfh]
    [app.common.files.variant :as cfv]
@@ -30,6 +31,7 @@
    [app.main.ui.context :as ctx]
    [app.main.ui.ds.buttons.icon-button :refer [icon-button*]]
    [app.main.ui.ds.controls.combobox :refer [combobox*]]
+   [app.main.ui.ds.foundations.assets.icon :refer [icon*]]
    [app.main.ui.ds.product.input-with-meta :refer [input-with-meta*]]
    [app.main.ui.hooks :as h]
    [app.main.ui.icons :as i]
@@ -234,11 +236,14 @@
           [:div {:class (stl/css  :counter)} (str size "/300")])]])))
 
 (mf/defc component-variant-main-instance*
-  [{:keys [components data]}]
-  (let [component    (first components)
-        variant-id   (:variant-id component)
-        objects      (-> (dsh/get-page data (:main-instance-page component))
-                         (get :objects))
+  [{:keys [components shape data]}]
+  (let [component      (first components)
+
+        variant-id     (:variant-id component)
+        variant-error? (:variant-error shape)
+
+        objects        (-> (dsh/get-page data (:main-instance-page component))
+                           (get :objects))
 
         properties-map (mapv :variant-properties components)
         component-ids  (mapv :id components)
@@ -248,6 +253,9 @@
 
         prop-vals       (mf/with-memo [data objects variant-id]
                           (cfv/extract-properties-values data objects variant-id))
+
+        empty-indicator "--"
+
         get-options
         (mf/use-fn
          (mf/deps prop-vals)
@@ -261,8 +269,10 @@
         (mf/use-fn
          (mf/deps component-ids)
          (fn [pos value]
-           (doseq [id component-ids]
-             (st/emit! (dwv/update-property-value id pos value)))))
+           (let [value (if (= value empty-indicator) "" value)]
+             (doseq [id component-ids]
+               (st/emit! (dwv/update-property-value id pos value))
+               (st/emit! (dwv/update-error id nil))))))
 
         update-property-name
         (mf/use-fn
@@ -275,20 +285,30 @@
              (st/emit! (dwv/update-property-name variant-id pos value)))))]
 
     [:*
-     (for [[pos prop] (map vector (range) properties)]
-       [:div {:key (str variant-id "-" pos) :class (stl/css :variant-property-container)}
-        [:*
-         [:div {:class (stl/css :variant-property-name-wrapper)}
-          [:> input-with-meta* {:value (:name prop)
-                                :data-position pos
-                                :on-blur update-property-name}]]
+     [:div {:class (stl/css :variant-property-list)}
+      (for [[pos prop] (map vector (range) properties)]
+        [:div {:key (str variant-id "-" pos) :class (stl/css :variant-property-container)}
+         [:*
+          [:div {:class (stl/css :variant-property-name-wrapper)}
+           [:> input-with-meta* {:value (:name prop)
+                                 :data-position pos
+                                 :on-blur update-property-name}]]
 
-         (let [mixed-value? (= (:value prop) false)]
-           [:> combobox* {:id (str "variant-prop-" variant-id "-" pos)
-                          :placeholder (if mixed-value? (tr "settings.multiple") "--")
-                          :default-selected (if mixed-value? "" (:value prop))
-                          :options (clj->js (get-options (:name prop)))
-                          :on-change (partial update-property-value pos)}])]])]))
+          (let [mixed-value? (= (:value prop) false)]
+            [:> combobox* {:id (str "variant-prop-" variant-id "-" pos)
+                           :placeholder (if mixed-value? (tr "settings.multiple") empty-indicator)
+                           :default-selected (if mixed-value? "" (:value prop))
+                           :options (clj->js (get-options (:name prop)))
+                           :on-change (partial update-property-value pos)}])]])]
+
+     (when variant-error?
+       [:div {:class (stl/css :variant-error-wrapper)}
+        [:> icon* {:icon-id "msg-neutral"
+                   :class (stl/css :variant-error-darken)}]
+        [:div {:class (stl/css :variant-error-highlight)}
+         (tr "workspace.options.component.variant.malformed.single")]
+        [:div {:class (stl/css :variant-error-darken)}
+         (tr "workspace.options.component.variant.malformed.structure")]])]))
 
 (mf/defc component-variant*
   [{:keys [component shape data]}]
@@ -325,7 +345,7 @@
                (when nearest-comp
                  (st/emit! (dwl/component-swap shape (:component-file shape) (:id nearest-comp) true)))))))]
 
-    [:*
+    [:div {:class (stl/css :variant-property-list)}
      (for [[pos prop] (map vector (range) properties)]
        [:div {:key (str (:id shape) pos) :class (stl/css :variant-property-container)}
         [:*
@@ -722,7 +742,9 @@
                          :class        (stl/css :title-spacing-component)}
            [:span {:class (stl/css :copy-text)}
             (if main-instance?
-              (tr "workspace.options.component.main")
+              (if is-variant?
+                (tr "workspace.options.component.variant")
+                (tr "workspace.options.component.main"))
               (tr "workspace.options.component.copy"))]])]
 
        (when open?
@@ -785,6 +807,7 @@
 
           (when (and is-variant? main-instance? same-variant? (not swap-opened?))
             [:> component-variant-main-instance* {:components components
+                                                  :shape shape
                                                   :data data}])
 
           (when (dbg/enabled? :display-touched)
@@ -806,8 +829,14 @@
         objects            (-> (dsh/get-page data current-page-id)
                                (get :objects))
 
-        first-variant      (get objects (first (:shapes shape)))
-        variant-id         (:variant-id first-variant)
+        variants           (mapv #(get objects %) (:shapes shape))
+
+        object-error-ids   (->> variants
+                                (filterv #(some? (:variant-error %)))
+                                (mapv :id))
+        variant-error?     (d/not-empty? object-error-ids)
+
+        variant-id         (:variant-id (first variants))
 
         properties         (mf/with-memo [data objects variant-id]
                              (cfv/extract-properties-values data objects (:id shape)))
@@ -852,7 +881,13 @@
                          (dom/get-data "position")
                          int)]
              (when (> (count properties) 1)
-               (st/emit! (dwv/remove-property variant-id pos))))))]
+               (st/emit! (dwv/remove-property variant-id pos))))))
+
+        select-shape-with-error
+        (mf/use-fn
+         (mf/deps object-error-ids)
+         #(st/emit! (dw/select-shape (first object-error-ids))))]
+
     (when (seq shapes)
       [:div {:class (stl/css :element-set)}
        [:div {:class (stl/css :element-title)}
@@ -893,11 +928,13 @@
                                     :on-close on-menu-close
                                     :menu-entries menu-entries
                                     :main-instance true}]])]
+
         (when-not multi?
-          [:*
+          [:div {:class (stl/css :variant-property-list)}
            (for [[pos property] (map vector (range) properties)]
              (let [meta (str/join ", " (:value property))]
-               [:div {:key (str (:id shape) pos) :class (stl/css :variant-property-row)}
+               [:div {:key (str (:id shape) pos)
+                      :class (stl/css :variant-property-row)}
                 [:> input-with-meta* {:value (:name property)
                                       :meta meta
                                       :data-position pos
@@ -907,4 +944,14 @@
                                   :on-click remove-property
                                   :data-position pos
                                   :icon "remove"
-                                  :disabled (<= (count properties) 1)}]]))])]])))
+                                  :disabled (<= (count properties) 1)}]]))])
+
+        (when variant-error?
+          [:div {:class (stl/css :variant-error-wrapper)}
+           [:> icon* {:icon-id "msg-neutral"
+                      :class (stl/css :variant-error-darken)}]
+           [:div {:class (stl/css :variant-error-highlight)}
+            (tr "workspace.options.component.variant.malformed.multi")]
+           [:button {:class (stl/css :variant-error-button)
+                     :on-click select-shape-with-error}
+            (tr "workspace.options.component.variant.malformed.locate")]])]])))

frontend/src/app/main/ui/workspace/sidebar/options/menus/component.scss

@@ -14,6 +14,7 @@
 
 .element-content {
   @include flexColumn;
+  gap: var(--sp-m);
 }
 
 .title-back {
@@ -706,7 +707,12 @@
   @include flexRow;
   justify-content: space-between;
   width: 100%;
-  margin-block-start: $s-12;
+}
+
+.variant-property-list {
+  display: flex;
+  flex-direction: column;
+  gap: var(--sp-xs);
 }
 
 .variant-property-container {
@@ -736,3 +742,32 @@
   flex: 0 0 auto;
   width: $s-104;
 }
+
+.variant-error-wrapper {
+  @include bodySmallTypography;
+  border: 1px solid var(--color-background-quaternary);
+  border-radius: $s-8;
+  padding: $s-12;
+  display: flex;
+  flex-direction: column;
+  gap: $s-8;
+}
+
+.variant-error-highlight {
+  color: var(--color-foreground-primary);
+}
+
+.variant-error-darken {
+  color: var(--color-foreground-secondary);
+}
+
+.variant-error-button {
+  @include bodySmallTypography;
+  background-color: transparent;
+  border: none;
+  appearance: none;
+  color: var(--color-accent-primary);
+  cursor: pointer;
+  padding: 0;
+  text-align: start;
+}

frontend/translations/en.po

@@ -5224,6 +5224,26 @@ msgstr "Swap component"
 msgid "workspace.options.component.swap.empty"
 msgstr "There are no assets in this library yet"
 
+#: src/app/main/ui/workspace/sidebar/options/menus/component.cljs:742
+msgid "workspace.options.component.variant"
+msgstr "Variant"
+
+#: src/app/main/ui/workspace/sidebar/options/menus/component.cljs:942
+msgid "workspace.options.component.variant.malformed.locate"
+msgstr "Locate invalid variants"
+
+#: src/app/main/ui/workspace/sidebar/options/menus/component.cljs:940
+msgid "workspace.options.component.variant.malformed.multi"
+msgstr "Some variants have invalid names"
+
+#: src/app/main/ui/workspace/sidebar/options/menus/component.cljs:307
+msgid "workspace.options.component.variant.malformed.single"
+msgstr "This variant has an invalid name. Try using the following structure:"
+
+#: src/app/main/ui/workspace/sidebar/options/menus/component.cljs:308
+msgid "workspace.options.component.variant.malformed.structure"
+msgstr "[property]=[value], [property]=[value]"
+
 #: src/app/main/ui/workspace/sidebar/options/menus/constraints.cljs:163
 msgid "workspace.options.constraints"
 msgstr "Constraints"

frontend/translations/es.po

@@ -5250,6 +5250,26 @@ msgstr "Intercambiar componente"
 msgid "workspace.options.component.swap.empty"
 msgstr "Aún no hay recursos en esta biblioteca"
 
+#: src/app/main/ui/workspace/sidebar/options/menus/component.cljs:742
+msgid "workspace.options.component.variant"
+msgstr "Variante"
+
+#: src/app/main/ui/workspace/sidebar/options/menus/component.cljs:942
+msgid "workspace.options.component.variant.malformed.locate"
+msgstr "Localizar variantes no válidas"
+
+#: src/app/main/ui/workspace/sidebar/options/menus/component.cljs:940
+msgid "workspace.options.component.variant.malformed.multi"
+msgstr "Algunas variantes tienen nombres no válidos"
+
+#: src/app/main/ui/workspace/sidebar/options/menus/component.cljs:307
+msgid "workspace.options.component.variant.malformed.single"
+msgstr "Esta variante tiene un nombre no válido. Prueba a utilizar la siguiente estructura:"
+
+#: src/app/main/ui/workspace/sidebar/options/menus/component.cljs:308
+msgid "workspace.options.component.variant.malformed.structure"
+msgstr "[propiedad]=[valor], [propiedad]=[valor]"
+
 #: src/app/main/ui/workspace/sidebar/options/menus/constraints.cljs:163
 msgid "workspace.options.constraints"
 msgstr "Restricciones"