diff --git a/common/src/app/common/logic/libraries.cljc b/common/src/app/common/logic/libraries.cljc index 848b837fc0..455d667c1c 100644 --- a/common/src/app/common/logic/libraries.cljc +++ b/common/src/app/common/logic/libraries.cljc @@ -825,20 +825,30 @@ changes))) (defn- find-main-container - "Find the container that has the main shape." - [container-inst shape-inst shape-main library component] + "Find the container that has the main shape. + + When walking up through nested component copies, each intermediate component + may live in a different library/file. We resolve the right file-data for each + component via `ctf/find-component-file` so we can locate components that span + multiple libraries." + [container-inst shape-inst shape-main library file libraries component] (loop [shape-inst' shape-inst - component' component] - (let [container (ctf/get-component-container library component')] ; TODO: this won't work if some intermediate component is in a different library - (if (some? (ctn/get-shape container (:id shape-main))) ; for this to work we need to have access to the libraries list here + library' library + component' component] + (let [container (ctf/get-component-container library' component')] + (if (some? (ctn/get-shape container (:id shape-main))) container - (let [parent (ctn/get-shape container-inst (:parent-id shape-inst')) + (let [parent (ctn/get-shape container-inst (:parent-id shape-inst')) shape-inst' (ctn/get-head-shape (:objects container-inst) parent) - component' (or (ctkl/get-component library (:component-id shape-inst')) - (ctkl/get-deleted-component library (:component-id shape-inst')))] + next-file (some-> shape-inst' + :component-file + (->> (ctf/find-component-file file libraries))) + next-data (some-> next-file :data) + component' (when next-data + (or (ctkl/get-component next-data (:component-id shape-inst')) + (ctkl/get-deleted-component next-data (:component-id shape-inst'))))] (if (some? component') - (recur shape-inst' - component') + (recur shape-inst' next-data component') nil)))))) (defn- generate-sync-shape-direct-recursive @@ -884,7 +894,7 @@ set-remote-synced? (change-remote-synced shape-inst container true)) - component-container (find-main-container container shape-inst shape-main library component) + component-container (find-main-container container shape-inst shape-main library file libraries component) children-inst (vec (ctn/get-direct-children container shape-inst)) children-main (vec (ctn/get-direct-children component-container shape-main)) diff --git a/common/test/common_tests/logic/comp_reset_test.cljc b/common/test/common_tests/logic/comp_reset_test.cljc index 649b25e757..b07c27c2f8 100644 --- a/common/test/common_tests/logic/comp_reset_test.cljc +++ b/common/test/common_tests/logic/comp_reset_test.cljc @@ -14,6 +14,7 @@ [app.common.test-helpers.files :as thf] [app.common.test-helpers.ids-map :as thi] [app.common.test-helpers.shapes :as ths] + [app.common.types.container :as ctn] [clojure.test :as t])) (t/use-fixtures :each thi/test-fixture) @@ -386,6 +387,85 @@ ;; After reset + propagation the copy should mirror the canonical color (t/is (= copy2-bottom-color "#aabbcc")))) +(t/deftest test-reset-nested-component-from-different-library-preserves-children + ;; Regression test for the cross-library find-main-container bug: + ;; avatar-img lives in library1, avatar (which nests avatar-img) lives in library2, + ;; and card (which nests avatar) lives in the current file. + ;; Resetting overrides on avatar used to delete avatar-img because find-main-container + ;; only searched within the single library argument (library2) and could not find + ;; the avatar-img component which belongs to library1. + (let [;; ==== Setup + ;; library1: avatar-img-component (simple component with a child rect) + library1 + (-> (thf/sample-file :library1 :is-shared true) + (tho/add-simple-component :avatar-img-component + :avatar-img-main + :avatar-img-child + :child-params {:fills (ths/sample-fills-color :fill-color "#abcdef")})) + + ;; library2: avatar-component (nests avatar-img from library1) + library2 + (-> (thf/sample-file :library2 :is-shared true) + (tho/add-frame :avatar-main) + (thc/instantiate-component :avatar-img-component + :avatar-img-nested-head + :library library1 + :parent-label :avatar-main + :children-labels [:avatar-img-nested-child]) + (thc/make-component :avatar-component :avatar-main)) + + ;; file: card-component (nests avatar from library2) + a copy of card + file + (-> (thf/sample-file :file) + (tho/add-frame :card-main) + (thc/instantiate-component :avatar-component + :avatar-in-card-main + :library library2 + :parent-label :card-main + :children-labels [:avatar-img-in-card-main]) + (thc/make-component :card-component :card-main) + (thc/instantiate-component :card-component + :card-copy + :children-labels [:avatar-in-card-copy])) + + page (thf/current-page file) + avatar-in-card-copy (ths/get-shape file :avatar-in-card-copy) + + ;; ==== Action: override a fill on the avatar copy, then reset + changes + (cls/generate-update-shapes (pcb/empty-changes nil (:id page)) + #{(:id avatar-in-card-copy)} + (fn [shape] + (assoc shape :fills (ths/sample-fills-color :fill-color "#fabada"))) + (:objects page) + {}) + + file-mdf (thf/apply-changes file changes) + container-mdf (ctn/get-container (:data file-mdf) :page (:id page)) + + changes + (cll/generate-reset-component (pcb/empty-changes) + file-mdf + {(:id file-mdf) file-mdf + (:id library1) library1 + (:id library2) library2} + container-mdf + (:id avatar-in-card-copy)) + + file' (thf/apply-changes file-mdf changes :validate? false) + + ;; ==== Get + avatar-in-card-copy' (ths/get-shape file' :avatar-in-card-copy) + ;; avatar-img nested head must still exist inside avatar-in-card-copy after reset + avatar-img-copy-id (first (:shapes avatar-in-card-copy')) + avatar-img-copy' (when avatar-img-copy-id + (ths/get-shape-by-id file' avatar-img-copy-id))] + + ;; ==== Check + (t/is (some? avatar-in-card-copy') "avatar copy must still exist after reset") + (t/is (some? avatar-img-copy-id) "avatar-img copy must not be deleted from avatar's children") + (t/is (some? avatar-img-copy') "avatar-img copy shape must be present in objects"))) + (t/deftest test-reset-without-propagation-does-not-update-copies ;; This is the regression test for the misplaced-parenthesis bug: when ;; propagate-fn is NOT passed to reset-overrides the copies of the component