From fc3f0e406484d1643153c711e03b00c21f045a85 Mon Sep 17 00:00:00 2001 From: "alonso.torres" Date: Thu, 15 Oct 2026 15:11:29 +0200 Subject: [PATCH] :bug: Fix what a variant switch keeps A fix and a check in the variant switch (Phase 3c). - Keep a fixed-sizing size on a switch when both variants agree. A keep-touched variant switch carries a copy's resized size when the old and new variants agree on it (decision (a)). A shape whose :layout-item-h-sizing or :layout-item-v-sizing is :fix took the new variant's size on that axis even when both variants had the same one, so a resize the user made was lost (F61). The fixed axes now follow the same rule as the others: the copy's size when the variants agree, the new variant's when they differ, which is what the tests of a switch between variants of different sizes already expect. variants-switch-test: a fixed-width rect keeps its resized size. - Leave the shape out of child-of-swapped? by id. A keep-touched switch skips the touched shapes below a swapped child, which move with it. child-of-swapped? looked for a swap among the heads between the switched root and the shape, and left the shape out by dropping the last head, which is the shape only when it is a head; below a swapped child, a shape that is not a head saw that child dropped instead and was not skipped (F100). The switch still did the right thing, as such a shape finds nothing to pair with in the new variant. The shape is now left out by its id. The golden master is unchanged. variants-switch-test: an override inside a swapped child survives a keep-touched switch. - Check a switched resized copy text keeps only its geometry. A keep-touched variant switch of a copy whose text was resized, between variants whose texts are measured differently, carries the size and touches only :geometry-group: it adds no :content-group for the position data it leaves to be measured again (F101 does not happen). Benchmark (lower quantile, ms; baseline / now / ratio): deepest-edit-through-three-levels 35.2 / 16.7 / 0.47 edit-through-a-chain-of-dependents 68.9 / 33.1 / 0.48 library-update-into-copies 83.1 / 65.1 / 0.78 main-edit-among-many-shapes 4.8 / 3.6 / 0.73 main-edit-into-10-copies 1.8 / 0.8 / 0.47 main-edit-into-100-copies 15.4 / 8.3 / 0.54 main-edit-into-1000-copies 153.1 / 86.6 / 0.57 variant-switch-into-copies 32.1 / 20.1 / 0.63 AI-assisted-by: claude-opus-5-5 --- common/src/app/common/logic/swap.cljc | 23 ++++----- common/src/app/common/logic/variants.cljc | 17 +++---- .../logic/variants_switch_test.cljc | 49 ++++++++++++++++--- 3 files changed, 59 insertions(+), 30 deletions(-) diff --git a/common/src/app/common/logic/swap.cljc b/common/src/app/common/logic/swap.cljc index f8b422ed7c..6dfd6f5df9 100644 --- a/common/src/app/common/logic/swap.cljc +++ b/common/src/app/common/logic/swap.cljc @@ -94,20 +94,15 @@ current-shape ; The shape after the switch (a clean copy) origin-shape ; The original shape attr] - ;; When there is a layout with fixed h or v sizing, we need - ;; to keep the width/height (and recalculate selrect and points) - (let [size (fn [dimension sizing] - (let [prev (-> prev-shape :selrect dimension) - current (-> current-shape :selrect dimension) - origin (-> origin-shape :selrect dimension)] - (cond - (= :fix (sizing prev-shape)) current - ;; same-size: preserve override - (= origin current) prev - ;; different-size: use new component's - :else current))) - width (size :width :layout-item-h-sizing) - height (size :height :layout-item-v-sizing)] + ;; Each dimension keeps the copy's size when both variants agree on it, + ;; and takes the new variant's otherwise (selrect and points follow) + (let [size (fn [dimension] + (let [prev (-> prev-shape :selrect dimension) + current (-> current-shape :selrect dimension) + origin (-> origin-shape :selrect dimension)] + (if (= origin current) prev current))) + width (size :width) + height (size :height)] (case attr :width width :height height diff --git a/common/src/app/common/logic/variants.cljc b/common/src/app/common/logic/variants.cljc index 1a60f4e7d5..5e13da1241 100644 --- a/common/src/app/common/logic/variants.cljc +++ b/common/src/app/common/logic/variants.cljc @@ -282,17 +282,14 @@ second))) (defn- child-of-swapped? - "Check if any ancestor of a shape (between base-parent-id and shape) was swapped" + "Check if any head between base-parent-id and the shape, both left out, + was swapped" [shape objects base-parent-id] - (let [ancestors (->> (ctn/get-parent-heads objects shape) - ;; Ignore ancestors ahead of base-parent - (drop-while #(not= base-parent-id (:id %))) - seq) - num-ancestors (count ancestors) - ;; Ignore first and last (base-parent and shape) - ancestors (when (and ancestors (<= 3 num-ancestors)) - (subvec (vec ancestors) 1 (dec num-ancestors)))] - (some ctk/swapped? ancestors))) + (->> (ctn/get-parent-heads objects shape) + (drop-while #(not= base-parent-id (:id %))) + (rest) + (remove #(= (:id shape) (:id %))) + (some ctk/swapped?))) (defn- find-shape-ref-child-of "Get the shape referenced by the shape-ref of the near main of the shape, diff --git a/common/test/common_tests/logic/variants_switch_test.cljc b/common/test/common_tests/logic/variants_switch_test.cljc index 4e15560644..5a33c744ca 100644 --- a/common/test/common_tests/logic/variants_switch_test.cljc +++ b/common/test/common_tests/logic/variants_switch_test.cljc @@ -15,7 +15,6 @@ [app.common.logic.shapes :as cls] [app.common.math :as mth] [app.common.test-helpers.components :as thc] - [app.common.test-helpers.composable.core :as tm] [app.common.test-helpers.compositions :as tho] [app.common.test-helpers.files :as thf] [app.common.test-helpers.ids-map :as thi] @@ -3197,11 +3196,8 @@ rect (copy-child file :copy01) file' (tho/swap-component-in-shape file :copy01 :c02 {:new-shape-label :copy02 :keep-touched? true})] (size-is rect 200 100) - ;; the variants agree on the size, so the touched size is carried; a - ;; fixed sizing takes the new variant's width instead - (let [rect' (copy-child file' :copy02)] - (tm/known-failure "F61" [200 100] [100 100] - (mapv #(mth/round (get-in rect' [:selrect %])) [:width :height]))))) + ;; the variants agree on the size, so the touched size is carried + (size-is (copy-child file' :copy02) 200 100))) (t/deftest test-switch-keeps-a-resized-path-of-same-size-variants (let [triangle (fn [x0 y0] @@ -3260,3 +3256,44 @@ (t/is (= "50" (get-in disagree font-size-path-1))) ;; nothing was carried, so nothing is touched (t/is (not (contains? (:touched disagree) :content-group))))) + +(t/deftest test-switch-keeps-an-override-inside-a-swapped-child + ;; both variants nest a copy of :item; the copy swaps it for :other and + ;; fills the rect of :other; a keep-touched switch keeps the swap with its + ;; override + (let [file (-> (thf/sample-file :file1) + (tho/add-simple-component :item :item-root :item-rect) + (tho/add-simple-component :other :other-root :other-rect) + (thv/add-variant :v01 :c01 :m01 :c02 :m02) + (thc/instantiate-component :item :n1 :parent-label :m01) + (thc/instantiate-component :item :n2 :parent-label :m02) + (thc/instantiate-component :c01 :copy01 :children-labels [:copy-n1]) + (tho/swap-component-in-shape :copy-n1 :other {:new-shape-label :swapped})) + rect (first (:shapes (ths/get-shape file :swapped))) + page (thf/current-page file) + file (thf/apply-changes file (cls/generate-update-shapes (pcb/empty-changes nil (:id page)) #{rect} + #(assoc % :opacity 0.5) (:objects page) {})) + file' (tho/swap-component-in-shape file :copy01 :c02 {:new-shape-label :copy02 :keep-touched? true}) + moved (ths/get-shape-by-id file' (thi/id :swapped))] + (t/is (= (thi/id :other) (:component-id moved))) + (t/is (= 0.5 (:opacity (ths/get-shape-by-id file' (first (:shapes moved)))))))) + +(t/deftest test-switch-of-a-resized-text-touches-only-its-geometry + ;; both variants hold a text, measured differently; the copy resizes its + ;; text, then switches keeping its overrides + (let [text (fn [x] + {:type :text + :grow-type :fixed + :content (txt/change-text (:content (ths/sample-shape :tmp :type :text)) "hello") + :position-data [{:x x :y 0 :width 30 :height 14 :fills [] :text "hello"}]}) + file (-> (thf/sample-file :file1) + (thv/add-variant-with-child :v01 :c01 :m01 :c02 :m02 :r01 :r02 + {:child1-params (text 1) :child2-params (text 2)}) + (thc/instantiate-component :c01 :copy01 :children-labels [:copy-r01]) + (resize-copy-child :copy01 2 1)) + ;; the switch moves the labels, so read the copy before it + text (copy-child file :copy01) + file' (tho/swap-component-in-shape file :copy01 :c02 {:new-shape-label :copy02 :keep-touched? true})] + (t/is (= #{:geometry-group} (:touched text))) + (t/is (= #{:geometry-group} (:touched (copy-child file' :copy02)))))) +