🐛 Fix group/ungroup shapes inside a component

CHANGES.md

@@ -38,6 +38,7 @@
 - Fix incorrect color in properties of multiple bool shapes [Taiga #4355](https://tree.taiga.io/project/penpot/issue/4355)
 - Fix pressing the enter key gives you an internal error [Github 2675](https://github.com/penpot/penpot/issues/2675) [Github 2577](https://github.com/penpot/penpot/issues/2577)
 - Fix confirm group name with enter doesn't work in assets modal [Taiga #4506](https://tree.taiga.io/project/penpot/issue/4506)
+- Fix group/ungroup shapes inside a component [Taiga #4052](https://tree.taiga.io/project/penpot/issue/4052)
 
 ### :arrow_up: Deps updates
 

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

@@ -49,3 +49,22 @@
   [shape]
   (some? (:shape-ref shape)))
 
+(defn in-component-instance-not-root?
+  "Check if the shape is inside a component instance and
+  is not the root shape."
+  [shape]
+  (and (some? (:shape-ref shape))
+       (nil? (:component-id shape))))
+
+(defn detach-shape
+  "Remove the links and leave it as a plain shape, detached from any component."
+  [shape]
+  (dissoc shape
+          :component-id
+          :component-file
+          :component-root?
+          :remote-synced?
+          :shape-ref
+          :touched))
+
+

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

@@ -10,8 +10,9 @@
    [app.common.geom.shapes :as gsh]
    [app.common.pages.changes-builder :as pcb]
    [app.common.pages.helpers :as cph]
+   [app.common.types.component :as ctk]
    [app.common.types.shape :as cts]
-   [app.common.types.shape-tree :as ctt]
+   [app.common.types.shape-tree :as ctst]
    [app.main.data.workspace.changes :as dch]
    [app.main.data.workspace.selection :as dws]
    [app.main.data.workspace.state-helpers :as wsh]
@@ -71,8 +72,8 @@
                            (= (count shapes) 1)
                            (= (:type (first shapes)) :group))
                     (:name (first shapes))
-                    (-> (ctt/retrieve-used-names objects)
-                        (ctt/generate-unique-name base-name)))
+                    (-> (ctst/retrieve-used-names objects)
+                        (ctst/generate-unique-name base-name)))
 
         selrect   (gsh/selection-rect shapes)
         group     (-> (cts/make-minimal-group frame-id selrect gname)
@@ -82,6 +83,10 @@
                              :frame-id frame-id
                              :index (::index (first shapes))))
 
+        ;; Shapes that are in a component, but are not root, must be detached,
+        ;; because they will be now children of a non instance group.
+        shapes-to-detach (filter ctk/in-component-instance-not-root? shapes)
+
         ;; Look at the `get-empty-groups-after-group-creation`
         ;; docstring to understand the real purpose of this code
         ids-to-delete (get-empty-groups-after-group-creation objects parent-id shapes)
@@ -90,6 +95,7 @@
                       (pcb/with-objects objects)
                       (pcb/add-object group {:index (::index (first shapes))})
                       (pcb/change-parent (:id group) shapes)
+                      (pcb/update-shapes (map :id shapes-to-detach) ctk/detach-shape)
                       (pcb/remove-objects ids-to-delete))]
 
     [group changes]))
@@ -106,25 +112,16 @@
              (filter #(#{(:id group)} (second %)))
              (ffirst))
 
-        ids-to-detach (when (:component-id group)
-                        (cph/get-children-ids objects (:id group)))
+        ;; Shapes that are in a component (including root) must be detached,
+        ;; because cannot be easyly synchronized back to the main component.
+        shapes-to-detach (filter ctk/in-component-instance? 
+                                 (cph/get-children-with-self objects (:id group)))]
 
-        detach-fn (fn [attrs]
-                    (dissoc attrs
-                            :component-id
-                            :component-file
-                            :component-root?
-                            :remote-synced?
-                            :shape-ref
-                            :touched))]
-
-    (cond-> (-> (pcb/empty-changes it page-id)
-                (pcb/with-objects objects)
-                (pcb/change-parent parent-id children index-in-parent)
-                (pcb/remove-objects [(:id group)]))
-
-      (some? ids-to-detach)
-      (pcb/update-shapes ids-to-detach detach-fn))))
+    (-> (pcb/empty-changes it page-id)
+        (pcb/with-objects objects)
+        (pcb/change-parent parent-id children index-in-parent)
+        (pcb/remove-objects [(:id group)])
+        (pcb/update-shapes (map :id shapes-to-detach) ctk/detach-shape))))
 
 (defn remove-frame-changes
   [it page-id frame objects]

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

@@ -945,7 +945,7 @@
 
         changes' (reduce add-undo-change
                          changes'
-                         (map :id children))]
+                         children)]
 
     (if (and (cph/touched-group? parent :shapes-group) omit-touched?)
       changes