github/penpot
1
0
Fork 0
mirror of https://github.com/penpot/penpot.git synced 2026-08-04 20:09:05 +00:00
Code Issues Packages Projects Releases Wiki Activity

🐛 Position overlays by frame selrect, not filter-inflated bounds (#10454)

Browse Source 
calc-overlay-position measured the destination overlay frame with its full
object bounds (get-object-bounds) while measuring the relative-to frame with
its selrect. Object bounds include padding for shadows, blur, outer strokes
and overflowing children, so centered/right/bottom overlays were shifted by
half that extra padding when the overlay frame had such effects (the overlay
appeared offset, e.g. a bit to the left).

Use the destination frame selrect (the visible frame box) instead, which
matches the sibling helper calc-overlay-pos-initial and the viewer, which
reserves the bounds size and re-aligns the selrect separately. The now unused
geom.shapes.bounds require is removed.

Adds a regression test asserting calc-overlay-position returns the same
position with and without a bounds-inflating drop shadow on the destination
frame.

Fixes #9048

Signed-off-by: Filip Sajdak <filip.sajdak@siili.com>
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Co-authored-by: Andrey Antukh <niwi@niwi.nz>
This commit is contained in:
Filip Sajdak 2026-08-04 16:42:55 +02:00 committed by GitHub
parent 14a6ea5c52
commit 7ae57a035f
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 54 additions and 2 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

9
common/src/app/common/types/shape/interactions.cljc
View File

@ -9,7 +9,6 @@
[app.common.data :as d]
[app.common.files.helpers :as cfh]
[app.common.geom.point :as gpt]
[app.common.geom.shapes.bounds :as gsb]
[app.common.schema :as sm]
[app.common.schema.generators :as sg]))
@ -482,7 +481,13 @@
(if (nil? dest-frame)
[(gpt/point 0 0) [:top :left]]
(let [overlay-size (gsb/get-object-bounds objects dest-frame)
(let [;; Use the destination frame selrect (the visible frame box) to compute
;; the overlay position, not its full object bounds. Bounds include
;; padding for shadows, blur, strokes and overflowing children, which
;; would make centered/right/bottom positions off by half that padding
;; (the visible frame ends up shifted). The viewer reserves the bounds
;; size and re-aligns the selrect separately (see viewer/calculate-delta).
overlay-size (:selrect dest-frame)
base-frame-size (:selrect base-frame)
relative-to-shape-size (:selrect relative-to-shape)
relative-to-adjusted-to-base-frame {:x (- (:x relative-to-shape-size) (:x base-frame-size))

47
common/test/common_tests/types/shape_interactions_test.cljc
View File

@ -10,6 +10,7 @@
[app.common.geom.point :as gpt]
[app.common.geom.rect :as grc]
[app.common.geom.shapes :as gsh]
[app.common.geom.shapes.bounds :as gsb]
[app.common.math :as mth]
[app.common.types.shape :as cts]
[app.common.types.shape.interactions :as ctsi]
@ -1078,3 +1079,49 @@
[overlay-pos snap] (ctsi/calc-overlay-position frame-relative base-frame objects base-frame base-frame overlay-frame frame-offset)]
(t/is (= (gpt/point 18 22) overlay-pos))
(t/is (= [:top :left] snap))))))
(t/deftest calc-overlay-position-ignores-filter-bounds
;; Regression for #9048: the overlay position must be computed from the
;; destination frame selrect (the visible frame box), not from its
;; filter-inflated object bounds. Shadows, blur, strokes or overflowing
;; children make get-object-bounds larger than the selrect, which used to
;; shift centered/right/bottom overlays by half that extra padding (the
;; overlay appeared offset, e.g. "a bit to the left").
(let [base-frame (cts/setup-shape {:type :frame :width 100 :height 100})
overlay-plain (cts/setup-shape {:type :frame :width 30 :height 20})
;; same selrect as overlay-plain, but with a drop shadow that widens
;; and heightens its object bounds well beyond the selrect.
overlay-shadow (-> (cts/setup-shape {:type :frame :width 30 :height 20})
(assoc :shadow [{:style :drop-shadow
:offset-x 0 :offset-y 0
:spread 10 :blur 0 :hidden false}]))
objects {(:id base-frame) base-frame
(:id overlay-plain) overlay-plain
(:id overlay-shadow) overlay-shadow}
frame-offset (gpt/point 5 5)
interaction (-> ctsi/default-interaction
(ctsi/set-action-type :open-overlay)
(ctsi/set-position-relative-to (:id base-frame)))]
;; Precondition: the shadow really does inflate the object bounds, so the
;; assertions below are meaningful (otherwise the test would be vacuous).
(t/is (> (:width (gsb/get-object-bounds objects overlay-shadow))
(:width (:selrect overlay-shadow))))
(t/is (> (:height (gsb/get-object-bounds objects overlay-shadow))
(:height (:selrect overlay-shadow))))
;; For every position type that depends on the overlay size, the computed
;; position must be identical whether or not the destination frame has a
;; bounds-inflating shadow.
(doseq [pos-type [:center :top-center :top-right :bottom-center :bottom-right]]
(let [i-plain (-> interaction
(ctsi/set-destination (:id overlay-plain))
(ctsi/set-overlay-pos-type pos-type base-frame objects))
i-shadow (-> interaction
(ctsi/set-destination (:id overlay-shadow))
(ctsi/set-overlay-pos-type pos-type base-frame objects))
[pos-plain snap-plain] (ctsi/calc-overlay-position i-plain base-frame objects base-frame base-frame overlay-plain frame-offset)
[pos-shadow snap-shadow] (ctsi/calc-overlay-position i-shadow base-frame objects base-frame base-frame overlay-shadow frame-offset)]
(t/testing (str "overlay position ignores filter bounds for " pos-type)
(t/is (= pos-plain pos-shadow))
(t/is (= snap-plain snap-shadow)))))))