From f2b252f84280d4e505e45cccb8e8981e6945d094 Mon Sep 17 00:00:00 2001 From: "alonso.torres" Date: Wed, 14 Oct 2026 16:25:51 +0200 Subject: [PATCH] :recycle: Share code that the component events repeated - component-changed marked the component modified and sent launch-component-sync, its only caller, which opened an undo transaction around the syncs of the copies. component-changed sends those events itself, in the same order. - The library update notification found the libraries that would change with a lazy filter, so the check ran up to the first one when the notification was built and the rest when the user clicked "Update". It is a filterv, so the whole check runs when the notification is built, as the comment above it says. - The check that a new name is valid (trimmed, not blank, no trailing separator) was written in the shape rename and the component rename; cpn/clean-valid-name gives the cleaned name, or nil. - Paste and the move of shapes into a parent each tested whether they put a component head inside a variant; ctn/head-into-variant? does. - Going to the local main of a component and combining components into variants each opened another page and ran their work once it was initialized, with the same stream code; dwl/go-to-page-then does it. - Making components from the selection mapped the check over the shapes and then tested every result; every? takes the check. - generate-components-menu-entries reads the current file and page with mf/use-ctx, so it is a hook, and called conditionally it would break the order of its caller's hooks. It is use-components-menu-entries, and its docstring says why. The analytics events that paste and duplicate send for each new shape are built the same way but look up the parent's layout in different objects, so they stay apart. No behavior change: the suites are unchanged. AI-assisted-by: claude-opus-5-5 --- common/src/app/common/path_names.cljc | 5 ++ common/src/app/common/types/container.cljc | 7 ++ .../app/main/data/workspace/clipboard.cljs | 7 +- .../app/main/data/workspace/libraries.cljs | 85 +++++++++---------- .../src/app/main/data/workspace/shapes.cljs | 7 +- .../src/app/main/data/workspace/variants.cljs | 15 +--- .../app/main/ui/workspace/context_menu.cljs | 2 +- .../ui/workspace/sidebar/assets/common.cljs | 4 +- .../sidebar/options/menus/component.cljs | 2 +- 9 files changed, 60 insertions(+), 74 deletions(-) diff --git a/common/src/app/common/path_names.cljc b/common/src/app/common/path_names.cljc index 3f5fd28603..b098b10281 100644 --- a/common/src/app/common/path_names.cljc +++ b/common/src/app/common/path_names.cljc @@ -66,6 +66,11 @@ Some naming conventions: (not (str/blank? clean-name))) clean-name))) +(defn clean-valid-name + "Return the normalized path name when it is valid." + [name] + (normalize-path-name name)) + (defn merge-path-item "Put the item at the end of the path." [path name] diff --git a/common/src/app/common/types/container.cljc b/common/src/app/common/types/container.cljc index f61e08e783..5e405a0995 100644 --- a/common/src/app/common/types/container.cljc +++ b/common/src/app/common/types/container.cljc @@ -388,6 +388,13 @@ [(fix-parent-id new-shape) (map fix-parent-id new-shapes)]))) +(defn head-into-variant? + "Whether placing `shapes` under `parent-id` puts a component head inside + a variant." + [objects parent-id shapes] + (and (some ctk/instance-head? shapes) + (some ctk/is-variant? (cfh/get-parents-with-self objects parent-id)))) + (defn get-first-valid-parent "Go through the parents until we find a shape that is not a copy of a component nor a variant container." diff --git a/frontend/src/app/main/data/workspace/clipboard.cljs b/frontend/src/app/main/data/workspace/clipboard.cljs index 41757c3b38..8f30686bea 100644 --- a/frontend/src/app/main/data/workspace/clipboard.cljs +++ b/frontend/src/app/main/data/workspace/clipboard.cljs @@ -989,12 +989,7 @@ (pcb/update-shapes [parent-id] #(ctl/add-children-to-cell % selected all-objects drop-cell))) - add-component-to-variant? (and - ;; Any of the shapes is a head - (some ctk/instance-head? orig-shapes) - ;; Any ancestor of the destination parent is a variant - (->> (cfh/get-parents-with-self page-objects parent-id) - (some ctk/is-variant?))) + add-component-to-variant? (ctn/head-into-variant? page-objects parent-id orig-shapes) undo-id (js/Symbol)] (rx/concat diff --git a/frontend/src/app/main/data/workspace/libraries.cljs b/frontend/src/app/main/data/workspace/libraries.cljs index 3cca13643a..1a5a2e1a44 100644 --- a/frontend/src/app/main/data/workspace/libraries.cljs +++ b/frontend/src/app/main/data/workspace/libraries.cljs @@ -389,7 +389,7 @@ (cfh/clean-loops objects)) selected-objects (map #(get objects %) selected) ;; We don't want to change the structure of component copies - can-make-component (every? true? (map #(ctn/valid-shape-for-component? objects %) selected-objects))] + can-make-component (every? #(ctn/valid-shape-for-component? objects %) selected-objects)] (when can-make-component (rx/of (-> (add-component2 id-ref selected) @@ -406,7 +406,7 @@ (cfh/clean-loops objects)) selected-objects (map #(get objects %) selected) ;; We don't want to change the structure of component copies - can-make-component (every? true? (map #(ctn/valid-shape-for-component? objects %) selected-objects)) + can-make-component (every? #(ctn/valid-shape-for-component? objects %) selected-objects) added-components (map (fn [id] (with-meta (add-component2 [id]) {:multiple true})) @@ -687,6 +687,18 @@ (assoc :update-layout update-layout?))] (rx/of (rt/nav :workspace params ::rt/new-window true)))))) +(defn go-to-page-then + "Open the page `page-id` and, once it is initialized, emit the events of + `(f)`." + [stream page-id f] + (rx/merge + (->> stream + (rx/filter (ptk/type? ::dwpg/initialize-page)) + (rx/take 1) + (rx/observe-on :async) + (rx/mapcat (fn [_] (f)))) + (rx/of (dcm/go-to-workspace :page-id page-id)))) + (defn go-to-local-component ;; id is the id of the component to go ;; additional-ids are ids of additional components on the same page @@ -716,16 +728,6 @@ (when update-layout? (rx/of (ptk/data-event :layout/update {:ids parent-ids}))))))) - redirect-to-page - (fn [page-id ids] - (rx/merge - (->> stream - (rx/filter (ptk/type? ::dwpg/initialize-page)) - (rx/take 1) - (rx/observe-on :async) - (rx/mapcat (fn [_] (select-and-zoom ids)))) - (rx/of (dcm/go-to-workspace :page-id page-id)))) - get-main-instance-id (fn [id page-id] (let [component (dm/get-in data [:components id])] @@ -741,7 +743,7 @@ (when (some? page-id) (if (= page-id current-page-id) (select-and-zoom ids) - (redirect-to-page page-id ids))))))))) + (go-to-page-then stream page-id #(select-and-zoom ids)))))))))) (defn library-thumbnails-fetched [thumbnails] @@ -955,20 +957,6 @@ (component-sync-events state component-id file-id undo-group) [(dwu/commit-undo-transaction undo-id)]))))))) -(defn launch-component-sync - "Launch a sync of the current file and of the library file of the given - component, in one undo entry." - [component-id file-id undo-group] - (ptk/reify ::launch-component-sync - ptk/WatchEvent - (watch [_ state _] - (let [undo-id (js/Symbol)] - (apply rx/of - (concat - [(dwu/start-undo-transaction undo-id)] - (component-sync-events state component-id file-id undo-group) - [(dwu/commit-undo-transaction undo-id)])))))) - (defn update-component-thumbnail "Update the thumbnail of the component with the given id, in the current file and in the imported libraries. @@ -1278,19 +1266,19 @@ ;; This runs asynchronously on the next tick. ;; Filter libraries to only those that would produce actual sync changes. (let [libraries (dsh/lookup-libraries state)] - (filter (fn [library] - (seq (:redo-changes - (cll/generate-sync-file-changes - (pcb/empty-changes) - nil - nil - file-id - nil - (:id library) - libraries - file-id - true)))) - libraries-need-sync)))) + (filterv (fn [library] + (seq (:redo-changes + (cll/generate-sync-file-changes + (pcb/empty-changes) + nil + nil + file-id + nil + (:id library) + libraries + file-id + true)))) + libraries-need-sync)))) (rx/filter seq) (rx/map (fn [libraries-with-changes] (let [do-more-info @@ -1341,18 +1329,23 @@ :save-undo? false})))))) (defn component-changed - "Notify that the component with the given id has changed, so it needs to be updated - in the current file and in the copies. And also update its thumbnails." + "Notify that the component with the given id has changed: mark it modified + and sync its copies, in the current file and in its library, in one undo + entry. The watcher also updates its thumbnails." [component-id file-id undo-group] (ptk/reify ::component-changed cljs.core/IDeref (-deref [_] [component-id file-id]) ptk/WatchEvent - (watch [_ _ _] - (rx/of - (touch-component component-id) - (launch-component-sync component-id file-id undo-group))))) + (watch [_ state _] + (let [undo-id (js/Symbol)] + (apply rx/of + (concat + [(touch-component component-id) + (dwu/start-undo-transaction undo-id)] + (component-sync-events state component-id file-id undo-group) + [(dwu/commit-undo-transaction undo-id)])))))) (defn watch-component-changes "Watch the state for changes that affect to any main instance. If a change is detected will call diff --git a/frontend/src/app/main/data/workspace/shapes.cljs b/frontend/src/app/main/data/workspace/shapes.cljs index 587956c084..347c997f6e 100644 --- a/frontend/src/app/main/data/workspace/shapes.cljs +++ b/frontend/src/app/main/data/workspace/shapes.cljs @@ -627,12 +627,7 @@ ids :ignore-parents? ignore-parents?)) - add-component-to-variant? (and - ;; Any of the shapes is a head - (some (comp ctc/instance-head? objects) ids) - ;; Any ancestor of the destination parent is a variant - (->> (cfh/get-parents-with-self objects parent-id) - (some ctc/is-variant?))) + add-component-to-variant? (ctn/head-into-variant? objects parent-id (map objects ids)) add-new-variant? (and ;; The parent is a variant container diff --git a/frontend/src/app/main/data/workspace/variants.cljs b/frontend/src/app/main/data/workspace/variants.cljs index 9dfd89a3f5..f8f320f5d4 100644 --- a/frontend/src/app/main/data/workspace/variants.cljs +++ b/frontend/src/app/main/data/workspace/variants.cljs @@ -28,7 +28,6 @@ [app.main.data.helpers :as dsh] [app.main.data.workspace.colors :as cl] [app.main.data.workspace.libraries :as dwl] - [app.main.data.workspace.pages :as dwpg] [app.main.data.workspace.selection :as dws] [app.main.data.workspace.shape-layout :as dwsl] [app.main.data.workspace.shapes :as dwsh] @@ -620,20 +619,10 @@ ;; microtask of previous events execute before the ;; commit (->> (rx/of (dwu/commit-undo-transaction undo-id)) - (rx/observe-on :async))))))) - - redirect-to-page - (fn [page-id] - (rx/merge - (->> stream - (rx/filter (ptk/type? ::dwpg/initialize-page)) - (rx/take 1) - (rx/observe-on :async) - (rx/mapcat (fn [_] (combine page-id)))) - (rx/of (dcm/go-to-workspace :page-id page-id))))] + (rx/observe-on :async)))))))] (if (and page-id (not= page-id current-page)) - (redirect-to-page page-id) + (dwl/go-to-page-then stream page-id #(combine page-id)) (combine current-page)))))) (defn valid-components-for-variants? diff --git a/frontend/src/app/main/ui/workspace/context_menu.cljs b/frontend/src/app/main/ui/workspace/context_menu.cljs index 7cb59b75e5..cbd77eacba 100644 --- a/frontend/src/app/main/ui/workspace/context_menu.cljs +++ b/frontend/src/app/main/ui/workspace/context_menu.cljs @@ -676,7 +676,7 @@ single? (= (count shapes) 1) objects (deref refs/workspace-page-objects) can-make-component (every? true? (map #(ctn/valid-shape-for-component? objects %) shapes)) - components-menu-entries (cmm/generate-components-menu-entries shapes) + components-menu-entries (cmm/use-components-menu-entries shapes) variant-container? (and single? (ctk/is-variant-container? (first shapes))) all-main? (every? ctk/main-instance? shapes) any-variant? (some ctk/is-variant? shapes) 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 ba3ff9786e..243f47c196 100644 --- a/frontend/src/app/main/ui/workspace/sidebar/assets/common.cljs +++ b/frontend/src/app/main/ui/workspace/sidebar/assets/common.cljs @@ -392,7 +392,9 @@ :show-grids? true :is-hidden is-hidden}]))) -(defn generate-components-menu-entries +(defn use-components-menu-entries + "The entries of the component menu of `shapes`: a hook, as it reads the + current file and page from the context." [shapes & {:keys [for-design-tab?]}] (let [multi (> (count shapes) 1) copies (filter ctk/in-component-copy? shapes) diff --git a/frontend/src/app/main/ui/workspace/sidebar/options/menus/component.cljs b/frontend/src/app/main/ui/workspace/sidebar/options/menus/component.cljs index 2f95e07918..c864f0ed09 100644 --- a/frontend/src/app/main/ui/workspace/sidebar/options/menus/component.cljs +++ b/frontend/src/app/main/ui/workspace/sidebar/options/menus/component.cljs @@ -1021,7 +1021,7 @@ (fn [] (swap! state* update :render inc))) - menu-entries (cmm/generate-components-menu-entries shapes {:for-design-tab? true}) + menu-entries (cmm/use-components-menu-entries shapes {:for-design-tab? true}) path (->> component (:path) (cpn/split-path) (cpn/join-path-with-dot))] (when (seq shapes)