From ed021711b69cf90513d74fc1ca80d2e7f99a03d3 Mon Sep 17 00:00:00 2001 From: FairyPiggyDev Date: Thu, 30 Apr 2026 02:48:04 -0400 Subject: [PATCH] :recycle: Extract make-delete-asset-group-fn helper for assets panel (#9211) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reviewer follow-up on PR #9151. The "Delete group" handler was duplicated across the three assets-panel sections (colors, typographies, components), each carrying the same skeleton — filter by group path, build an undo-id, run the deletes inside one transaction, and show the same confirm modal — with only the path predicate and the per-asset delete event differing. Add `app.main.ui.workspace.sidebar.assets.common/make-delete-asset-group-fn` that takes the differing parts as options: - `:assets` collection to filter. - `:on-clear-selection` invoked before the deletes. - `:delete-events` `(fn [matching-assets] => seq-of-events)`. - `:path-filter` predicate (defaults to `str/starts-with?`), overridden to `cpn/inside-path?` for components so nested group paths match the same way the existing ungroup/combine helpers do. The factory returns `(fn [path] …)` so each call site stays a straight `mf/use-fn`. The variant-container dedup in components (one `delete-shapes` per container, not one per sibling variant) moves into that section's `:delete-events` fn and is unchanged in behavior. Cleanup ------- The `:as i18n` alias is no longer needed in any of the three section files (its only use was `i18n/c` for the modal count, which the helper now handles); reduced to `:refer [tr]`. Github #9141 Signed-off-by: FairyPigDev Co-authored-by: Andrey Antukh --- .../ui/workspace/sidebar/assets/colors.cljs | 38 ++------- .../ui/workspace/sidebar/assets/common.cljs | 33 ++++++++ .../workspace/sidebar/assets/components.cljs | 77 ++++++------------- .../sidebar/assets/typographies.cljs | 38 ++------- 4 files changed, 70 insertions(+), 116 deletions(-) diff --git a/frontend/src/app/main/ui/workspace/sidebar/assets/colors.cljs b/frontend/src/app/main/ui/workspace/sidebar/assets/colors.cljs index 83df80b9db..b4e94bb92d 100644 --- a/frontend/src/app/main/ui/workspace/sidebar/assets/colors.cljs +++ b/frontend/src/app/main/ui/workspace/sidebar/assets/colors.cljs @@ -29,7 +29,7 @@ [app.main.ui.workspace.sidebar.assets.groups :as grp] [app.util.color :as uc] [app.util.dom :as dom] - [app.util.i18n :as i18n :refer [tr]] + [app.util.i18n :refer [tr]] [app.util.keyboard :as kbd] [cuerdas.core :as str] [okulary.core :as l] @@ -501,38 +501,12 @@ file-id)))) (st/emit! (dwu/commit-undo-transaction undo-id))))) - ;; Issue #9141. Delete every color under a group path in a - ;; single undo transaction, after user confirmation. on-delete-group - (mf/use-fn - (mf/deps colors on-clear-selection) - (fn [path] - (let [group-colors - (->> colors - (filter #(str/starts-with? (:path %) path))) - - ;; Hoisted so the start/commit pair is bound to the - ;; same symbol regardless of how `do-delete` is - ;; invoked by the confirm modal. Review suggestion - ;; on PR #9151. - undo-id (js/Symbol) - - do-delete - (fn [] - (on-clear-selection) - (st/emit! (dwu/start-undo-transaction undo-id)) - (run! st/emit! - (map #(dwl/delete-color {:id (:id %)}) group-colors)) - (st/emit! (dwu/commit-undo-transaction undo-id)))] - (when (seq group-colors) - (st/emit! - (modal/show - {:type :confirm - :title (tr "modals.delete-asset-group.title") - :message (tr "modals.delete-asset-group.message" - (i18n/c (count group-colors))) - :accept-label (tr "labels.delete") - :on-accept do-delete})))))) + (mf/with-memo [colors on-clear-selection] + (cmm/make-delete-asset-group-fn + {:assets colors + :on-clear-selection on-clear-selection + :delete-events #(map (fn [c] (dwl/delete-color {:id (:id c)})) %)})) on-asset-click (mf/use-fn (mf/deps groups on-asset-click) (partial on-asset-click groups))] diff --git a/frontend/src/app/main/ui/workspace/sidebar/assets/common.cljs b/frontend/src/app/main/ui/workspace/sidebar/assets/common.cljs index 14f494504d..28eb7e4699 100644 --- a/frontend/src/app/main/ui/workspace/sidebar/assets/common.cljs +++ b/frontend/src/app/main/ui/workspace/sidebar/assets/common.cljs @@ -198,6 +198,39 @@ (run! st/emit!)) (st/emit! (dwu/commit-undo-transaction undo-id)))) +(defn make-delete-asset-group-fn + "Build an `:on-delete-group` handler that filters `assets` by group + path, asks the user to confirm, and on accept emits every event + produced by `delete-events` inside one undo transaction. + + Options: + - `:assets` collection to filter. + - `:on-clear-selection` invoked before the deletes. + - `:delete-events` `(fn [matching-assets] => seq-of-events)`. + - `:path-filter` `(fn [asset-path group-path] => bool)` deciding + which assets fall under the group. Defaults to + `str/starts-with?`." + [{:keys [assets on-clear-selection delete-events path-filter] + :or {path-filter str/starts-with?}}] + (fn [path] + (let [matching (filter #(path-filter (:path %) path) assets) + undo-id (js/Symbol) + do-delete + (fn [] + (on-clear-selection) + (st/emit! (dwu/start-undo-transaction undo-id)) + (run! st/emit! (delete-events matching)) + (st/emit! (dwu/commit-undo-transaction undo-id)))] + (when (seq matching) + (st/emit! + (modal/show + {:type :confirm + :title (tr "modals.delete-asset-group.title") + :message (tr "modals.delete-asset-group.message" + (c (count matching))) + :accept-label (tr "labels.delete") + :on-accept do-delete})))))) + (defn on-drop-asset [event asset dragging* selected selected-full selected-paths rename] (let [create-typed-assets-group (partial create-assets-group rename)] diff --git a/frontend/src/app/main/ui/workspace/sidebar/assets/components.cljs b/frontend/src/app/main/ui/workspace/sidebar/assets/components.cljs index 2d9d1b72e7..e2f0637c02 100644 --- a/frontend/src/app/main/ui/workspace/sidebar/assets/components.cljs +++ b/frontend/src/app/main/ui/workspace/sidebar/assets/components.cljs @@ -33,7 +33,7 @@ [app.main.ui.workspace.sidebar.assets.groups :as grp] [app.util.dom :as dom] [app.util.dom.dnd :as dnd] - [app.util.i18n :as i18n :refer [tr]] + [app.util.i18n :refer [tr]] [cuerdas.core :as str] [okulary.core :as l] [potok.v2.core :as ptk] @@ -496,59 +496,32 @@ (map #(dwv/rename-comp-or-variant-and-main (:id %) (cmm/ungroup % path))))) (st/emit! (dwu/commit-undo-transaction undo-id))))) - ;; Issue #9141. Delete every component under a group path in a - ;; single undo transaction, after user confirmation. Variants - ;; are handled via their variant container (matching the - ;; per-item delete dispatch in file_library.cljs); sibling - ;; variants sharing a container are deduplicated so we delete - ;; each container only once. on-delete-group - (mf/use-fn - (mf/deps components on-clear-selection) - (fn [path] - (let [group-components - (->> components - (filter #(cpn/inside-path? (:path %) path))) + (mf/with-memo [components on-clear-selection] + (cmm/make-delete-asset-group-fn + {:assets components + :on-clear-selection on-clear-selection + :path-filter cpn/inside-path? + ;; Variants are handled via their variant container + ;; (matching the per-item delete dispatch in + ;; file_library.cljs); sibling variants sharing a + ;; container are deduplicated so we delete each container + ;; only once. + :delete-events + (fn [matching] + (let [{variants true non-variants false} + (group-by (comp boolean ctc/is-variant?) matching) - {variants true non-variants false} - (group-by (comp boolean ctc/is-variant?) group-components) - - ;; One delete-shapes per variant container, not per - ;; sibling variant within that container. - variant-containers - (->> variants - (group-by :variant-id) - (map (fn [[_ comps]] (first comps)))) - - ;; Hoisted so the start/commit pair is bound to the - ;; same symbol regardless of how `do-delete` is - ;; invoked by the confirm modal. Review suggestion - ;; on PR #9151. - undo-id (js/Symbol) - - do-delete - (fn [] - (on-clear-selection) - (st/emit! (dwu/start-undo-transaction undo-id)) - (run! st/emit! - (map (fn [component] - (dwsh/delete-shapes (:main-instance-page component) - #{(:variant-id component)})) - variant-containers)) - (run! st/emit! - (map (fn [component] - (dwl/delete-component {:id (:id component)})) - non-variants)) - (st/emit! (dwu/commit-undo-transaction undo-id)))] - (when (seq group-components) - (st/emit! - (modal/show - {:type :confirm - :title (tr "modals.delete-asset-group.title") - :message (tr "modals.delete-asset-group.message" - (i18n/c (count group-components))) - :accept-label (tr "labels.delete") - :on-accept do-delete})))))) + variant-containers + (->> variants + (group-by :variant-id) + (map (fn [[_ comps]] (first comps))))] + (concat + (map #(dwsh/delete-shapes (:main-instance-page %) + #{(:variant-id %)}) + variant-containers) + (map #(dwl/delete-component {:id (:id %)}) + non-variants))))})) on-group-combine-variants (mf/use-fn diff --git a/frontend/src/app/main/ui/workspace/sidebar/assets/typographies.cljs b/frontend/src/app/main/ui/workspace/sidebar/assets/typographies.cljs index cf95f8b477..174648e8ca 100644 --- a/frontend/src/app/main/ui/workspace/sidebar/assets/typographies.cljs +++ b/frontend/src/app/main/ui/workspace/sidebar/assets/typographies.cljs @@ -26,7 +26,7 @@ [app.main.ui.workspace.sidebar.assets.groups :as grp] [app.main.ui.workspace.sidebar.options.menus.typography :refer [typography-entry]] [app.util.dom :as dom] - [app.util.i18n :as i18n :refer [tr]] + [app.util.i18n :refer [tr]] [cuerdas.core :as str] [okulary.core :as l] [potok.v2.core :as ptk] @@ -354,38 +354,12 @@ (cmm/ungroup % path))))) (st/emit! (dwu/commit-undo-transaction undo-id))))) - ;; Issue #9141. Delete every typography under a group path in a - ;; single undo transaction, after user confirmation. on-delete-group - (mf/use-fn - (mf/deps typographies file-id on-clear-selection) - (fn [path] - (let [group-typographies - (->> typographies - (filter #(str/starts-with? (:path %) path))) - - ;; Hoisted so the start/commit pair is bound to the - ;; same symbol regardless of how `do-delete` is - ;; invoked by the confirm modal. Review suggestion - ;; on PR #9151. - undo-id (js/Symbol) - - do-delete - (fn [] - (on-clear-selection) - (st/emit! (dwu/start-undo-transaction undo-id)) - (run! st/emit! - (map #(dwl/delete-typography (:id %)) group-typographies)) - (st/emit! (dwu/commit-undo-transaction undo-id)))] - (when (seq group-typographies) - (st/emit! - (modal/show - {:type :confirm - :title (tr "modals.delete-asset-group.title") - :message (tr "modals.delete-asset-group.message" - (i18n/c (count group-typographies))) - :accept-label (tr "labels.delete") - :on-accept do-delete})))))) + (mf/with-memo [typographies on-clear-selection] + (cmm/make-delete-asset-group-fn + {:assets typographies + :on-clear-selection on-clear-selection + :delete-events #(map (fn [t] (dwl/delete-typography (:id t))) %)})) on-context-menu (mf/use-fn