Merge pull request #2169 from penpot/hiru-create-nested-component

🎉 Allow to create a nested component in one step

common/src/app/common/types/component.cljc

@@ -6,6 +6,10 @@
 
 (ns app.common.types.component)
 
+(defn instance-root?
+  [shape]
+  (some? (:component-id shape)))
+ 
 (defn instance-of?
   [shape file-id component-id]
   (and (some? (:component-id shape))

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

@@ -59,35 +59,41 @@
 ;; ---- Components and instances creation ----
 
 (defn generate-add-component
-  "If there is exactly one id, and it's a group, use it as root. Otherwise,
+  "If there is exactly one id, and it's a group, and not already a component, use
-  create a group that contains all ids. Then, make a component with it,
+  it as root. Otherwise, create a group that contains all ids. Then, make a
-  and link all shapes to their corresponding one in the component."
+  component with it, and link all shapes to their corresponding one in the component."
   [it shapes objects page-id file-id components-v2]
-  (if (and (= (count shapes) 1)
+  (let [[group changes]
-           (:component-id (first shapes)))
+        (if (and (= (count shapes) 1)
-    [(first shapes) (pcb/empty-changes it)]
+                 (= (:type (first shapes)) :group)
-    (let [name        (if (= 1 (count shapes)) (:name (first shapes)) "Component-1")
+                 (not (ctk/instance-root? (first shapes))))
-          [path name] (cph/parse-path-name name)
+          [(first shapes) (-> (pcb/empty-changes it page-id)
+                              (pcb/with-objects objects))]
+          (let [group-name (if (= 1 (count shapes))
+                             (:name (first shapes))
+                             "Component-1")]
+            (dwg/prepare-create-group it
+                                      objects
+                                      page-id
+                                      shapes
+                                      group-name
+                                      (not (ctk/instance-root? (first shapes))))))
 
-          [group changes]
+        name (:name group)
-          (if (and (= (count shapes) 1)
+        [path name] (cph/parse-path-name name)
-                   (= (:type (first shapes)) :group))
-            [(first shapes) (-> (pcb/empty-changes it page-id)
-                                (pcb/with-objects objects))]
-            (dwg/prepare-create-group it objects page-id shapes name true))
 
-          [new-shape new-shapes updated-shapes]
+        [new-shape new-shapes updated-shapes]
-          (ctn/make-component-shape group objects file-id components-v2)
+        (ctn/make-component-shape group objects file-id components-v2)
 
-          changes (-> changes
+        changes (-> changes
-                      (pcb/add-component (:id new-shape)
+                    (pcb/add-component (:id new-shape)
-                                         path
+                                       path
-                                         name
+                                       name
-                                         new-shapes
+                                       new-shapes
-                                         updated-shapes
+                                       updated-shapes
-                                         (:id group)
+                                       (:id group)
-                                         page-id))]
+                                       page-id))]
-      [group new-shape changes])))
+    [group new-shape changes]))
 
 (defn duplicate-component
   "Clone the root shape of the component and all children. Generate new

frontend/src/app/main/ui/workspace/context_menu.cljs

@@ -407,20 +407,19 @@
                                         :accept-style :primary
                                         :on-accept do-update-component-in-bulk}))]
     [:*
-     (when (and (not has-frame?) (not is-component?))
+     (when (not has-frame?)
        [:*
         [:& menu-separator]
         [:& menu-entry {:title (tr "workspace.shape.menu.create-component")
                         :shortcut (sc/get-tooltip :create-component)
                         :on-click do-add-component}]
-        (when has-component?
+        (when (and has-component? (not single?))
           [:*
            [:& menu-entry {:title (tr "workspace.shape.menu.detach-instances-in-bulk")
                            :shortcut (sc/get-tooltip :detach-component)
                            :on-click do-detach-component-in-bulk}]
-           (when (not single?)
+           [:& menu-entry {:title (tr "workspace.shape.menu.update-components-in-bulk")
-             [:& menu-entry {:title (tr "workspace.shape.menu.update-components-in-bulk")
+                           :on-click do-update-in-bulk}]])])
-                             :on-click do-update-in-bulk}])])])
 
      (when is-component?
        ;; WARNING: this menu is the same as the context menu at the sidebar.

frontend/src/debug.cljs

@@ -207,7 +207,7 @@
   (dump-selected' @st/state))
 
 (defn dump-tree'
-  ([state ] (dump-tree' state false false))
+  ([state] (dump-tree' state false false))
   ([state show-ids] (dump-tree' state show-ids false))
   ([state show-ids show-touched]
    (let [page-id    (get state :current-page-id)

frontend/test/app/components_basic_test.cljs

@@ -4,6 +4,7 @@
     [app.common.geom.point :as gpt]
     [app.common.pages.helpers :as cph]
     [app.common.types.container :as ctn]
+    [app.common.types.file :as ctf]
     [app.main.data.workspace :as dw]
     [app.main.data.workspace.groups :as dwg]
     [app.main.data.workspace.libraries :as dwl]
@@ -33,6 +34,11 @@
 
             store (the/prepare-store state done
                     (fn [new-state]
+                      ;; Uncomment to debug
+                      ;; (ctf/dump-tree (get new-state :workspace-data)
+                      ;;                (get new-state :current-page-id)
+                      ;;                (get new-state :workspace-libraries))
+
                       ; Expected shape tree:
                       ;
                       ; [Page]
@@ -40,7 +46,7 @@
                       ;   Rect-2              #--> Rect-2
                       ;     Rect-1            ---> Rect-1
                       ;
-                      ; [Rect-1]
+                      ; [Rect-2]
                       ; Rect-2
                       ;   Rect-1
                       ;
@@ -55,7 +61,7 @@
 
                         (t/is (= (:name shape1) "Rect-1"))
                         (t/is (= (:name group) "Rect-2"))
-                        (t/is (= (:name component) "Rect-1"))
+                        (t/is (= (:name component) "Rect-2"))
                         (t/is (= (:name c-shape1) "Rect-1"))
                         (t/is (= (:name c-group) "Rect-2"))
 
@@ -207,6 +213,70 @@
        (dwl/add-component)
        :the/end))))
 
+(t/deftest test-add-component-from-component
+  (t/async
+   done
+   (let [state (-> thp/initial-state
+                   (thp/sample-page)
+                   (thp/sample-shape :shape1 :rect
+                                     {:name "Rect-1"})
+                   (thp/make-component :instance1 :component1
+                                       [(thp/id :shape1)]))
+
+         store (the/prepare-store state done
+                 (fn [new-state]
+                   ; Expected shape tree:
+                   ;
+                   ; [Page]
+                   ; Root Frame
+                   ;   Rect-3              #--> Rect-3
+                   ;     Rect-2            @--> Rect-2
+                   ;       Rect-1          ---> Rect-1
+                   ;
+                   ; [Rect-2]
+                   ; Rect-2
+                   ;   Rect-1
+                   ;
+                   ; [Rect-2]
+                   ; Rect-3
+                   ;   Rect-2              @--> Rect-2
+                   ;     Rect-1            ---> Rect-1
+                   ;
+                   (let [[[instance1 shape1]
+                          [c-instance1 c-shape1]
+                          component1]
+                         (thl/resolve-instance-and-main
+                           new-state
+                           (thp/id :instance1)
+                           true)
+
+                         [[instance2 instance1' shape1']
+                          [c-instance2 c-instance1' c-shape1']
+                          component2]
+                         (thl/resolve-instance-and-main
+                           new-state
+                           (:parent-id instance1))]
+
+                     (t/is (= (:name shape1) "Rect-1"))
+                     (t/is (= (:name instance1) "Rect-2"))
+                     (t/is (= (:name component1) "Rect-2"))
+                     (t/is (= (:name c-shape1) "Rect-1"))
+                     (t/is (= (:name c-instance1) "Rect-2"))
+
+                     (t/is (= (:name shape1') "Rect-1"))
+                     (t/is (= (:name instance1') "Rect-2"))
+                     (t/is (= (:name instance2) "Rect-3"))
+                     (t/is (= (:name component2) "Rect-3"))
+                     (t/is (= (:name c-shape1') "Rect-1"))
+                     (t/is (= (:name c-instance1') "Rect-2"))
+                     (t/is (= (:name c-instance2) "Rect-3")))))]
+
+     (ptk/emit!
+       store
+       (dw/select-shape (thp/id :instance1))
+       (dwl/add-component)
+       :the/end))))
+
 (t/deftest test-rename-component
   (t/async
    done
@@ -269,7 +339,7 @@
                    ; Rect-2
                    ;   Rect-1
                    ;
-                   ; [Rect-2]
+                   ; [Rect-3]
                    ; Rect-2
                    ;   Rect-1
                    ;
@@ -293,7 +363,7 @@
                            new-state
                            new-component-id)]
 
-                     (t/is (= (:name component2) "Rect-2")))))]
+                     (t/is (= (:name component2) "Rect-3")))))]
 
      (ptk/emit!
        store

frontend/test/app/test_helpers/libraries.cljs

@@ -77,39 +77,44 @@
 (defn resolve-instance-and-main
   "Get the shape with the given id and all its children, and also
    the main component and all its shapes."
-  [state root-inst-id]
+  ([state root-inst-id]
-  (let [page          (thp/current-page state)
+   (resolve-instance-and-main state root-inst-id false))
-        root-inst     (ctn/get-shape page root-inst-id)
 
-        libs          (wsh/get-libraries state)
+  ([state root-inst-id subinstance?]
-        component     (cph/get-component libs (:component-id root-inst))
+   (let [page          (thp/current-page state)
+         root-inst     (ctn/get-shape page root-inst-id)
 
-        shapes-inst   (cph/get-children-with-self (:objects page) root-inst-id)
+         libs          (wsh/get-libraries state)
-        shapes-main   (cph/get-children-with-self (:objects component) (:shape-ref root-inst))
+         component     (cph/get-component libs (:component-id root-inst))
 
-        unique-refs   (into #{} (map :shape-ref) shapes-inst)
+         shapes-inst   (cph/get-children-with-self (:objects page) root-inst-id)
+         shapes-main   (cph/get-children-with-self (:objects component) (:shape-ref root-inst))
 
-        main-exists?  (fn [shape]
+         unique-refs   (into #{} (map :shape-ref) shapes-inst)
-                        (let [component-shape
-                              (cph/get-component-shape (:objects page) shape)
 
-                              component
+         main-exists?  (fn [shape]
-                              (cph/get-component libs (:component-id component-shape))
+                         (let [component-shape
+                               (cph/get-component-shape (:objects page) shape)
 
-                              main-shape
+                               component
-                              (ctn/get-shape component (:shape-ref shape))]
+                               (cph/get-component libs (:component-id component-shape))
 
-                        (t/is (some? main-shape))))]
+                               main-shape
+                               (ctn/get-shape component (:shape-ref shape))]
 
-    ;; Validate that the instance tree is well constructed
+                           (t/is (some? main-shape))))]
-    (is-instance-root (first shapes-inst))
-    (run! is-instance-inner (rest shapes-inst))
-    (t/is (= (count shapes-inst)
-             (count shapes-main)
-             (count unique-refs)))
-    (run! main-exists? shapes-inst)
 
-    [shapes-inst shapes-main component]))
+     ;; Validate that the instance tree is well constructed
+     (if subinstance?
+       (is-instance-subroot (first shapes-inst))
+       (is-instance-root (first shapes-inst)))
+     (run! is-instance-inner (rest shapes-inst))
+     (t/is (= (count shapes-inst)
+              (count shapes-main)
+              (count unique-refs)))
+     (run! main-exists? shapes-inst)
+
+     [shapes-inst shapes-main component])))
 
 (defn resolve-instance-and-main-allow-dangling
   "Get the shape with the given id and all its children, and also