From 69bbdad570c829fbff4dcd629ea6f1b5b66c73f6 Mon Sep 17 00:00:00 2001 From: Pablo Alba Date: Wed, 29 Oct 2025 12:39:54 +0100 Subject: [PATCH] :bug: Fix nested variant in a component doesn't keep inherited overrides (3) --- common/src/app/common/logic/libraries.cljc | 11 ++-- common/src/app/common/logic/variants.cljc | 35 ++++------- .../src/app/common/test_helpers/variants.cljc | 10 ++-- common/src/app/common/types/file.cljc | 27 +++++++++ .../logic/variants_switch_test.cljc | 58 +++++++++++++++++++ .../app/main/data/workspace/clipboard.cljs | 26 +-------- 6 files changed, 109 insertions(+), 58 deletions(-) diff --git a/common/src/app/common/logic/libraries.cljc b/common/src/app/common/logic/libraries.cljc index 95a87057be..b3373e73c4 100644 --- a/common/src/app/common/logic/libraries.cljc +++ b/common/src/app/common/logic/libraries.cljc @@ -1992,6 +1992,12 @@ ;; If the values are already equal, don't copy them (= (get previous-shape attr) (get current-shape attr)) + ;; If the value is the same as the origin, don't copy it + (= (get previous-shape attr) (get origin-ref-shape attr)) + + ;; If the attr is not touched, don't copy it + (not (touched attr-group)) + ;; If both variants (origin and destiny) don't have the same value ;; for that attribute, don't copy it. ;; Exceptions: :points :selrect and :content can be different @@ -2007,10 +2013,7 @@ (not= (get origin-ref-shape attr) (get current-shape attr))) ;; The :content attr cant't be copied to elements of different type - (and (= attr :content) (not= (:type previous-shape) (:type current-shape))) - - ;; If the attr is not touched, don't copy it - (not (touched attr-group))) + (and (= attr :content) (not= (:type previous-shape) (:type current-shape)))) ;; On texts, both text (the actual letters) ;; and attrs (bold, font, etc) are in the same attr :content. diff --git a/common/src/app/common/logic/variants.cljc b/common/src/app/common/logic/variants.cljc index c0dc0409d0..0159b0c126 100644 --- a/common/src/app/common/logic/variants.cljc +++ b/common/src/app/common/logic/variants.cljc @@ -11,8 +11,7 @@ [app.common.types.container :as ctn] [app.common.types.file :as ctf] [app.common.types.variant :as ctv] - [app.common.uuid :as uuid] - [clojure.set :as set])) + [app.common.uuid :as uuid])) (defn generate-add-new-variant [changes shape variant-id new-component-id new-shape-id prop-num] @@ -131,34 +130,19 @@ ref-shape-container (when ref-shape (:container (meta ref-shape))) ref-shape-parents-set (when ref-shape - (->> (cfh/get-parents (:objects ref-shape-container) (:id ref-shape)) + (->> (cfh/get-parents-with-self (:objects ref-shape-container) (:id ref-shape)) (into #{} d/xf:map-id)))] (if (or (nil? ref-shape) (contains? ref-shape-parents-set parent-id)) ref-shape (find-shape-ref-child-of ref-shape-container libraries ref-shape parent-id)))) -(defn- get-ref-chain - "Returns a vector with the shape ref chain including itself" - [container libraries shape] - (loop [chain [shape] - current shape] - (if-let [ref (ctf/find-ref-shape nil container libraries current :with-context? true)] - (recur (conj chain ref) ref) - chain))) - (defn- add-touched-from-ref-chain "Adds to the :touched attr of a shape the content of the :touched of all its chain of ref shapes" [container libraries shape] - (let [chain (get-ref-chain container libraries shape) - more-touched (->> chain - (map :touched) - (remove nil?) - (apply set/union) - (remove ctk/swap-slot?) - set)] - (update shape :touched #(set/union (or % #{}) more-touched)))) + (let [new-touched (ctf/get-touched-from-ref-chain-until-target-ref container libraries shape nil)] + (assoc shape :touched new-touched))) (defn generate-keep-touched "This is used as part of the switch process, when you switch from @@ -198,15 +182,15 @@ ;; The original-shape is in a copy. For the relation rules, we need the referenced ;; shape on the main component - orig-ref-shape (ctf/find-remote-shape container libraries original-shape {:with-context? true}) - orig-ref-objects (:objects (:container (meta orig-ref-shape))) + orig-base-ref-shape (ctf/find-remote-shape container libraries original-shape {:with-context? true}) + orig-ref-objects (:objects (:container (meta orig-base-ref-shape))) ;; Adds a :shape-path attribute to the children of the orig-ref-shape, ;; that contains the type of its ancestors and its name o-ref-shapes-wp (add-unique-path - (reverse (cfh/get-children-with-self orig-ref-objects (:id orig-ref-shape))) + (reverse (cfh/get-children-with-self orig-ref-objects (:id orig-base-ref-shape))) orig-ref-objects - (:id orig-ref-shape)) + (:id orig-base-ref-shape)) ;; Creates a map to quickly find a child of the orig-ref-shape by its shape-path o-ref-shapes-p-map (into {} (map (juxt :id :shape-path)) o-ref-shapes-wp) @@ -221,7 +205,7 @@ ;; orig-child-touched is in a copy. Get the referenced shape on the main component ;; If there is a swap slot, we will get the referenced shape in another way orig-ref-shape (when-not swap-slot - (find-shape-ref-child-of container libraries orig-child-touched (:id orig-ref-shape))) + (find-shape-ref-child-of container libraries orig-child-touched (:id orig-base-ref-shape))) orig-ref-id (if swap-slot ;; If there is a swap slot, find the referenced shape id @@ -231,6 +215,7 @@ ;; Get the shape path of the referenced main shape-path (get o-ref-shapes-p-map orig-ref-id) + ;; Get its related shape in the children of new-shape: the one that ;; has the same shape-path related-shape-in-new (get new-shapes-map shape-path) diff --git a/common/src/app/common/test_helpers/variants.cljc b/common/src/app/common/test_helpers/variants.cljc index 4975e7c2a6..a562454c36 100644 --- a/common/src/app/common/test_helpers/variants.cljc +++ b/common/src/app/common/test_helpers/variants.cljc @@ -14,13 +14,14 @@ (defn add-variant [file variant-label component1-label root1-label component2-label root2-label - & {:keys []}] + & {:keys [variant1-params variant2-params] + :or {variant1-params {} variant2-params {}}}] (let [file (ths/add-sample-shape file variant-label :type :frame :is-variant-container true) variant-id (thi/id variant-label)] (-> file - (ths/add-sample-shape root2-label :type :frame :parent-label variant-label :variant-id variant-id :variant-name "Value2") - (ths/add-sample-shape root1-label :type :frame :parent-label variant-label :variant-id variant-id :variant-name "Value1") + (ths/add-sample-shape root2-label (assoc variant2-params :type :frame :parent-label variant-label :variant-id variant-id :variant-name "Value2")) + (ths/add-sample-shape root1-label (assoc variant1-params :type :frame :parent-label variant-label :variant-id variant-id :variant-name "Value1")) (thc/make-component component1-label root1-label) (thc/update-component component1-label {:variant-id variant-id :variant-properties [{:name "Property 1" :value "Value1"}]}) (thc/make-component component2-label root2-label) @@ -42,7 +43,8 @@ (defn add-variant-with-child [file variant-label component1-label root1-label component2-label root2-label child1-label child2-label - & {:keys [child1-params child2-params]}] + & {:keys [child1-params child2-params] + :or {child1-params {} child2-params {}}}] (let [file (ths/add-sample-shape file variant-label :type :frame :is-variant-container true) variant-id (thi/id variant-label)] (-> file diff --git a/common/src/app/common/types/file.cljc b/common/src/app/common/types/file.cljc index 5a2deaabaa..8545931ec6 100644 --- a/common/src/app/common/types/file.cljc +++ b/common/src/app/common/types/file.cljc @@ -32,6 +32,7 @@ [app.common.types.typographies-list :as ctyl] [app.common.types.typography :as cty] [app.common.uuid :as uuid] + [clojure.set :as set] [cuerdas.core :as str])) ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; @@ -1119,3 +1120,29 @@ (defn set-base-font-size [file-data base-font-size] (assoc-in file-data [:options :base-font-size] base-font-size)) + + +;; Ref Chains +(defn get-ref-chain-until-target-ref + "Returns a vector with the shape ref chain until target-ref, including itself" + [container libraries shape target-ref] + (loop [chain [shape] + current shape] + (if (= current target-ref) + chain + (if-let [ref (find-ref-shape nil container libraries current :with-context? true)] + (recur (conj chain ref) ref) + chain)))) + +(defn get-touched-from-ref-chain-until-target-ref + "Returns a set with the :touched of all the items on the shape + ref chain until target-ref, including itself" + [container libraries shape target-ref] + (let [chain (get-ref-chain-until-target-ref container libraries shape target-ref) + more-touched (->> chain + (map :touched) + (remove nil?) + (apply set/union) + (remove ctk/swap-slot?) + set)] + (set/union (or (:touched shape) #{}) more-touched))) diff --git a/common/test/common_tests/logic/variants_switch_test.cljc b/common/test/common_tests/logic/variants_switch_test.cljc index 99a072b540..f271489ae1 100644 --- a/common/test/common_tests/logic/variants_switch_test.cljc +++ b/common/test/common_tests/logic/variants_switch_test.cljc @@ -18,6 +18,29 @@ (t/use-fixtures :each thi/test-fixture) + +(t/deftest test-basic-switch + (let [;; ==== Setup + file (-> (thf/sample-file :file1) + (thv/add-variant + :v01 :c01 :m01 :c02 :m02 + {:variant1-params {:width 5} + :variant2-params {:width 15}}) + + (thc/instantiate-component :c01 + :copy01)) + copy01 (ths/get-shape file :copy01) + + ;; ==== Action + file' (tho/swap-component file copy01 :c02 {:new-shape-label :copy02 :keep-touched? true}) + + copy01' (ths/get-shape file' :copy02)] + (thf/dump-file file :keys [:width]) + ;; The copy had width 5 before the switch + (t/is (= (:width copy01) 5)) + ;; The rect has width 15 after the switch + (t/is (= (:width copy01') 15)))) + (t/deftest test-simple-switch (let [;; ==== Setup file (-> (thf/sample-file :file1) @@ -45,6 +68,41 @@ ;; The rect has width 15 after the switch (t/is (= (:width rect02') 15)))) + +(t/deftest test-basic-switch-override + (let [;; ==== Setup + file (-> (thf/sample-file :file1) + (thv/add-variant + :v01 :c01 :m01 :c02 :m02 + {:variant1-params {:width 5} + :variant2-params {:width 5}}) + + (thc/instantiate-component :c01 + :copy01)) + copy01 (ths/get-shape file :copy01) + + ;; Change width of copy + page (thf/current-page file) + changes (cls/generate-update-shapes (pcb/empty-changes nil (:id page)) + #{(:id copy01)} + (fn [shape] + (assoc shape :width 25)) + (:objects page) + {}) + + file (thf/apply-changes file changes) + copy01 (ths/get-shape file :copy01) + + ;; ==== Action + file' (tho/swap-component file copy01 :c02 {:new-shape-label :copy02 :keep-touched? true}) + + copy01' (ths/get-shape file' :copy02)] + (thf/dump-file file :keys [:width]) + ;; The copy had width 25 before the switch + (t/is (= (:width copy01) 25)) + ;; The override is keept: The copy still has width 25 after the switch + (t/is (= (:width copy01') 25)))) + (t/deftest test-switch-with-override (let [;; ==== Setup file (-> (thf/sample-file :file1) diff --git a/frontend/src/app/main/data/workspace/clipboard.cljs b/frontend/src/app/main/data/workspace/clipboard.cljs index 998ef8cca6..32bb1f92d6 100644 --- a/frontend/src/app/main/data/workspace/clipboard.cljs +++ b/frontend/src/app/main/data/workspace/clipboard.cljs @@ -60,30 +60,6 @@ [promesa.core :as p])) -(defn- get-ref-chain-until-target-ref - "Returns a vector with the shape ref chain until target-ref, including itself" - [container libraries shape target-ref] - (loop [chain [shape] - current shape] - (if (= current target-ref) - chain - (if-let [ref (ctf/find-ref-shape nil container libraries current :with-context? true)] - (recur (conj chain ref) ref) - chain)))) - -(defn- get-touched-from-ref-chain-until-target-ref - "Returns a set with the :touched of all the items on the shape - ref chain until target-ref, including itself" - [container libraries shape target-ref] - (let [chain (when target-ref (get-ref-chain-until-target-ref container libraries shape target-ref)) - more-touched (->> chain - (map :touched) - (remove nil?) - (apply set/union) - (remove ctc/swap-slot?) - set)] - (set/union (or (:touched shape) #{}) more-touched))) - (defn copy-selected [] (letfn [(sort-selected [state data] @@ -191,7 +167,7 @@ (advance-shape [file libraries page level-delta objects shape] (let [new-shape-ref (ctf/advance-shape-ref file page libraries shape level-delta {:include-deleted? true}) container (ctn/make-container page :page) - new-touched (get-touched-from-ref-chain-until-target-ref container libraries shape new-shape-ref)] + new-touched (ctf/get-touched-from-ref-chain-until-target-ref container libraries shape new-shape-ref)] (cond-> objects (and (some? new-shape-ref) (not= new-shape-ref (:shape-ref shape))) (-> (assoc-in [(:id shape) :shape-ref] new-shape-ref)