From 892869b03926aa2ad5f796243eda87a0ef5e5f33 Mon Sep 17 00:00:00 2001 From: "alonso.torres" Date: Fri, 29 May 2026 00:35:49 +0200 Subject: [PATCH] :bug: Fix stroke caps misplaced when adding node in middle of path --- .../common_tests/types/path_data_test.cljc | 29 +++++++++++++ .../main/ui/workspace/shapes/path/editor.cljs | 41 ++++++++++--------- 2 files changed, 51 insertions(+), 19 deletions(-) diff --git a/common/test/common_tests/types/path_data_test.cljc b/common/test/common_tests/types/path_data_test.cljc index d3cca9da07..f56d61a539 100644 --- a/common/test/common_tests/types/path_data_test.cljc +++ b/common/test/common_tests/types/path_data_test.cljc @@ -999,6 +999,35 @@ ;; result should have more segments than original (splits added) (t/is (> (count result) (count sample-content-square))))) +;; Regression test for issue #8301: clicking a hover point on a path segment +;; must insert exactly one node. The bug caused two actions to fire together — +;; create-node-at-position (correct) and add-node (extra) — so the midpoint M +;; was appended again as a tail endpoint, giving [M A L M L B L M] instead of +;; the correct [M A L M L B]. The stroke markerEnd then landed in the middle. +;; The invariant: split-segments on a 2-command open path produces exactly 3 +;; commands and does not repeat the inserted midpoint at the tail. +(t/deftest segment-split-segments-no-duplicate-endpoint + (let [;; Simple open path: A=(0,0) → B=(100,0) + segments [{:command :move-to :params {:x 0.0 :y 0.0}} + {:command :line-to :params {:x 100.0 :y 0.0}}] + from-p (gpt/point 0.0 0.0) + to-p (gpt/point 100.0 0.0) + content (path/content segments) + result (path.segment/split-segments content #{from-p to-p} 0.5) + cmds (mapv :command result)] + + ;; Must be exactly 3 commands: move-to A, line-to M, line-to B + (t/is (= 3 (count result))) + (t/is (= [:move-to :line-to :line-to] cmds)) + + ;; Midpoint M must be at (50, 0) + (t/is (mth/close? 50.0 (get-in result [1 :params :x]))) + (t/is (mth/close? 0.0 (get-in result [1 :params :y]))) + + ;; Original endpoint B must still be at (100, 0) — not shadowed by a duplicate M + (t/is (mth/close? 100.0 (get-in result [2 :params :x]))) + (t/is (mth/close? 0.0 (get-in result [2 :params :y]))))) + (t/deftest segment-content->selrect (let [content (path/content sample-content-square) rect (path.segment/content->selrect content)] diff --git a/frontend/src/app/main/ui/workspace/shapes/path/editor.cljs b/frontend/src/app/main/ui/workspace/shapes/path/editor.cljs index 62fc3dc357..34b00faf93 100644 --- a/frontend/src/app/main/ui/workspace/shapes/path/editor.cljs +++ b/frontend/src/app/main/ui/workspace/shapes/path/editor.cljs @@ -68,31 +68,34 @@ (dom/stop-propagation event) (dom/prevent-default event) + ;; When clicking on a hover point that lies on a segment (has metadata with + ;; split params), only insert the node — don't also run draw-mode actions which + ;; would add the same position as an extra endpoint, corrupting the path order + ;; and misplacing stroke caps. ;; FIXME: revisit this, using meta here breaks equality checks - (when (and is-new (some? (meta position))) - (st/emit! (drp/create-node-at-position (meta position)))) + (if (and is-new (some? (meta position))) + (st/emit! (drp/create-node-at-position (meta position))) + (let [is-shift (kbd/shift? event) + is-mod (kbd/mod? event)] + (cond + is-last + (st/emit! (drp/reset-last-handler)) - (let [is-shift (kbd/shift? event) - is-mod (kbd/mod? event)] - (cond - is-last - (st/emit! (drp/reset-last-handler)) + (and is-move is-mod (not is-curve)) + (st/emit! (drp/make-curve position)) - (and is-move is-mod (not is-curve)) - (st/emit! (drp/make-curve position)) + (and is-move is-mod is-curve) + (st/emit! (drp/make-corner position)) - (and is-move is-mod is-curve) - (st/emit! (drp/make-corner position)) + is-move + ;; If we're dragging a selected item we don't change the selection + (st/emit! (drp/start-move-path-point position is-shift)) - is-move - ;; If we're dragging a selected item we don't change the selection - (st/emit! (drp/start-move-path-point position is-shift)) + (and is-draw is-start-path) + (st/emit! (drp/start-path-from-point position)) - (and is-draw is-start-path) - (st/emit! (drp/start-path-from-point position)) - - (and is-draw (not is-start-path)) - (st/emit! (drp/close-path-drag-start position))))))] + (and is-draw (not is-start-path)) + (st/emit! (drp/close-path-drag-start position)))))))] [:g.path-point [:circle.path-point