Skip to content

Commit 653b8f6

Browse files
angusbezzinaclaude
andcommitted
fix(cards): Shift+Enter inserts a line break instead of eating the text
The card promises "⏎ save · ⇧⏎ newline" in its own header and the second half was not true. It returned `.ignored` for Shift+Return and trusted the field to do it; a vertical TextField on macOS has no newline gesture, so the key reached AppKit as an ordinary `insertNewline:`, which ends editing. Measured rather than reasoned about, and the failure is worse than the report: typing "a", Shift+Return, "b" left the binding holding "b" — no line break, and the text already typed was GONE. A field with no key handler at all behaved identically, so the interception was never the cause. `insertNewlineIgnoringFieldEditor(_:)` sent to the field editor — the NSTextView that actually holds the text while a SwiftUI TextField is focused — is AppKit's action for "a line break, do NOT end editing", what Option+Return does in any NSTextField. Going through the editor rather than appending to the binding is what lands the break at the CARET, so someone fixing the middle of a sentence gets it where they are typing. Rejected TextEditor, which does honour Shift+Return: it would have meant re-creating the rounded border, the 2-5 line growth and the placeholder by hand — restyling the one control on both cards to fix a keystroke. Probe phase 13 asserts it end to end, through the composer's real field: type "a", Shift+Return, "b", Enter, and read what was FILED, since the draft is @State in the view and there is no honest way to read it from outside. Rides the same opt-in flag as the other real-input legs. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 92a83fd commit 653b8f6

3 files changed

Lines changed: 153 additions & 7 deletions

File tree

DECISIONS.md

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -296,6 +296,30 @@ a mode the user has deliberately entered, so a press on the catcher is always me
296296
for the catcher. The alternative — making the panel unable to become key — would
297297
take the composer's typing with it.
298298

299+
### Shift+Enter inserts the line break by hand (dogfooding)
300+
301+
The card promises `⏎ save · ⇧⏎ newline` in its own header, and the second half was
302+
not true. It used to `return .ignored` for Shift+Return and trust the field. A
303+
vertical `TextField` on macOS has no newline gesture: the key arrives at AppKit as
304+
an ordinary `insertNewline:`, which ENDS EDITING. Measured — typing "a",
305+
Shift+Return, "b" left the binding holding **"b"**: no break, and the text already
306+
typed was gone. A field with no key handler at all behaved identically, so the
307+
interception was never the cause.
308+
309+
`insertNewlineIgnoringFieldEditor(_:)` sent to the FIELD EDITOR — the `NSTextView`
310+
that actually holds the text while a SwiftUI `TextField` is focused — is AppKit's
311+
action for "a line break, do NOT end editing" (what Option+Return does in any
312+
`NSTextField`). Going through the editor rather than appending to the binding is
313+
what puts the break at the CARET, so someone fixing the middle of a sentence gets
314+
it where they are typing instead of stapled to the end.
315+
316+
Rejected: swapping the field for a `TextEditor`, which does honour Shift+Return.
317+
It would have meant re-creating the rounded border, the 2–5 line growth and the
318+
placeholder by hand — restyling the one control on both cards to fix a keystroke.
319+
320+
macOS-only by necessity; on iOS `.ignored` is correct, since UIKit's multiline
321+
field inserts the break itself and a touch keyboard has no Shift+Return.
322+
299323
### The live selection follows the content on scroll, not just the notes
300324

301325
"The manually drawn frames disappear on scroll." They do not disappear, they

Sources/AnnotKit/Overlay/OverlayView.swift

Lines changed: 38 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,7 @@
11
import SwiftUI
2+
#if os(macOS)
3+
import AppKit
4+
#endif
25

