🐛 Fix import/export components with boards inside

common/src/app/common/file_builder.cljc

@@ -44,7 +44,12 @@
    (let [component-id (:current-component-id file)
          change (cond-> change
                   (and add-container? (some? component-id))
-                  (assoc :component-id component-id)
+                  (cond->
+                   :always
+                    (assoc :component-id component-id)
+
+                    (some? (:current-frame-id file))
+                    (assoc :frame-id (:current-frame-id file)))
 
                   (and add-container? (nil? component-id))
                   (assoc :page-id  (:current-page-id file)
@@ -223,7 +228,6 @@
       (clear-names)))
 
 (defn add-artboard [file data]
-  (assert (nil? (:current-component-id file)))
   (let [obj (-> (cts/make-minimal-shape :frame)
                 (merge data)
                 (check-name file :frame)
@@ -237,11 +241,11 @@
         (update :parent-stack conjv (:id obj)))))
 
 (defn close-artboard [file]
-  (assert (nil? (:current-component-id file)))
-
   (let [parent-id (-> file :parent-id peek)
         parent (lookup-shape file parent-id)
-        current-frame-id (or (:frame-id parent) root-frame)]
+        current-frame-id (or (:frame-id parent)
+                             (when (nil? (:current-component-id file))
+                               root-frame))]
     (-> file
         (assoc :current-frame-id current-frame-id)
         (update :parent-stack pop))))
@@ -561,35 +565,38 @@
           :id id}))))
 
 (defn start-component
-  [file data]
+  ([file data] (start-component file data :group))
+  ([file data root-type]
+   (let [selrect (if (and (:x data) (:y data) (:width data) (:height data))
+                   (gsh/make-selrect (:x data) (:y data) (:width data) (:height data))
+                   cts/empty-selrect)
+         name               (:name data)
+         path               (:path data)
+         main-instance-id   (:main-instance-id data)
+         main-instance-page (:main-instance-page data)
+         obj (-> (cts/make-shape root-type selrect data)
+                 (dissoc :path
+                         :main-instance-id
+                         :main-instance-page
+                         :main-instance-x
+                         :main-instance-y)
+                 (check-name file root-type)
+                 (d/without-nils))]
+     (-> file
+         (commit-change
+          {:type :add-component
+           :id (:id obj)
+           :name name
+           :path path
+           :main-instance-id main-instance-id
+           :main-instance-page main-instance-page
+           :shapes [obj]})
 
-  (let [selrect cts/empty-selrect
+         (assoc :last-id (:id obj))
-        name               (:name data)
+         (update :parent-stack conjv (:id obj))
-        path               (:path data)
+         (assoc :current-component-id (:id obj))
-        main-instance-id   (:main-instance-id data)
+         (assoc :current-frame-id (when (= (:type obj) :frame)
-        main-instance-page (:main-instance-page data)
+                                    (:id obj)))))))
-        obj (-> (cts/make-minimal-group nil selrect name)
-                (merge data)
-                (dissoc :path
-                        :main-instance-id
-                        :main-instance-page
-                        :main-instance-x
-                        :main-instance-y)
-                (check-name file :group)
-                (d/without-nils))]
-    (-> file
-        (commit-change
-         {:type :add-component
-          :id (:id obj)
-          :name name
-          :path path
-          :main-instance-id main-instance-id
-          :main-instance-page main-instance-page
-          :shapes [obj]})
-
-        (assoc :last-id (:id obj))
-        (update :parent-stack conjv (:id obj))
-        (assoc :current-component-id (:id obj)))))
 
 (defn finish-component
   [file]
@@ -624,7 +631,7 @@
 
              {:add-container? true}))
 
-          :else
+          (= (:type component) :group)
           (let [component' (gsh/update-group-selrect component children)]
             (commit-change
              file
@@ -637,11 +644,13 @@
                {:type :set :attr :y      :val (-> component' :selrect :y) :ignore-touched true}
                {:type :set :attr :width  :val (-> component' :selrect :width) :ignore-touched true}
                {:type :set :attr :height :val (-> component' :selrect :height) :ignore-touched true}]}
+             {:add-container? true}))
 
-             {:add-container? true})))]
+          :else file)]
 
     (-> file
         (dissoc :current-component-id)
+        (dissoc :current-frame-id)
         (update :parent-stack pop))))
 
 (defn finish-deleted-component
@@ -700,7 +709,7 @@
                                      (gpt/point x
                                                 y)
                                      #_{:main-instance? true
-                                      :force-id main-instance-id})]
+                                        :force-id main-instance-id})]
 
     (as-> file $
       (reduce #(commit-change %1

common/src/app/common/geom/shapes.cljc

@@ -183,7 +183,6 @@
 (dm/export gsi/rect-contains-shape?)
 
 ;; Bool
-
 (dm/export gsb/calc-bool-content)
 
 ;; Constraints
@@ -196,4 +195,3 @@
 
 ;; Modifiers
 (dm/export gsm/set-objects-modifiers)
-

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

@@ -270,7 +270,6 @@
          (update :undo-changes #(reduce mk-undo-change % shapes))
          (apply-changes-local)))))
 
