Skip to content

Commit 7993a67

Browse files
angusbezzinaclaude
andcommitted
fix(session): a region -> element re-selection must drop the stale region offset
select() assigned the new hit but the didSet only clears selectedRegionOffset when selected becomes NIL - the catcher stays active behind an open composer, so a dead click (region) followed by a click on a real element left the region's offset to leak onto the ELEMENT note (bogus **Region** line in the export). Reset the offset at the top of select(); regression test covers the transition. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 3b85756 commit 7993a67

2 files changed

Lines changed: 39 additions & 0 deletions

File tree

Sources/AnnotKit/Overlay/AnnotationSession.swift

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,11 @@ public final class AnnotationSession: ObservableObject {
104104
// Any catcher tap dismisses an open pin editor: a tap on empty space is a
105105
// click-away close, and a tap on an element hands the stage to the composer.
106106
editingNoteID = nil
107+
// Every selection starts offset-free: a region -> element re-selection
108+
// (the catcher stays active behind an open composer) must not leak the
109+
// previous region's offset onto an ELEMENT note — the didSet only
110+
// clears the offset when `selected` becomes nil, not on replacement.
111+
selectedRegionOffset = nil
107112
selected = source.hitTest(point)
108113
if selected == nil,
109114
let anchorSource = source as? RegionAnchorSource,

Tests/AnnotKitTests/AnnotationSessionTests.swift

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,22 @@ private final class StubSource: ElementSource {
1616
}
1717
}
1818

19+
/// A source whose hit-test result can be swapped mid-test: nil first (region
20+
/// path), then a real element (re-selection path).
21+
@MainActor
22+
private final class SwitchableSource: ElementSource, RegionAnchorSource {
23+
let anchor: Element
24+
var hit: Element?
25+
init(anchor: Element) { self.anchor = anchor }
26+
func snapshot() -> [WindowSnapshot] { [] }
27+
func hitTest(_ point: CGPoint) -> Element? { hit }
28+
func regionAnchor(at point: CGPoint) -> Element? { anchor }
29+
func selector(for element: Element) -> String { "#\(element.id)" }
30+
func screenshot(of element: Element?) async throws -> CapturedImage {
31+
CapturedImage(pngData: Data(), pixelWidth: 1, pixelHeight: 1)
32+
}
33+
}
34+
1935
/// A source with NO element anywhere but a fixed nearby anchor, driving the
2036
/// region-fallback path.
2137
@MainActor
@@ -72,6 +88,24 @@ final class AnnotationSessionTests: XCTestCase {
7288
XCTAssertNil(note?.regionOffset, "ordinary element notes have no region")
7389
}
7490

91+
func testRegionThenElementReselectionDropsTheStaleOffset() {
92+
// The catcher stays active behind an open composer, so region -> element
93+
// re-selection without cancelling is a supported flow; the element note
94+
// must NOT inherit the region's offset.
95+
let anchor = makeElement()
96+
let source = SwitchableSource(anchor: anchor)
97+
let session = AnnotationSession(source: source, sink: NotesFileSink(path: "/dev/null"))
98+
session.start()
99+
session.select(atAXPoint: CGPoint(x: 22, y: 14))
100+
XCTAssertEqual(session.selected?.role, "AXRegion", "first click lands the region")
101+
source.hit = anchor
102+
session.select(atAXPoint: .zero)
103+
XCTAssertEqual(session.selected?.role, "AXButton", "second click re-selects a real element")
104+
XCTAssertNil(session.selectedRegionOffset, "the region offset must not survive re-selection")
105+
let note = session.addNote(comment: "element after region")
106+
XCTAssertNil(note?.regionOffset, "element note must not inherit the stale region offset")
107+
}
108+
75109
func testClearHoverDropsHighlightButKeepsSelection() {
76110
let session = AnnotationSession(source: StubSource(makeElement()), sink: NotesFileSink(path: "/dev/null"))
77111
session.start()

0 commit comments

Comments
 (0)