36
/// The overlay's SwiftUI content, shared by the macOS and iOS hosts.
47
///
@@ -707,10 +710,10 @@ private struct AnnotationCard<Footer: View>: View {
707710
.lineLimit(2 ... 5)
708711
.frame(width: 260)
709712
.focused($focused)
710-
// Enter submits; Shift+Enter falls through to insert a newline in
711-
// the multiline field.
713+
// Enter submits; Shift+Enter inserts a line break (see
714+
// ``insertLineBreak()`` for why that needs doing by hand).
712715
.onKeyPress(keys: [.return]) { key in
713-
if key.modifiers.contains(.shift) { return .ignored }
716+
guard !key.modifiers.contains(.shift) else { return insertLineBreak() }
714717
onSubmit()
715718
return .handled
716719
}
@@ -750,6 +753,38 @@ private struct AnnotationCard<Footer: View>: View {
750753
// acting again on the state that leaves behind.
751754
}
752755

756+
/// Insert a line break at the caret for Shift+Enter, and keep editing.
757+
///
758+
/// This used to `return .ignored` and trust the field to do it. It does not —
759+
/// and the failure is not merely inert, which is why it was worth measuring
760+
/// rather than reasoning about. On a `TextField(axis: .vertical)`, typing "a",
761+
/// Shift+Return, "b" leaves the binding holding **"b"**: no line break, and the
762+
/// text that was already typed is GONE. A field with no key handler at all
763+
/// behaves identically, so the interception was never the problem — a vertical
764+
/// TextField on macOS simply has no newline gesture, and Shift+Return reaches
765+
/// AppKit as an ordinary `insertNewline:`, which ends editing.
766+
///
767+
/// `insertNewlineIgnoringFieldEditor(_:)` is the action AppKit provides for
768+
/// exactly this — "a line break, do NOT end editing", what Option+Return does in
769+
/// any `NSTextField` — sent to the FIELD EDITOR, the `NSTextView` that actually
770+
/// holds the text while a SwiftUI `TextField` is focused. Going through it
771+
/// rather than appending to the binding is what makes the break land at the
772+
/// CARET: a user fixing the middle of a sentence gets it where they are typing,
773+
/// not stapled to the end.
774+
///
775+
/// macOS-only by necessity (`NSTextView`, `NSApp`). On iOS the key is
776+
/// `.ignored`, which is the right answer there: UIKit's multiline field inserts
777+
/// the break itself, and on a touch keyboard there is no Shift+Return to press.
778+
private func insertLineBreak() -> KeyPress.Result {
779+
#if os(macOS)
780+
guard let editor = NSApp.keyWindow?.firstResponder as? NSTextView else { return .ignored }
781+
editor.insertNewlineIgnoringFieldEditor(nil)
782+
return .handled
783+
#else
784+
return .ignored
785+
#endif
786+
}
787+
753788
/// Focus the text field, making the host panel key FIRST so the non-activating
754789
/// child window accepts keystrokes. A `@FocusState` set in the same layout pass
755790
/// that inserts the field can be dropped on first appearance (the classic

Sources/AnnotKitOverlayProbe/main.swift

Lines changed: 91 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3057,11 +3057,11 @@ final class OverlayProbeDelegate: NSObject, NSApplicationDelegate {
30573057

30583058
guard realClicks else {
30593059
print(" (end-to-end real-click legs skipped — set ANNOTKIT_PROBE_REALCLICK=1)")
3060-
controller.unmount(); host.window.orderOut(nil); return finish()
3060+
controller.unmount(); host.window.orderOut(nil); return phase13Keyboard()
30613061
}
30623062
guard AXIsProcessTrusted() else {
30633063
check12(false, "posting real clicks needs Accessibility trust — the end-to-end legs CANNOT run")
3064-
controller.unmount(); host.window.orderOut(nil); return finish()
3064+
controller.unmount(); host.window.orderOut(nil); return phase13Keyboard()
30653065
}
30663066
host.window.setFrameOrigin(NSPoint(x: visibleFrame.minX + 140, y: visibleFrame.minY + 240))
30673067
NSApp.activate(ignoringOtherApps: true)
@@ -3075,7 +3075,7 @@ final class OverlayProbeDelegate: NSObject, NSApplicationDelegate {
30753075
"ONE click on the catcher selects — the first press is not spent making the panel key")
30763076
controller.unmount()
30773077
host.window.orderOut(nil)
3078-
self.finish()
3078+
self.phase13Keyboard()
30793079
}
30803080
])
30813081
}
@@ -3128,6 +3128,92 @@ final class OverlayProbeDelegate: NSObject, NSApplicationDelegate {
31283128
}
31293129
}
31303130

3131+
// ---- Phase 13: the card's keyboard contract ----------------------------
3132+
// The card promises "⏎ save · ⇧⏎ newline" in its own header, and the second
3133+
// half of that was not true: Shift+Return inserted nothing AND discarded what
3134+
// had already been typed, because a vertical `TextField` has no newline gesture
3135+
// on macOS and the key reached AppKit as an ordinary `insertNewline:`.
3136+
//
3137+
// Asserted END TO END, through the composer's real text field, because that is
3138+
// the only place the claim is meaningful: the draft is `@State` in the view, so
3139+
// the only honest way to read it is to make the note and look at what was
3140+
// filed. Real key events, so it rides the same opt-in flag as the click legs.
3141+
var passKeyboard = true
3142+
func check13(_ c: Bool, _ m: String) {
3143+
print(" " + (c ? "ok " : "FAIL ") + m)
3144+
passKeyboard = passKeyboard && c
3145+
}
3146+
3147+
var keyboardController: OverlayController?
3148+
3149+
func phase13Keyboard() {
3150+
print("\n--- Phase 13: the card's keyboard contract (⏎ saves, ⇧⏎ inserts a line break) ---")
3151+
guard realClicks else {
3152+
print(" (needs real key events — skipped; set ANNOTKIT_PROBE_REALCLICK=1)")
3153+
return finish()
3154+
}
3155+
guard AXIsProcessTrusted() else {
3156+
check13(false, "posting real key events needs Accessibility trust")
3157+
return finish()
3158+
}
3159+
let host = makeHostWindow(title: "AnnotKit Harness W13 (keyboard)")
3160+
host.window.setFrameOrigin(NSPoint(x: visibleFrame.minX + 160, y: visibleFrame.minY + 260))
3161+
NSApp.activate(ignoringOtherApps: true)
3162+
host.window.makeKeyAndOrderFront(nil)
3163+
3164+
let session = AnnotationSession(source: MacElementSource(), sink: NotesFileSink(path: "/dev/null"))
3165+
let controller = OverlayController(session: session)
3166+
controller.mount(on: host.window)
3167+
controller.start()
3168+
keyboardController = controller
3169+
3170+
runSteps([
3171+
{
3172+
// Open the composer on a real control. The card focuses itself and
3173+
// makes the panel key, which is what puts the field editor in the
3174+
// responder chain the keystrokes will reach.
3175+
NSApp.activate(ignoringOtherApps: true)
3176+
session.select(atAXPoint: axCenter(of: host.primary))
3177+
self.check13(session.selected != nil, "sanity: the composer is open on a real element")
3178+
},
3179+
{
3180+
// "a", Shift+Return, "b" — the sequence that used to leave "b".
3181+
self.typeKey(0)
3182+
self.typeKey(36, shift: true)
3183+
self.typeKey(11)
3184+
},
3185+
{
3186+
// Enter commits, which is the only way to read a draft that lives
3187+
// in the view's own @State.
3188+
self.typeKey(36)
3189+
},
3190+
{
3191+
let comment = session.pending.last?.comment
3192+
print(" filed comment = \(comment.map { "\"\($0.replacingOccurrences(of: "\n", with: "\\n"))\"" } ?? "nil")")
3193+
self.check13(session.pending.count == 1, "⏎ filed the note (got \(session.pending.count))")
3194+
self.check13(comment == "a\nb",
3195+
"⇧⏎ inserted a LINE BREAK and kept what was already typed (the old behaviour filed \"b\")")
3196+
controller.unmount()
3197+
host.window.orderOut(nil)
3198+
self.finish()
3199+
}
3200+
])
3201+
}
3202+
3203+
/// One real keystroke. Virtual key codes are hardware positions, so they are
3204+
/// layout-independent: 0 = "a", 11 = "b", 36 = Return.
3205+
func typeKey(_ code: CGKeyCode, shift: Bool = false) {
3206+
let source = CGEventSource(stateID: .hidSystemState)
3207+
guard let down = CGEvent(keyboardEventSource: source, virtualKey: code, keyDown: true),
3208+
let up = CGEvent(keyboardEventSource: source, virtualKey: code, keyDown: false) else { return }
3209+
if shift {
3210+
down.flags = .maskShift
3211+
up.flags = .maskShift
3212+
}
3213+
down.post(tap: .cghidEventTap)
3214+
up.post(tap: .cghidEventTap)
3215+
}
3216+
31313217
func finish() {
31323218
print("\n issue-2 (per-control hit-test through the expanded overlay): \(passIssue2 ? "PASS" : "FAIL")")
31333219
print(" issue-1 (retention / copy / export / pill persistence): \(pass1 ? "PASS" : "FAIL")")
@@ -3142,8 +3228,9 @@ final class OverlayProbeDelegate: NSObject, NSApplicationDelegate {
31423228
print(" Phase 10 (Escape closes the menu): \(passEscape ? "PASS" : "FAIL")")
31433229
print(" Phase 11 (recallable marks: toolbar pass-through, pins inert in frame mode, hover recall): \(passMarks ? "PASS" : "FAIL")")
31443230
print(" Phase 12 (the FIRST click on the overlay acts — no click tax): \(passFirstClick ? "PASS" : "FAIL")")
3231+
print(" Phase 13 (the card's keyboard contract: ⏎ saves, ⇧⏎ newlines): \(passKeyboard ? "PASS" : "FAIL")")
31453232
print("\n=== AnnotKitOverlayProbe complete ===")
3146-
exit(pass1 && passIssue2 && passPins && passResize && passChrome && passCard && passSpec && passMarquee && passNav && passClamp && passEscape && passMarks && passFirstClick ? 0 : 1)
3233+
exit(pass1 && passIssue2 && passPins && passResize && passChrome && passCard && passSpec && passMarquee && passNav && passClamp && passEscape && passMarks && passFirstClick && passKeyboard ? 0 : 1)
31473234
}
31483235

31493236
func collectIDs(_ elements: [Element]) -> [String] {

0 commit comments

Comments
 (0)