Merge pull request #3743 from penpot/hiru-components-bugfix

Hiru components bugfix

common/src/app/common/files/validate.cljc

@@ -125,17 +125,29 @@
                           :child-id child-id)))))))
 
 (defn validate-frame
-  "Validate that the frame-id shape exists and is indeed a frame."
+  "Validate that the frame-id shape exists and is indeed a frame. Also it must point to the
+   parent shape (if this is a frame) or to the frame-id of the parent (if not)."
   [shape file page]
   (let [frame (ctst/get-shape page (:frame-id shape))]
     (if (nil? frame)
       (report-error :frame-not-found
                     (str/format "Frame %s not found" (:frame-id shape))
                     shape file page)
-      (when (not= (:type frame) :frame)
+      (if (not= (:type frame) :frame)
         (report-error :invalid-frame
                       (str/format "Frame %s is not actually a frame" (:frame-id shape))
-                      shape file page)))))
+                      shape file page)
+        (let [parent (ctst/get-shape page (:parent-id shape))]
+          (when (some? parent)
+            (if (= (:type parent) :frame)
+              (when-not (= (:frame-id shape) (:id parent))
+                (report-error :invalid-frame
+                              (str/format "Frame-id should point to parent" (:id parent))
+                              shape file page))
+              (when-not (= (:frame-id shape) (:frame-id parent))
+                (report-error :invalid-frame
+                              (str/format "Frame-id should point to parent frame" (:frame-id parent))
+                              shape file page)))))))))
 
 (defn validate-component-main-head
   "Validate shape is a main instance head, component exists and its main-instance points to this shape."

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

@@ -192,6 +192,23 @@
       (ctk/instance-head? shape)
       (inside-component-main? objects shape)))
 
+(defn convert-shape-in-component
+  "Set the shape as a main root instance, pointing to a new component.
+   Also remove component-root of all children. Return the same structure
+   as make-component-shape."
+  [root objects file-id]
+  (let [new-id       (uuid/next)
+        new-root     (assoc root
+                            :component-id new-id
+                            :component-file file-id
+                            :component-root true
+                            :main-instance true)
+        new-children (->> (cph/get-children objects (:id root))
+                          (map #(dissoc % :component-root)))]
+    [(assoc new-root :id new-id)
+     nil
+     (into [new-root] new-children)]))
+
 (defn make-component-shape
   "Clone the shape and all children. Generate new ids and detach
   from parent and frame. Update the original shapes to have links
@@ -344,21 +361,6 @@
      [(remap-ids new-shape)
       (map remap-ids new-shapes)])))
 
-(defn get-top-instance
-  "The case of having an instance that contains another instances.
-  The topmost one, that is not part of other instance, is the Top instance"
-  [objects shape current-top]
-  (let [current-top (if (and
-                         (not (ctk/main-instance? shape))
-                         (ctk/instance-head? shape))
-                      shape current-top)
-        parent-id   (:parent-id shape)
-        parent      (get objects parent-id)]
-    (if (= parent-id uuid/zero)
-      current-top
-      (get-top-instance objects parent current-top))))
-
-
 (defn get-first-not-copy-parent
   "Go trough the parents until we find a shape that is not a copy of a component."
   [objects id]

common/src/app/common/types/file.cljc

@@ -205,7 +205,7 @@
 (defn find-remote-shape
   "Recursively go back by the :shape-ref of the shape until find the correct shape of the original component"
   [container libraries shape]
-  (let [top-instance        (ctn/get-top-instance (:objects container) shape nil)
+  (let [top-instance        (ctn/get-component-shape (:objects container) shape)
         component-file      (get-in libraries [(:component-file top-instance) :data])
         component           (ctkl/get-component component-file (:component-id top-instance) true)
         remote-shape        (get-ref-shape component-file component shape)

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

@@ -1814,15 +1814,6 @@
                 (assoc change :index (get map-ids (:old-id change)))
                 change)))
 
-          ;; Check if the shape is an instance whose master is defined in a
-          ;; library that is not linked to the current file
-          (foreign-instance? [shape paste-objects state]
-            (let [root         (ctn/get-component-shape paste-objects shape {:allow-main? true})
-                  root-file-id (:component-file root)]
-              (and (some? root)
-                   (not= root-file-id (:current-file-id state))
-                   (nil? (get-in state [:workspace-libraries root-file-id])))))
-
           ;; Proceed with the standard shape paste process.
           (do-paste [it state mouse-pos media]
             (let [libraries    (wsh/get-libraries state)
@@ -1841,25 +1832,13 @@
 
                   process-shape
                   (fn [_ shape]
-                    (let [parent                 (get page-objects parent-id)
+                    (let [assign-shapes? (and (or (cph/group-shape? shape)
-                          component-shape        (ctn/get-component-shape page-objects shape)
-                          component-shape-parent (ctn/get-component-shape page-objects parent)
-                          ;; if foreign instance, or a shape belonging to another component, detach the shape
-                          detach? (or (foreign-instance? shape paste-objects state)
-                                      (and (ctk/in-component-copy-not-head? shape)
-                                           (not= (:id component-shape)
-                                                 (:id component-shape-parent))))
-                          assign-shapes? (and (or (cph/group-shape? shape)
                                                   (cph/bool-shape? shape))
                                               (nil? (:shapes shape)))]
                       (-> shape
                           (assoc :frame-id frame-id :parent-id parent-id)
                           (cond-> assign-shapes?
                             (assoc :shapes []))
-                          (cond-> detach?
-                            ;; this is used later, if the paste needs to create a new component from the detached shape
-                            (-> (assoc :saved-component-root (:component-root shape))
-                                (ctk/detach-shape)))
                           ;; if is a text, remove references to external typographies
                           (cond-> (= (:type shape) :text)
                             (ctt/remove-external-typographies file-id)))))

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

@@ -70,14 +70,7 @@
         [root-shape new-shapes updated-shapes]
         (if-not components-v2
           (ctn/make-component-shape root objects file-id components-v2)
-          (let [new-id (uuid/next)]
+          (ctn/convert-shape-in-component root objects file-id))
-            [(assoc root :id new-id)
-             nil
-             [(assoc root
-                     :component-id new-id
-                     :component-file file-id
-                     :component-root true
-                     :main-instance true)]]))
 
         changes (-> changes
                     (pcb/add-component (:id root-shape)

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

@@ -463,7 +463,7 @@
 
            new-obj (cond-> new-obj
                      (not duplicating-component?)
-                     (dissoc :shape-ref))
+                     (ctk/detach-shape))
 
            ; We want the first added object to touch it's parent, but not subsequent children
            changes (-> (pcb/add-object changes new-obj {:ignore-touched (and duplicating-component? child?)})