fix: emoji input blocked in command palette rename#2291
fix: emoji input blocked in command palette rename#2291pandec wants to merge 3 commits intomanaflow-ai:mainfrom
Conversation
|
@pandec is attempting to deploy a commit to the Manaflow Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughRefactors command-palette text input: loosens marked-text responder checks in AppDelegate, replaces SwiftUI TextField with an AppKit-backed NSTextView representable, removes the focus-lock timer, and adds ownership checks via WindowCommandPaletteOverlayController. Selection geometry invalidation and notification handling were updated. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Window
participant PaletteController
participant NativeTextView
participant AppDelegate
User->>Window: open command palette
Window->>PaletteController: present overlay
PaletteController->>Window: scheduleFocusIntoPalette
Window->>PaletteController: is key & ownsTextInputResponder?
alt owns responder
PaletteController->>NativeTextView: allow input (becomes first responder)
NativeTextView->>NativeTextView: handle commands (doCommandBy)
NativeTextView->>PaletteController: notify selection/edits
PaletteController->>NativeTextView: invalidateCharacterCoordinates / textInputClientDidUpdateSelection()
else does not own
PaletteController->>AppDelegate: query marked-text via AppDelegate helper
AppDelegate->>NativeTextView: check hasMarkedText()
AppDelegate-->>PaletteController: marked-text status
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Greptile SummaryThis PR fixes unreliable emoji input in the command palette rename field on macOS by moving it from the shared SwiftUI/AppKit field-editor path ( Key changes:
Confidence Score: 5/5Safe to merge — all remaining findings are P2 style/naming suggestions with no impact on runtime correctness. The fix is well-scoped and carefully handles the emoji-picker focus edge cases. The only issues found are: (1) the function No files require special attention. Important Files Changed
Sequence DiagramsequenceDiagram
participant User
participant EmojiPicker as macOS Emoji Picker (NSPanel)
participant NativeTV as CommandPaletteNativeTextView
participant Coordinator as NSTextViewDelegate (Coordinator)
participant Overlay as WindowCommandPaletteOverlayController
participant SwiftUI as SwiftUI State
User->>NativeTV: Opens emoji picker (⌃⌘Space)
EmojiPicker->>EmojiPicker: Becomes NSApp.keyWindow (NSPanel)
Note over Overlay: updateFocusLockForWindowState:<br/>!window.isKeyWindow → skip clear first responder<br/>(keeps NativeTV as first responder)
EmojiPicker->>NativeTV: Inserts emoji (replaces selected text)
NativeTV->>Coordinator: textDidChange(_:)
Coordinator->>Coordinator: sanitizedSingleLineText()
Coordinator->>SwiftUI: parent.text = sanitizedText
Coordinator->>NativeTV: invalidateCharacterCoordinates()
Note over NativeTV: Emoji picker stays anchored
User->>EmojiPicker: Selects another emoji
EmojiPicker->>NativeTV: Replaces selection again
Coordinator->>Coordinator: textDidEndEditing check:<br/>NSApp.keyWindow is NSPanel → skip isFocused=false
Note over NativeTV: Field stays focused across<br/>repeated emoji replacements
Reviews (2): Last reviewed commit: "fix: restore rename placeholder and sing..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 2 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="Sources/ContentView.swift">
<violation number="1" location="Sources/ContentView.swift:3801">
P3: Use `target.placeholder` for the native rename input placeholder so the localized empty-state hint is preserved.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Sources/ContentView.swift`:
- Around line 4070-4078: textDidChange currently passes pasted newlines through;
ensure the rename input remains single-line by stripping any newline characters
before updating state: in textDidChange(_:) sanitize textView.string (e.g.,
replace "\n" and "\r" with a single space or remove them) and assign the
sanitized value to parent.text and back into textView.string so the view shows
the single-line text; then continue to call enclosingInputView(for:) ->
syncTextViewFrameToContentSize() and invalidateTextInputGeometry(for:) as
before. Reference textDidChange(_:), enclosingInputView(for:),
container.syncTextViewFrameToContentSize(), and
invalidateTextInputGeometry(for:) when applying this fix.
- Around line 3799-3803: The placeholder for the rename field was removed;
restore it by passing the existing CommandPaletteRenameTarget.placeholder into
the CommandPaletteTextInputRepresentable initializer instead of the empty string
literal (""); update the initializer call in the HStack that creates
CommandPaletteTextInputRepresentable to use
CommandPaletteRenameTarget.placeholder so the localized prompt reappears when
the draft is blank.
- Around line 6929-6933: The deleteBackward currently grabs whichever NSTextView
is firstResponder (via observedWindow ?? NSApp.keyWindow ?? NSApp.mainWindow ->
window.firstResponder), which can target other editors; restrict the deletion to
the palette-owned editor by checking that the firstResponder text view is the
palette's rename field (e.g., compare to a stored reference like
commandPaletteRenameEditor or verify the responder is inside the palette
view/class) before calling deleteBackward and updating
commandPaletteRenameDraft; update the logic that references observedWindow,
window.firstResponder, NSTextView, deleteBackward, and commandPaletteRenameDraft
so the delete action runs only when the responder is the actual command-palette
rename NSTextView.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e6d8fcd1-f315-414b-9980-d7e5f94e7b0e
📒 Files selected for processing (2)
Sources/AppDelegate.swiftSources/ContentView.swift
|
@greptile |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Sources/ContentView.swift (1)
1206-1221:⚠️ Potential issue | 🟡 MinorScope the
">"selection normalization to the command palette search field.The command-mode prefix semantics (restoring caret-at-end for
">"queries) are specific to the palette search field. However,isPaletteTextInputFirstResponder(editor)at line 1209 matches any editable NSTextView descendant ofcontainerViewwhen it's not a field editor (line 1086 in the helper), which could include workspace/tab rename fields. A rename field starting with">"would incorrectly have its full selection normalized away on refocus.Use
isPaletteFieldEditor(editor)instead to ensure this normalization applies only to the palette's search field.🛠️ Suggested fix
guard let window, let editor = window.firstResponder as? NSTextView, - isPaletteTextInputFirstResponder(editor) else { return } + isPaletteFieldEditor(editor) else { return }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/ContentView.swift` around lines 1206 - 1221, The normalization in normalizeSelectionAfterProgrammaticFocus is currently gated by isPaletteTextInputFirstResponder(editor) which matches any editable NSTextView descendant and can wrongly affect rename fields; change the guard to use isPaletteFieldEditor(editor) instead so the caret-at-end restoration only runs for the palette's search field (update the guard that checks isPaletteTextInputFirstResponder to call isPaletteFieldEditor with the same editor variable).
♻️ Duplicate comments (1)
Sources/ContentView.swift (1)
1129-1154:⚠️ Potential issue | 🟠 MajorGuard the async focus retries on palette/window state.
updateFocusLockForWindowState()now cancelsscheduledFocusWorkItem, but theasyncAftertail here is independent. If the window resigns key or the palette hides after the first retry starts, a late callback can still runmakeFirstResponder(...)and yank focus back from the emoji/system panel.🛠️ Suggested fix
private func focusIntoPalette(retries: Int) { - guard let window else { return } + guard let window, isPaletteVisible, window.isKeyWindow else { return } if isPaletteTextInputFirstResponder(window.firstResponder) { return }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/ContentView.swift` around lines 1129 - 1154, The async retry in focusIntoPalette can run after the palette/window state changes because it uses DispatchQueue.main.asyncAfter directly; replace that tail with a cancellable DispatchWorkItem stored in scheduledFocusWorkItem (the same token updateFocusLockForWindowState cancels) or otherwise guard the retry by checking the palette/window state before attempting focus: capture self weakly, create a DispatchWorkItem that first verifies the window is still key and the palette is visible (the same conditions used elsewhere), then calls focusIntoPalette(retries: -1); assign that work item to scheduledFocusWorkItem so updateFocusLockForWindowState can cancel it, and when canceled do not call makeFirstResponder on containerView/firstEditableTextInputView.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@Sources/ContentView.swift`:
- Around line 1206-1221: The normalization in
normalizeSelectionAfterProgrammaticFocus is currently gated by
isPaletteTextInputFirstResponder(editor) which matches any editable NSTextView
descendant and can wrongly affect rename fields; change the guard to use
isPaletteFieldEditor(editor) instead so the caret-at-end restoration only runs
for the palette's search field (update the guard that checks
isPaletteTextInputFirstResponder to call isPaletteFieldEditor with the same
editor variable).
---
Duplicate comments:
In `@Sources/ContentView.swift`:
- Around line 1129-1154: The async retry in focusIntoPalette can run after the
palette/window state changes because it uses DispatchQueue.main.asyncAfter
directly; replace that tail with a cancellable DispatchWorkItem stored in
scheduledFocusWorkItem (the same token updateFocusLockForWindowState cancels) or
otherwise guard the retry by checking the palette/window state before attempting
focus: capture self weakly, create a DispatchWorkItem that first verifies the
window is still key and the palette is visible (the same conditions used
elsewhere), then calls focusIntoPalette(retries: -1); assign that work item to
scheduledFocusWorkItem so updateFocusLockForWindowState can cancel it, and when
canceled do not call makeFirstResponder on
containerView/firstEditableTextInputView.
1 participant
Summary
NSTextViewpath and update palette responder ownership and focus handling so the emoji picker stays attached to the rename field.Related: #2161
Test plan
(Retested after the greptile/coderabit review fixes as well 👍)
Summary by cubic
Fixes unreliable emoji input in the command palette rename on macOS by moving rename to a dedicated AppKit
NSTextView. Restores placeholder and single-line behavior, and tightens focus so the emoji picker stays attached during repeated replacements.NSTextView-backedCommandPaletteTextInputRepresentablefor rename with placeholder and single-line enforcement, bypassing the shared field editor.NSTextView), schedule focus without clearing first responder, and removed the periodic focus timer.Written for commit 0b07651. Summary will update on new commits.
Summary by CodeRabbit
Bug Fixes
New Features