fix(desktop): support IME composition in chat inputs#7539
Conversation
Greptile SummaryThis PR fixes IME (Input Method Editor) composition support in the macOS desktop chat inputs so that typing CJK characters via candidate selection no longer fights with SwiftUI's text bindings or accidentally submits mid-composition on Return.
Confidence Score: 4/5Safe to merge for the primary keyboard-Return IME fix; a secondary path (clicking the send arrow button mid-composition) is still unguarded and can produce an inconsistent input state. The core logic preventing SwiftUI from overwriting the composing buffer and blocking Return from submitting during composition is correctly implemented and well-structured. The remaining gap is that the send button action and disabled predicate in both ChatInputView and AskAIInputView do not consult hasMarkedText, so a mouse click mid-composition sends intermediate text and leaves the text field in a state that temporarily repopulates after the IME session closes. The send-button path in ChatInputView.swift (handleSubmit) and AskAIInputView.swift (the Button action closure) would benefit from a hasMarkedText guard. Important Files Changed
Sequence DiagramsequenceDiagram
participant User
participant OmiNSTextView
participant Coordinator
participant SwiftUI View
User->>OmiNSTextView: Begins IME composition
OmiNSTextView->>OmiNSTextView: setMarkedText(_:selectedRange:replacementRange:)
OmiNSTextView->>Coordinator: onMarkedTextStatusChange(true)
Coordinator->>Coordinator: updateMarkedTextState(true)
Coordinator-->>SwiftUI View: onMarkedTextChange(true) [async]
SwiftUI View->>SwiftUI View: hasMarkedText = true (hide placeholder)
User->>OmiNSTextView: Presses Return (confirm candidate)
OmiNSTextView->>Coordinator: doCommandBy(insertNewline:)
Coordinator->>Coordinator: "hasMarkedText() == true, return false"
Note over OmiNSTextView: Default IME confirm behavior runs
OmiNSTextView->>Coordinator: insertText (committed text)
OmiNSTextView->>OmiNSTextView: publishMarkedTextStatus() false
Coordinator->>Coordinator: updateMarkedTextState(false)
Coordinator-->>SwiftUI View: onMarkedTextChange(false) [async]
SwiftUI View->>SwiftUI View: hasMarkedText = false
User->>OmiNSTextView: Presses Return (send message)
OmiNSTextView->>Coordinator: doCommandBy(insertNewline:)
Coordinator->>Coordinator: "hasMarkedText() == false, proceed"
Coordinator->>SwiftUI View: onSubmit()
|
| func updateMarkedTextState(_ hasMarkedText: Bool) { | ||
| guard hasMarkedText != lastMarkedTextState else { return } | ||
| lastMarkedTextState = hasMarkedText | ||
| DispatchQueue.main.async { [weak self] in | ||
| guard self?.lastMarkedTextState == hasMarkedText else { return } | ||
| self?.onMarkedTextChange?(hasMarkedText) | ||
| } | ||
| } |
There was a problem hiding this comment.
updateMarkedTextState is called both from the OmiNSTextView override callbacks (via onMarkedTextStatusChange) and unconditionally at the end of every updateNSView. Since AppKit delegate methods and updateNSView all run on the main thread, the DispatchQueue.main.async adds a single-runloop-cycle delay to every state change. In practice the cancellation guard (self?.lastMarkedTextState == hasMarkedText) prevents stale deliveries, but the async hop means there is a brief window between setMarkedText and the next main-queue drain where lastMarkedTextState is already true while SwiftUI still sees hasMarkedText == false. Calling onMarkedTextChange directly on the main thread (without the async hop) would make the state transition atomic with respect to the AppKit text-change event.
kodjima33
left a comment
There was a problem hiding this comment.
thanks — IME composition fix looks good
Bot fit:
Summary:
Tests:
Notes: