🐛 Don't allow a variant switch when that will provoke a components loop

common/src/app/common/files/helpers.cljc

@@ -426,38 +426,15 @@
 
 (defn components-nesting-loop?
   "Check if a nesting loop would be created if the given shape is moved below the given parent"
-  [objects shape-id parent-id]
+  ([objects shape-id parent-id]
-  (let [xf-get-component-id (keep :component-id)
+   (let [children (get-children-with-self objects shape-id)
-
+         parents  (get-parents-with-self objects parent-id)]
-        children            (get-children-with-self objects shape-id)
+     (components-nesting-loop? children parents)))
-        child-components    (into #{} xf-get-component-id children)
+  ([children parents]
-
+   (let [xf-get-component-id (keep :component-id)
-        parents             (get-parents-with-self objects parent-id)
+         child-components    (into #{} xf-get-component-id children)
-        parent-components   (into #{} xf-get-component-id parents)]
+         parent-components   (into #{} xf-get-component-id parents)]
-    (seq (set/intersection child-components parent-components))))
+     (seq (set/intersection child-components parent-components)))))
-
-(defn variants-nesting-loop?
-  "Check if a variants nesting loop would be created if the given shape is moved below the given parent"
-  [objects libraries shape parent pasting-cutted-mains?]
-  ;; If we are cut-pasting mains into its own variant, it is ok
-  (if (and pasting-cutted-mains?
-           (:is-variant-container parent)
-           (= (:variant-id shape) (:id parent)))
-    nil
-    (let [get-variant-id #(or (:variant-id %)
-                              (when (:is-variant-container %) (:id %))
-                              (when (:component-id %)
-                                (dm/get-in libraries [(:component-file %)
-                                                      :data
-                                                      :components
-                                                      (:component-id %)
-                                                      :variant-id])))
-          child-variant-ids  (into #{} (keep get-variant-id)
-                                   (get-children-with-self objects (:id shape)))
-          parent-variant-ids (into #{} (keep get-variant-id)
-                                   (get-parents-with-self objects (:id parent)))]
-      (seq (set/intersection child-variant-ids parent-variant-ids)))))
-
 
 ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
 ;; ALGORITHMS & TRANSFORMATIONS FOR SHAPES

common/src/app/common/types/container.cljc

@@ -434,9 +434,9 @@
       (mapcat collect-main-shapes children objects)
       [])))
 
-(defn- invalid-structure-for-component?
+(defn invalid-structure-for-component?
   "Check if the structure generated nesting children in parent is invalid in terms of nested components"
-  [objects parent children pasting? all-comp-cut? libraries]
+  [objects parent children pasting? libraries]
   (let [; If the original shapes had been cutted, and we are pasting them now, they aren't
         ; in objects. We can add them to locate later
         objects (merge objects
@@ -457,11 +457,7 @@
         parent-in-component?    (in-any-component? objects parent)
         comps-nesting-loop?     (not (->> children
                                           (map #(cfh/components-nesting-loop? objects (:id %) (:id parent)))
-                                          (every? nil?)))
+                                          (every? nil?)))]
-
-        variants-nesting-loop? (not (->> children
-                                         (map #(cfh/variants-nesting-loop? objects libraries % parent (and pasting? all-comp-cut?)))
-                                         (every? nil?)))]
     (or
       ;;We don't want to change the structure of component copies
      (ctk/in-component-copy? parent)
@@ -470,8 +466,7 @@
      (and selected-main-instance? parent-in-component?)
       ;; Avoid placing a shape as a direct or indirect child of itself,
       ;; or inside its main component if it's in a copy.
-     comps-nesting-loop?
+     comps-nesting-loop?)))
-     variants-nesting-loop?)))
 
 (defn find-valid-parent-and-frame-ids
   "Navigate trough the ancestors until find one that is valid. Returns [ parent-id frame-id ]"
@@ -511,7 +506,7 @@
                                             true))
                   (every? :deleted)))]
        (if (or no-changes?
-               (and (not (invalid-structure-for-component? objects parent children pasting? all-comp-cut? libraries))
+               (and (not (invalid-structure-for-component? objects parent children pasting? libraries))
                     ;; If we are moving into a main component, no descendant can be main
                     (or (nil? any-main-descendant) (not (ctk/main-instance? parent)))
                     ;; If we are moving into a variant-container, all the items should be main

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]
@@ -15,8 +16,10 @@
    [app.common.types.components-list :as ctkl]
    [app.common.types.file :as ctf]
    [app.common.types.variant :as ctv]
+   [app.common.uuid :as uuid]
    [app.main.data.helpers :as dsh]
    [app.main.data.modal :as modal]
+   [app.main.data.notifications :as ntf]
    [app.main.data.workspace :as dw]
    [app.main.data.workspace.libraries :as dwl]
    [app.main.data.workspace.specialized-panel :as dwsp]
@@ -397,7 +400,8 @@
 
 (mf/defc component-variant-copy*
   [{:keys [component shape data current-file-id]}]
-  (let [component-id (:id component)
+  (let [page-objects (mf/deref refs/workspace-page-objects)
+        component-id (:id component)
         properties   (:variant-properties component)
         variant-id   (:variant-id component)
         objects      (-> (dsh/get-page data (:main-instance-page component))
@@ -444,6 +448,10 @@
                (st/emit! (dwl/go-to-local-component :id (first ids) :additional-ids (rest ids)))
                (st/emit! (dwl/go-to-component-file (:component-file shape) (first malformed-comps) false))))))
 
+        ;; Used to force a remount after an error
+        key*     (mf/use-state (uuid/next))
+        key      (deref key*)
+
         switch-component
         (mf/use-fn
          (mf/deps shape component component-id variant-comps)
@@ -455,9 +463,17 @@
                                      (remove #(= (:id %) component-id))
                                      (filter #(= (dm/get-in % [:variant-properties pos :value]) val))
                                      (reverse))
-                   nearest-comp (apply min-key #(ctv/distance target-props (:variant-properties %)) valid-comps)]
+                   nearest-comp (apply min-key #(ctv/distance target-props (:variant-properties %)) valid-comps)
+                   parents (cfh/get-parents-with-self page-objects (:parent-id shape))
+                   children (cfh/get-children-with-self objects (:main-instance-id nearest-comp))
+                   comps-nesting-loop? (seq? (cfh/components-nesting-loop? children parents))]
+
                (when nearest-comp
-                 (st/emit! (dwl/component-swap shape (:component-file shape) (:id nearest-comp) true)))))))]
+                 (if comps-nesting-loop?
+                   (do
+                     (st/emit! (ntf/error (tr "workspace.component.swap.loop-error")))
+                     (reset! key* (uuid/next)))
+                   (st/emit! (dwl/component-swap shape (:component-file shape) (:id nearest-comp) true))))))))]
 
     [:*
      [:div {:class (stl/css :variant-property-list)}
@@ -474,7 +490,8 @@
           [:> select* {:default-selected (:value prop)
                        :options (get-options (:name prop))
                        :empty-to-end true
-                       :on-change (partial switch-component pos)}]]])]
+                       :on-change (partial switch-component pos)
+                       :key (str (:value prop) "-" key)}]]])]
 
      (if (seq malformed-comps)
        [:div {:class (stl/css :variant-warning-wrapper)}

frontend/translations/en.po

@@ -5607,6 +5607,9 @@ msgstr "Swap component"
 msgid "workspace.options.component.swap.empty"
 msgstr "There are no assets in this library yet"
 
+msgid "workspace.component.swap.loop-error"
+msgstr "A component can't be a child of itself"
+
 #: src/app/main/ui/workspace/sidebar/options/menus/component.cljs:973
 msgid "workspace.options.component.unlinked"
 msgstr "Unlinked"

frontend/translations/es.po

@@ -5614,6 +5614,9 @@ msgstr "Intercambiar componente"
 msgid "workspace.options.component.swap.empty"
 msgstr "Aún no hay recursos en esta biblioteca"
 
+msgid "workspace.component.swap.loop-error"
+msgstr "Un componente no puede ser hijo de si mismo"
+
 #: src/app/main/ui/workspace/sidebar/options/menus/component.cljs:973
 msgid "workspace.options.component.unlinked"
 msgstr "Desvinculado"