-
 (defn changed-attrs
   "Returns the list of attributes that will change when `update-fn` is applied"
   [object update-fn {:keys [attrs]}]

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

@@ -12,6 +12,7 @@
    [app.common.types.page :as ctp]
    [app.common.types.shape :as cts]
    [app.common.types.typography :as ctt]
+   [app.common.uuid :as uuid]
    [clojure.spec.alpha :as s]))
 
 (s/def ::index integer?)
@@ -61,7 +62,7 @@
            (some? (:frame-id o)))
       (and (contains? o :component-id)
            (not (contains? o :page-id))
-           (nil? (:frame-id o)))))
+           (not= (:frame-id o) uuid/zero))))
 
 (defn- valid-container-id?
   [o]

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

@@ -69,13 +69,16 @@
   (assert (nil? (:component-id shape)))
   (assert (nil? (:component-file shape)))
   (assert (nil? (:shape-ref shape)))
-  (let [;; Ensure that the component root is not an instance and
+  (let [frame-ids-map (volatile! {})
-        ;; it's no longer tied to a frame.
+
-        update-new-shape (fn [new-shape _original-shape]
+        ;; Ensure that the component root is not an instance
+        update-new-shape (fn [new-shape original-shape]
+                           (when (= (:type original-shape) :frame)
+                             (vswap! frame-ids-map assoc (:id original-shape) (:id new-shape)))
+
                            (cond-> new-shape
                              true
-                             (-> (assoc :frame-id nil)
+                             (dissoc :component-root?)
-                                 (dissoc :component-root?))
 
                              (nil? (:parent-id new-shape))
                              (dissoc :component-id
@@ -100,9 +103,17 @@
                                   (assoc :main-instance? true)
 
                                   (some? (:parent-id new-shape))
-                                  (dissoc :component-root?)))]
+                                  (dissoc :component-root?)))
 
-    (ctst/clone-object shape nil objects update-new-shape update-original-shape)))
+        [new-root-shape new-shapes updated-shapes]
+        (ctst/clone-object shape nil objects update-new-shape update-original-shape)
+
+        ;; If frame-id points to a shape inside the component, remap it to the
+        ;; corresponding new frame shape. If not, set it to nil.
+        remap-frame-id (fn [shape]
+                         (update shape :frame-id #(get @frame-ids-map % nil)))]
+
+    [new-root-shape (map remap-frame-id new-shapes) updated-shapes]))
 
 (defn make-component-instance
   "Clone the shapes of the component, generating new names and ids, and linking
@@ -115,13 +126,14 @@
     {:keys [main-instance? force-id] :or {main-instance? false force-id nil}}]
    (let [component-shape (get-shape component (:id component))
 
-         orig-pos  (gpt/point (:x component-shape) (:y component-shape))
+         orig-pos        (gpt/point (:x component-shape) (:y component-shape))
-         delta     (gpt/subtract position orig-pos)
+         delta           (gpt/subtract position orig-pos)
 
-         objects   (:objects container)
+         objects         (:objects container)
-         unames    (volatile! (ctst/retrieve-used-names objects))
+         unames          (volatile! (ctst/retrieve-used-names objects))
 
-         frame-id  (ctst/frame-id-by-position objects (gpt/add orig-pos delta))
+         frame-id        (ctst/frame-id-by-position objects (gpt/add orig-pos delta))
+         frame-ids-map   (volatile! {})
 
          update-new-shape
          (fn [new-shape original-shape]
@@ -130,14 +142,13 @@
              (when (nil? (:parent-id original-shape))
                (vswap! unames conj new-name))
 
+             (when (= (:type original-shape) :frame)
+               (vswap! frame-ids-map assoc (:id original-shape) (:id new-shape)))
+
              (cond-> new-shape
                true
-               (as-> $
+               (-> (gsh/move delta)
-                 (gsh/move $ delta)
+                   (dissoc :touched))
-                 (assoc $ :frame-id frame-id)
-                 (assoc $ :parent-id
-                        (or (:parent-id $) (:frame-id $)))
-                 (dissoc $ :touched))
 
                (nil? (:shape-ref original-shape))
                (assoc :shape-ref (:id original-shape))
@@ -160,7 +171,15 @@
                             (get component :objects)
                             update-new-shape
                             (fn [object _] object)
-                            force-id)]
+                            force-id)
 
-     [new-shape new-shapes])))
+        ;; If frame-id points to a shape inside the component, remap it to the
+        ;; corresponding new frame shape. If not, set it to the destination frame.
+        ;; Also fix empty parent-id.
+        remap-frame-id (fn [shape]
+                         (as-> shape $
+                           (update $ :frame-id #(get @frame-ids-map % frame-id))
+                           (update $ :parent-id #(or % (:frame-id $)))))]
+
+     [new-shape (map remap-frame-id new-shapes)])))
 

frontend/src/app/main/render.cljs

@@ -397,11 +397,13 @@
 
         group-wrapper
         (mf/use-memo
-         (mf/deps objects root-shape)
+         (mf/deps objects)
-         (fn []
+         (fn [] (group-wrapper-factory objects)))
-           (case (:type root-shape)
+
-             :group (group-wrapper-factory objects)
+        frame-wrapper
-             :frame (frame-wrapper-factory objects))))]
+        (mf/use-memo
+         (mf/deps objects)
+         (fn [] (frame-wrapper-factory objects)))]
 
     [:> "symbol" #js {:id (str id)
                       :viewBox vbox
@@ -412,7 +414,9 @@
                       "penpot:main-instance-y" main-instance-y}
      [:title name]
      [:> shape-container {:shape root-shape}
-      [:& group-wrapper {:shape root-shape :view-box vbox}]]]))
+      (case (:type root-shape)
+        :group [:& group-wrapper {:shape root-shape :view-box vbox}]
+        :frame [:& frame-wrapper {:shape root-shape :view-box vbox}])]]))
 
 (mf/defc components-sprite-svg
   {::mf/wrap-props false}

frontend/src/app/util/import/parser.cljs

@@ -64,9 +64,9 @@
    (or (find-node node :penpot:shape)
        (find-node node :penpot:page)))
 
