mirror of
https://github.com/penpot/penpot.git
synced 2026-10-29 05:50:35 +00:00
🐛 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
This commit is contained in:
parent
a5d2ae277a
commit
fc3f0e4064
@ -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
|
||||
|
||||
@ -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,
|
||||
|
||||
@ -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))))))
|
||||
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user