diff --git a/common/src/app/common/logic/libraries.cljc b/common/src/app/common/logic/libraries.cljc index ea274a0b32..62b422b0b4 100644 --- a/common/src/app/common/logic/libraries.cljc +++ b/common/src/app/common/logic/libraries.cljc @@ -486,36 +486,41 @@ that use assets of the given type in the given library. If an asset id is given, only shapes linked to this particular asset will - be synchronized." - [changes file-id asset-type asset-id library-id libraries current-file-id] - (assert (contains? #{:colors :components :typographies} asset-type)) - (assert (or (nil? asset-id) (uuid? asset-id))) - (assert (uuid? file-id)) - (assert (uuid? library-id)) + be synchronized. - (container-log :info asset-id - :msg "Sync file with library" - :asset-type asset-type - :asset-id asset-id - :file (pretty-file file-id libraries current-file-id) - :library (pretty-file library-id libraries current-file-id)) + If early-return? is true, stops as soon as the first change is generated." + ([changes file-id asset-type asset-id library-id libraries current-file-id] + (generate-sync-file changes file-id asset-type asset-id library-id libraries current-file-id false)) + ([changes file-id asset-type asset-id library-id libraries current-file-id early-return?] + (assert (contains? #{:colors :components :typographies} asset-type)) + (assert (or (nil? asset-id) (uuid? asset-id))) + (assert (uuid? file-id)) + (assert (uuid? library-id)) - (let [file (get-in libraries [file-id :data])] - (loop [containers (ctf/object-containers-seq file) - changes changes] - (if-let [container (first containers)] - (do - (recur (next containers) - (pcb/concat-changes ;;TODO Remove concat changes - changes - (generate-sync-container (pcb/empty-changes nil) - asset-type - asset-id - library-id - container - libraries - current-file-id)))) - changes)))) + (container-log :info asset-id + :msg "Sync file with library" + :asset-type asset-type + :asset-id asset-id + :file (pretty-file file-id libraries current-file-id) + :library (pretty-file library-id libraries current-file-id)) + + (let [file (get-in libraries [file-id :data])] + (loop [containers (ctf/object-containers-seq file) + changes changes] + (let [container (first containers)] + (if (or (nil? container) + (and early-return? (seq (:redo-changes changes)))) + changes + (recur (next containers) + (pcb/concat-changes ;;TODO Remove concat changes + changes + (generate-sync-container (pcb/empty-changes nil) + asset-type + asset-id + library-id + container + libraries + current-file-id))))))))) (defn generate-sync-library "Generate changes to synchronize all shapes in all components of the @@ -523,35 +528,41 @@ the given library. If an asset id is given, only shapes linked to this particular asset will - be synchronized." - [changes file-id asset-type asset-id library-id libraries current-file-id] - (assert (contains? #{:colors :components :typographies} asset-type)) - (assert (or (nil? asset-id) (uuid? asset-id))) - (assert (uuid? file-id)) - (assert (uuid? library-id)) + be synchronized. - (container-log :info asset-id - :msg "Sync local components with library" - :asset-type asset-type - :asset-id asset-id - :file (pretty-file file-id libraries current-file-id) - :library (pretty-file library-id libraries current-file-id)) + If early-return? is true, stops as soon as the first change is generated." + ([changes file-id asset-type asset-id library-id libraries current-file-id] + (generate-sync-library changes file-id asset-type asset-id library-id libraries current-file-id false)) + ([changes file-id asset-type asset-id library-id libraries current-file-id early-return?] + (assert (contains? #{:colors :components :typographies} asset-type)) + (assert (or (nil? asset-id) (uuid? asset-id))) + (assert (uuid? file-id)) + (assert (uuid? library-id)) - (let [file (get-in libraries [file-id :data])] - (loop [local-components (ctkl/components-seq file) - changes changes] - (if-let [local-component (first local-components)] - (recur (next local-components) - (pcb/concat-changes ;;TODO Remove concat changes - changes - (generate-sync-container (pcb/empty-changes nil) - asset-type - asset-id - library-id - (cfh/make-container local-component :component) - libraries - current-file-id))) - changes)))) + (container-log :info asset-id + :msg "Sync local components with library" + :asset-type asset-type + :asset-id asset-id + :file (pretty-file file-id libraries current-file-id) + :library (pretty-file library-id libraries current-file-id)) + + (let [file (get-in libraries [file-id :data])] + (loop [local-components (ctkl/components-seq file) + changes changes] + (let [local-component (first local-components)] + (if (or (nil? local-component) + (and early-return? (seq (:redo-changes changes)))) + changes + (recur (next local-components) + (pcb/concat-changes ;;TODO Remove concat changes + changes + (generate-sync-container (pcb/empty-changes nil) + asset-type + asset-id + library-id + (cfh/make-container local-component :component) + libraries + current-file-id))))))))) (defn- generate-sync-container "Generate changes to synchronize all shapes in a particular container (a page @@ -2645,29 +2656,30 @@ (generate-new-shape-for-swap shape file page libraries id-new-component index target-cell keep-props-values))] [new-shape all-parents changes])) -(defn generate-sync-file-changes - [changes undo-group asset-type file-id asset-id library-id libraries current-file-id] - (let [sync-components? (or (nil? asset-type) (= asset-type :components)) - sync-colors? (or (nil? asset-type) (= asset-type :colors)) - sync-typographies? (or (nil? asset-type) (= asset-type :typographies))] - (cond-> changes - :always - (pcb/set-undo-group undo-group) - ;; library-changes - sync-components? - (generate-sync-library file-id :components asset-id library-id libraries current-file-id) - sync-colors? - (generate-sync-library file-id :colors asset-id library-id libraries current-file-id) - sync-typographies? - (generate-sync-library file-id :typographies asset-id library-id libraries current-file-id) +(defn- maybe-sync + [c enabled? done? f] + (if (and enabled? (not (done? c))) + (f c) + c)) + +(defn generate-sync-file-changes + ([changes undo-group asset-type file-id asset-id library-id libraries current-file-id] + (generate-sync-file-changes changes undo-group asset-type file-id asset-id library-id libraries current-file-id false)) + ([changes undo-group asset-type file-id asset-id library-id libraries current-file-id early-return?] + (let [sync-components? (or (nil? asset-type) (= asset-type :components)) + sync-colors? (or (nil? asset-type) (= asset-type :colors)) + sync-typographies? (or (nil? asset-type) (= asset-type :typographies)) + done? (fn [c] (and early-return? (seq (:redo-changes c))))] + (-> (pcb/set-undo-group changes undo-group) + ;; library-changes + (maybe-sync sync-components? done? #(generate-sync-library % file-id :components asset-id library-id libraries current-file-id early-return?)) + (maybe-sync sync-colors? done? #(generate-sync-library % file-id :colors asset-id library-id libraries current-file-id early-return?)) + (maybe-sync sync-typographies? done? #(generate-sync-library % file-id :typographies asset-id library-id libraries current-file-id early-return?)) + ;; file-changes + (maybe-sync sync-components? done? #(generate-sync-file % file-id :components asset-id library-id libraries current-file-id early-return?)) + (maybe-sync sync-colors? done? #(generate-sync-file % file-id :colors asset-id library-id libraries current-file-id early-return?)) + (maybe-sync sync-typographies? done? #(generate-sync-file % file-id :typographies asset-id library-id libraries current-file-id early-return?)))))) - ;; file-changes - sync-components? - (generate-sync-file file-id :components asset-id library-id libraries current-file-id) - sync-colors? - (generate-sync-file file-id :colors asset-id library-id libraries current-file-id) - sync-typographies? - (generate-sync-file file-id :typographies asset-id library-id libraries current-file-id)))) (defn generate-sync-head [changes file-full libraries container id reset?] diff --git a/common/test/common_tests/logic/comp_sync_test.cljc b/common/test/common_tests/logic/comp_sync_test.cljc index a52ea3d373..06bf7c4912 100644 --- a/common/test/common_tests/logic/comp_sync_test.cljc +++ b/common/test/common_tests/logic/comp_sync_test.cljc @@ -8,6 +8,8 @@ (:require [app.common.data :as d] [app.common.files.changes-builder :as pcb] + [app.common.geom.point :as gpt] + [app.common.geom.shapes :as gsh] [app.common.logic.libraries :as cll] [app.common.logic.shapes :as cls] [app.common.test-helpers.components :as thc] @@ -488,3 +490,52 @@ (t/is (= (:fill-opacity fill') 1)) (t/is (= (:touched copy2-root') nil)) (t/is (= (:touched copy2-child') nil)))) + +(t/deftest test-no-sync-changes-when-only-position-changes + ;; Regression: the library sync dialog was shown even when a library component + ;; was only moved (x/y changed). Position changes are normalised by + ;; reposition-shape during sync and never propagate to copies, so + ;; generate-sync-file-changes must return empty :redo-changes in this case. + (let [;; ==== Setup + ;; Use integer width/height so that floating-point arithmetic of + ;; move(+delta) followed by reposition(-delta) cancels out exactly. + file (-> (thf/sample-file :file1) + (tho/add-simple-component-with-copy :component1 + :main-root + :main-child + :copy-root + :main-root-params {:width 100 :height 100} + :main-child-params {:width 50 :height 50})) + page (thf/current-page file) + main-root (ths/get-shape file :main-root) + main-child (ths/get-shape file :main-child) + + ;; ==== Action + ;; Move the entire main component (root + child) by a non-zero integer delta. + ;; This is a position-only change: no fills, strokes or other + ;; attributes are modified. + delta (gpt/point 100 150) + changes1 (cls/generate-update-shapes (pcb/empty-changes nil (:id page)) + #{(:id main-root) (:id main-child)} + (fn [shape] (gsh/move shape delta)) + (:objects page) + {}) + + updated-file (thf/apply-changes file changes1) + + ;; Run the full sync to check whether any real redo-changes are produced. + ;; The fixed frontend code filters out libraries whose sync produces no + ;; :redo-changes before showing the "library updated" notification. + sync-changes (cll/generate-sync-file-changes (pcb/empty-changes) + nil + :components + (:id updated-file) + (thi/id :component1) + (:id updated-file) + {(:id updated-file) updated-file} + (:id updated-file))] + + ;; ==== Check + ;; A position-only change in the main component must not propagate to copies + ;; and therefore must produce no redo-changes. + (t/is (empty? (:redo-changes sync-changes))))) diff --git a/frontend/src/app/main/data/workspace/libraries.cljs b/frontend/src/app/main/data/workspace/libraries.cljs index 6c5625a9e7..1ca2d4cab7 100644 --- a/frontend/src/app/main/data/workspace/libraries.cljs +++ b/frontend/src/app/main/data/workspace/libraries.cljs @@ -1281,38 +1281,62 @@ file (dsh/lookup-file state file-id) file-data (get file :data) ignore-until (get file :ignore-sync-until) - permissions (:permissions state) + permissions (:permissions state) libraries-need-sync (->> (vals (get state :files)) (filter #(= (:library-of %) file-id)) - (filter #(seq (assets-need-sync % file-data ignore-until)))) + (filter #(seq (assets-need-sync % file-data ignore-until))))] - do-more-info - #(modal/show! :libraries-dialog {:starting-tab "updates" :file-id file-id}) + (if-not (and (:can-edit permissions) + (seq libraries-need-sync)) + ;; Fast path: no libraries need sync based on timestamps + (rx/empty) - do-update - #(do (apply st/emit! (map (fn [library] - (sync-file (:current-file-id state) - (:id library))) - libraries-need-sync)) - (st/emit! (ntf/hide))) + ;; Defer the expensive change generation check to avoid blocking the UI. + ;; For files with many libraries, this prevents stuttering/freezing. + (->> (rx/timer 0) + (rx/map (fn [_] + ;; 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)))) + (rx/filter seq) + (rx/map (fn [libraries-with-changes] + (let [do-more-info + #(modal/show! :libraries-dialog {:starting-tab "updates" :file-id file-id}) - do-dismiss - #(st/emit! ignore-sync (ntf/hide))] + do-update + #(do (apply st/emit! (map (fn [library] + (sync-file file-id (:id library))) + libraries-with-changes)) + (st/emit! (ntf/hide))) - (when (and (:can-edit permissions) - (seq libraries-need-sync)) - (rx/of (ntf/dialog - :content (tr "workspace.updates.there-are-updates") - :controls :inline-actions - :links [{:label (tr "workspace.updates.more-info") - :callback do-more-info}] - :cancel {:label (tr "workspace.updates.dismiss") - :callback do-dismiss} - :accept {:label (tr "workspace.updates.update") - :callback do-update} - :tag :sync-dialog))))))) + do-dismiss + #(st/emit! ignore-sync (ntf/hide))] + + (ntf/dialog + :content (tr "workspace.updates.there-are-updates") + :controls :inline-actions + :links [{:label (tr "workspace.updates.more-info") + :callback do-more-info}] + :cancel {:label (tr "workspace.updates.dismiss") + :callback do-dismiss} + :accept {:label (tr "workspace.updates.update") + :callback do-update} + :tag :sync-dialog)))))))))) (defn touch-component "Update the modified-at attribute of the component to now"