-   ([node tag]
+  ([node tag]
-    (-> (get-data node)
+   (-> (get-data node)
-        (find-node tag))))
+       (find-node tag))))
 
 (defn get-type
   [node]
@@ -217,10 +217,10 @@
       (let [;; Old .penpot files doesn't have "g" nodes. They have a clipPath reference as a node attribute
             to-url #(dm/str "url(#" % ")")
             frame-clip-rect-node  (->> (find-all-nodes node :defs)
-                                   (mapcat #(find-all-nodes % :clipPath))
+                                       (mapcat #(find-all-nodes % :clipPath))
-                                   (filter #(= (to-url (:id (:attrs %))) (:clip-path node-attrs)))
+                                       (filter #(= (to-url (:id (:attrs %))) (:clip-path node-attrs)))
-                                   (mapcat #(find-all-nodes % #{:rect :path}))
+                                       (mapcat #(find-all-nodes % #{:rect :path}))
-                                   (first))
+                                       (first))
 
             ;; The nodes with the "frame-background" class can have some anidation depending on the strokes they have
             g-nodes    (find-all-nodes node :g)
@@ -1007,8 +1007,8 @@
                      (ctsi/has-overlay-opts interaction)
                      (assoc :overlay-pos-type    (get-meta node :overlay-pos-type keyword)
                             :overlay-position    (gpt/point
-                                                   (get-meta node :overlay-position-x d/parse-double)
+                                                  (get-meta node :overlay-position-x d/parse-double)
-                                                   (get-meta node :overlay-position-y d/parse-double))
+                                                  (get-meta node :overlay-position-y d/parse-double))
                             :close-click-outside (get-meta node :close-click-outside str->bool)
                             :background-overlay  (get-meta node :background-overlay str->bool)))))))))
 

frontend/src/app/worker/import.cljs

@@ -389,21 +389,22 @@
         old-id             (cip/get-id node)
         id                 (resolve old-id)
         path               (get-in node [:attrs :penpot:path] "")
+        type               (cip/get-type content)
         main-instance-id   (resolve (uuid (get-in node [:attrs :penpot:main-instance-id] "")))
         main-instance-page (resolve (uuid (get-in node [:attrs :penpot:main-instance-page] "")))
-        data               (-> (cip/parse-data :group content)
+        data               (-> (cip/parse-data type content)
                                (assoc :path path)
                                (assoc :id id)
                                (assoc :main-instance-id main-instance-id)
                                (assoc :main-instance-page main-instance-page))
 
-        file               (-> file (fb/start-component data))
+        file               (-> file (fb/start-component data type))
-        children            (cip/node-seq node)]
+        children           (cip/node-seq node)]
 
     (->> (rx/from children)
          (rx/filter cip/shape?)
-         (rx/skip 1)
+         (rx/skip 1)       ;; Skip the outer component and the respective closint tag
-         (rx/skip-last 1)
+         (rx/skip-last 1)  ;; because they are handled in start-component an finish-component
          (rx/mapcat (partial resolve-media context file-id))
          (rx/reduce (partial process-import-node context) file)
          (rx/map fb/finish-component))))
@@ -419,8 +420,9 @@
         main-instance-page (resolve (uuid (get-in node [:attrs :penpot:main-instance-page] "")))
         main-instance-x    (get-in node [:attrs :penpot:main-instance-x] "")
         main-instance-y    (get-in node [:attrs :penpot:main-instance-y] "")
+        type               (cip/get-type content)
 
-        data (-> (cip/parse-data :group content)
+        data (-> (cip/parse-data type content)
                  (assoc :path path)
                  (assoc :id id)
                  (assoc :main-instance-id main-instance-id)