mirror of
https://github.com/penpot/penpot.git
synced 2026-09-14 07:59:36 +00:00
🐛 Anchor variant switch geometry to target
Preserve real size overrides during variant switches without copying stale absolute composite geometry from the source variant. Signed-off-by: Codex <codex@openai.com>
This commit is contained in:
parent
d249fd106a
commit
744c1b98c0
@ -2101,6 +2101,39 @@
|
|||||||
(grc/rect->center selrect)
|
(grc/rect->center selrect)
|
||||||
(or (:transform current-shape) (gmt/matrix)))))))
|
(or (:transform current-shape) (gmt/matrix)))))))
|
||||||
|
|
||||||
|
|
||||||
|
(defn- switch-geom-change-value
|
||||||
|
[prev-shape current-shape attr]
|
||||||
|
;; Composite geometry stores absolute coordinates. When preserving a size
|
||||||
|
;; override across variants, keep the target variant's position and only carry
|
||||||
|
;; the previous dimensions; otherwise :x/:y can disagree with :selrect/:points.
|
||||||
|
(let [prev-selrect (:selrect prev-shape)
|
||||||
|
current-selrect (:selrect current-shape)
|
||||||
|
final-width (:width prev-selrect)
|
||||||
|
final-height (:height prev-selrect)
|
||||||
|
x (:x current-selrect)
|
||||||
|
y (:y current-selrect)
|
||||||
|
selrect (assoc current-selrect
|
||||||
|
:width final-width
|
||||||
|
:height final-height
|
||||||
|
:x x
|
||||||
|
:y y
|
||||||
|
:x1 x
|
||||||
|
:y1 y
|
||||||
|
:x2 (+ x final-width)
|
||||||
|
:y2 (+ y final-height))]
|
||||||
|
(case attr
|
||||||
|
:selrect
|
||||||
|
selrect
|
||||||
|
|
||||||
|
:points
|
||||||
|
(-> selrect
|
||||||
|
(grc/rect->points)
|
||||||
|
(gsh/transform-points
|
||||||
|
(grc/rect->center selrect)
|
||||||
|
(or (:transform current-shape) (gmt/matrix)))))))
|
||||||
|
|
||||||
|
|
||||||
(defn- equal-geometry?
|
(defn- equal-geometry?
|
||||||
"Returns true when the value of `attr` in `shape` is considered equal
|
"Returns true when the value of `attr` in `shape` is considered equal
|
||||||
to the corresponding value in `origin-shape`, ignoring positional
|
to the corresponding value in `origin-shape`, ignoring positional
|
||||||
@ -2270,6 +2303,10 @@
|
|||||||
(contains? #{:points :selrect :width :height} attr))
|
(contains? #{:points :selrect :width :height} attr))
|
||||||
(switch-fixed-layout-geom-change-value previous-shape current-shape origin-ref-shape attr)
|
(switch-fixed-layout-geom-change-value previous-shape current-shape origin-ref-shape attr)
|
||||||
|
|
||||||
|
(and (contains? #{:points :selrect} attr)
|
||||||
|
(not path-change?))
|
||||||
|
(switch-geom-change-value previous-shape current-shape attr)
|
||||||
|
|
||||||
:else
|
:else
|
||||||
(get previous-shape attr)))
|
(get previous-shape attr)))
|
||||||
|
|
||||||
|
|||||||
@ -2866,18 +2866,14 @@
|
|||||||
(t/is (= (get-in rect02' [:selrect :width]) 150))))
|
(t/is (= (get-in rect02' [:selrect :width]) 150))))
|
||||||
|
|
||||||
|
|
||||||
(t/deftest test-switch-when-source-master-child-has-touched-geometry
|
(t/deftest test-switch-skips-composite-geometry-with-subpixel-drift
|
||||||
;; Regression: when the previous-shape's geometry has sub-pixel drift
|
;; Regression: when the previous-shape's geometry only has sub-pixel drift
|
||||||
;; relative to its source master (a state produced by interactive transform
|
;; relative to its source master (a state produced by interactive transform
|
||||||
;; modifiers, e.g. alt-drag duplicate of a variant whose children are
|
;; modifiers, e.g. alt-drag duplicate of a variant whose children are
|
||||||
;; component copies), the equal-geometry? guard in update-attrs-on-switch
|
;; component copies), equal-geometry? must classify it as unchanged and skip
|
||||||
;; uses exact equality and fails. The :else branch then copies
|
;; copying composite geometry. Otherwise, :selrect/:points can carry stale
|
||||||
;; previous-shape's :selrect verbatim onto the freshly-instantiated target,
|
;; absolute positions from the source variant onto the freshly-instantiated
|
||||||
;; leaving :y correct (the per-attr y skip catches that) but :selrect.y
|
;; target, producing the visible "cut off" symptom.
|
||||||
;; stale. The shape ends up internally inconsistent (:y disagrees with
|
|
||||||
;; :selrect.y); the renderer reads :selrect, so the child appears at the
|
|
||||||
;; source variant's position inside a parent that has resized to the
|
|
||||||
;; target's dimensions — the visible "cut off" symptom.
|
|
||||||
(let [;; ==== Setup
|
(let [;; ==== Setup
|
||||||
;; A self-contained Input/Button-like component, plus a variant
|
;; A self-contained Input/Button-like component, plus a variant
|
||||||
;; container whose two variants each instance that component
|
;; container whose two variants each instance that component
|
||||||
@ -2911,8 +2907,8 @@
|
|||||||
;; The copy carries an Input/Button instance (Frame1). Introduce
|
;; The copy carries an Input/Button instance (Frame1). Introduce
|
||||||
;; sub-pixel drift in its :width and :selrect.width — the kind of
|
;; sub-pixel drift in its :width and :selrect.width — the kind of
|
||||||
;; floating-point error produced by the alt-drag modifier path in
|
;; floating-point error produced by the alt-drag modifier path in
|
||||||
;; production. This drift is what defeats equal-geometry?'s
|
;; production. The drift is small enough to be treated as unchanged
|
||||||
;; exact-equality comparison and lets the bug surface.
|
;; geometry by equal-geometry?.
|
||||||
page (thf/current-page file)
|
page (thf/current-page file)
|
||||||
copy01 (ths/get-shape file :copy01)
|
copy01 (ths/get-shape file :copy01)
|
||||||
copy-btn-id (->> (cfh/get-children-ids-with-self (:objects page) (:id copy01))
|
copy-btn-id (->> (cfh/get-children-ids-with-self (:objects page) (:id copy01))
|
||||||
@ -3035,3 +3031,73 @@
|
|||||||
(t/is (= target-rel-y actual-rel-y)
|
(t/is (= target-rel-y actual-rel-y)
|
||||||
(str "path :selrect.y should match target master layout (expected "
|
(str "path :selrect.y should match target master layout (expected "
|
||||||
target-rel-y " got " actual-rel-y ")"))))
|
target-rel-y " got " actual-rel-y ")"))))
|
||||||
|
|
||||||
|
|
||||||
|
(t/deftest test-switch-preserves-size-override-at-target-position
|
||||||
|
(let [move-to (fn [shape x y]
|
||||||
|
(gsh/move shape (gpt/point (- x (:x shape))
|
||||||
|
(- y (:y shape)))))
|
||||||
|
|
||||||
|
;; ==== Setup: each variant contains the same nested component instance.
|
||||||
|
;; The nested instance has identical size in both variants, but a different
|
||||||
|
;; position relative to the variant root.
|
||||||
|
file (-> (thf/sample-file :file1)
|
||||||
|
(tho/add-simple-component
|
||||||
|
:nested-component :nested-main :nested-label
|
||||||
|
:root-params {:width 100 :height 50}
|
||||||
|
:child-params {:width 30 :height 10})
|
||||||
|
(thv/add-variant-with-copy
|
||||||
|
:v01 :c01 :m01 :c02 :m02 :r01 :r02 :nested-component))
|
||||||
|
|
||||||
|
page (thf/current-page file)
|
||||||
|
r01 (ths/get-shape file :r01)
|
||||||
|
r02 (ths/get-shape file :r02)
|
||||||
|
changes (cls/generate-update-shapes (pcb/empty-changes nil (:id page))
|
||||||
|
#{(:id r01) (:id r02)}
|
||||||
|
(fn [shape]
|
||||||
|
(cond
|
||||||
|
(= (:id shape) (:id r01)) (move-to shape 20 100)
|
||||||
|
(= (:id shape) (:id r02)) (move-to shape 20 70)
|
||||||
|
:else shape))
|
||||||
|
(:objects page)
|
||||||
|
{})
|
||||||
|
file (thf/apply-changes file changes)
|
||||||
|
|
||||||
|
file (thc/instantiate-component file :c01
|
||||||
|
:copy01
|
||||||
|
:children-labels [:copy-r01])
|
||||||
|
page (thf/current-page file)
|
||||||
|
copy01 (ths/get-shape file :copy01)
|
||||||
|
copy-r01 (get-in page [:objects (-> copy01 :shapes first)])
|
||||||
|
|
||||||
|
;; This is a real geometry override, not float drift. The switch should
|
||||||
|
;; preserve the overridden size while anchoring composite geometry to
|
||||||
|
;; the target variant's position.
|
||||||
|
changes (cls/generate-update-shapes (pcb/empty-changes nil (:id page))
|
||||||
|
#{(:id copy-r01)}
|
||||||
|
(fn [shape]
|
||||||
|
(let [new-width 150
|
||||||
|
sr (:selrect shape)
|
||||||
|
new-sr (-> sr
|
||||||
|
(assoc :width new-width)
|
||||||
|
(assoc :x2 (+ (:x1 sr) new-width)))]
|
||||||
|
(-> shape
|
||||||
|
(assoc :width new-width)
|
||||||
|
(assoc :selrect new-sr)
|
||||||
|
(assoc :touched #{:geometry-group}))))
|
||||||
|
(:objects page)
|
||||||
|
{})
|
||||||
|
file (thf/apply-changes file changes)
|
||||||
|
|
||||||
|
;; ==== Action
|
||||||
|
file' (tho/swap-component-in-shape file :copy01 :c02 {:new-shape-label :copy02 :keep-touched? true})
|
||||||
|
|
||||||
|
page' (thf/current-page file')
|
||||||
|
copy02' (ths/get-shape file' :copy02)
|
||||||
|
rect02' (get-in page' [:objects (-> copy02' :shapes first)])]
|
||||||
|
|
||||||
|
;; The width override is preserved, but the target variant position remains
|
||||||
|
;; authoritative for absolute composite geometry.
|
||||||
|
(t/is (= 150 (:width rect02')))
|
||||||
|
(t/is (= (+ (:y copy02') 70) (:y rect02')))
|
||||||
|
(t/is (= (:y rect02') (get-in rect02' [:selrect :y])))))
|
||||||
|
|||||||
Loading…
x
Reference in New Issue
Block a user