Skip to content

Commit d7cc0eb

Browse files
angusbezzinaclaude
andcommitted
fix(marquee): reject a degenerate drag before touching any state
A perfectly axis-aligned drag (dy == 0) clears the gesture layer's travel threshold, so select(inAXRect:) really does receive a zero-height rect in normal use. Rejecting it after the per-selection clears silently stripped an open frame selection's anchor -- re-anchoring a composer mid-typing -- and did so without assigning selected, so the @published change driving the re-render never fired and the overlay kept stale geometry. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 4554f49 commit d7cc0eb

2 files changed

Lines changed: 51 additions & 17 deletions

File tree

Sources/AnnotKit/Overlay/AnnotationSession.swift

Lines changed: 25 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -318,6 +318,31 @@ public final class AnnotationSession: ObservableObject {
318318
@discardableResult
319319
public func select(inAXRect rect: CGRect) -> Element? {
320320
guard mode == .annotating else { return nil }
321+
322+
// Normalize and reject a degenerate rect BEFORE touching any state, so a
323+
// drag that framed nothing is a TRUE no-op.
324+
//
325+
// A zero-width or zero-height rect is a click that never moved, not a
326+
// marquee. Returning nil is a DELIBERATE divergence from "fall back to the
327+
// region path when resolution yields nothing": that fallback is for a drag
328+
// that framed nothing annotatable, whereas this is not a drag at all.
329+
// Falling through would anchor a zero-area frame to whatever sits near the
330+
// press and plant a note the user never asked for — worse than nothing,
331+
// because it looks deliberate. This also bails BEFORE consulting the
332+
// source, so a degenerate rect never reaches `MarqueeTargetRule.resolve`
333+
// or `regionAnchor(at:)`.
334+
//
335+
// Bailing FIRST is load-bearing, not tidiness. A perfectly axis-aligned
336+
// drag (dy == 0) clears the gesture layer's travel threshold and arrives
337+
// here with zero height, so this branch IS reachable in normal use. Clear
338+
// per-selection state before the guard and such a drag would silently strip
339+
// an open frame selection's anchor — re-anchoring a composer the user was
340+
// still typing into, and doing it WITHOUT assigning `selected`, so the
341+
// @Published change that drives the re-render never fires and the overlay
342+
// is left rendering stale geometry.
343+
let normalized = rect.standardized
344+
guard normalized.width > 0, normalized.height > 0 else { return nil }
345+
321346
// A drag on the catcher dismisses an open pin editor for the same reason a
322347
// tap does: the composer is about to take the stage.
323348
editingNoteID = nil
@@ -334,23 +359,6 @@ public final class AnnotationSession: ObservableObject {
334359
// rect keeps one definition of "the centre of what was drawn".
335360
resetNavigation(hint: nil)
336361

337-
// Normalize once: a right-to-left or bottom-to-top drag arrives with
338-
// negative extents.
339-
//
340-
// A zero-width or zero-height rect is a click that never moved, not a
341-
// marquee. Returning nil here is a DELIBERATE divergence from "the caller
342-
// falls back to the region path when resolution yields nothing": that
343-
// fallback is for a drag that framed nothing annotatable, whereas this is
344-
// not a drag at all. Falling through would anchor a zero-area frame to
345-
// whatever happens to sit near the press and plant a note the user never
346-
// asked for — worse than nothing, because it looks deliberate. The gesture
347-
// recognizer routes this case to ``select(atAXPoint:)`` instead (see the
348-
// caller contract above). Note this also bails BEFORE consulting the
349-
// source, so a degenerate rect never reaches `MarqueeTargetRule.resolve`
350-
// or `regionAnchor(at:)`.
351-
let normalized = rect.standardized
352-
guard normalized.width > 0, normalized.height > 0 else { return nil }
353-
354362
if let marqueeSource = source as? MarqueeTargetSource {
355363
let ladder = marqueeSource.marqueeLadder(in: normalized)
356364
if let target = ladder.first {

Tests/AnnotKitTests/AnnotationSessionTests.swift

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -427,6 +427,32 @@ final class AnnotationSessionTests: XCTestCase {
427427
XCTAssertEqual(session.selectedMarqueeRect, CGRect(x: 110, y: 120, width: 40, height: 20))
428428
}
429429

430+
/// A degenerate drag must be a TRUE no-op, not a partial one.
431+
///
432+
/// A perfectly axis-aligned drag (dy == 0) clears the gesture layer's travel
433+
/// threshold, so it really does arrive here with zero height — this is normal
434+
/// use, not a synthetic edge case. Rejecting it AFTER the per-selection clears
435+
/// would silently strip an open frame selection's anchor, re-anchoring a
436+
/// composer the user was still typing into, and would do it without assigning
437+
/// `selected`, so the @Published change that drives the re-render never fires
438+
/// and the overlay keeps rendering the old geometry.
439+
func testDegenerateDragLeavesAnOpenFrameSelectionUntouched() {
440+
let card = makeLadderElement("Card", frame: CGRect(x: 100, y: 100, width: 60, height: 40))
441+
let source = MarqueeSource(ladder: [card])
442+
let session = AnnotationSession(source: source, sink: NotesFileSink(path: "/dev/null"))
443+
session.start()
444+
let drawn = CGRect(x: 110, y: 120, width: 40, height: 20)
445+
session.select(inAXRect: drawn)
446+
XCTAssertEqual(session.selectionAnchorFrame, drawn, "the frame is the anchor before the stray drag")
447+
448+
// A long, perfectly horizontal drag: passes the travel threshold, zero height.
449+
XCTAssertNil(session.select(inAXRect: CGRect(x: 200, y: 300, width: 80, height: 0)))
450+
451+
XCTAssertEqual(session.selected?.id, "Card", "the open selection survives")
452+
XCTAssertEqual(session.selectionAnchorFrame, drawn, "and so does its frame anchor")
453+
XCTAssertEqual(session.selectedMarqueeRect, drawn, "the note's record of the drag is intact")
454+
}
455+
430456
func testMarqueeThenClickDropsTheStaleFrame() {
431457
// The 7993a67 hazard, marquee edition: the catcher stays live behind an
432458
// open composer, so drag-then-click WITHOUT capturing is a supported flow.

0 commit comments

Comments
 (0)