Merge pull request #1417 from manaflow-ai/issue-1414-cmd-palette-arrow-keys

Fix command palette command-mode shortcut and navigation
This commit is contained in:
Austin Wang 2026-03-13 20:15:28 -07:00 committed by GitHub
commit 85ebbb686f
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 265 additions and 23 deletions

View file

@ -7902,11 +7902,13 @@ final class AppDelegate: NSObject, NSApplicationDelegate, UNUserNotificationCent
let commandPaletteResponderActiveInTargetWindow = commandPaletteTargetWindow.map {
isCommandPaletteResponderActive(in: $0)
} ?? false
let commandPaletteEffectiveInTargetWindow =
let commandPaletteInteractiveInTargetWindow =
commandPaletteVisibleInTargetWindow
|| commandPalettePendingOpenInTargetWindow
|| commandPaletteOverlayVisibleInTargetWindow
|| commandPaletteResponderActiveInTargetWindow
let commandPaletteEffectiveInTargetWindow =
commandPaletteInteractiveInTargetWindow
|| commandPalettePendingOpenInTargetWindow
if normalizedFlags.isEmpty, event.keyCode == 53 {
let activePaletteWindow = activeCommandPaletteWindow()
@ -7995,7 +7997,7 @@ final class AppDelegate: NSObject, NSApplicationDelegate, UNUserNotificationCent
chars: chars,
keyCode: event.keyCode
),
commandPaletteVisibleInTargetWindow,
commandPaletteInteractiveInTargetWindow,
let paletteWindow = commandPaletteShortcutWindow {
NotificationCenter.default.post(
name: .commandPaletteMoveSelection,
@ -8005,7 +8007,7 @@ final class AppDelegate: NSObject, NSApplicationDelegate, UNUserNotificationCent
return true
}
if commandPaletteVisibleInTargetWindow,
if commandPaletteInteractiveInTargetWindow,
let paletteWindow = commandPaletteShortcutWindow {
let paletteFieldEditorHasMarkedText = commandPaletteFieldEditorHasMarkedText(in: paletteWindow)
if normalizedFlags.isEmpty, event.keyCode == 53 {
@ -8050,6 +8052,16 @@ final class AppDelegate: NSObject, NSApplicationDelegate, UNUserNotificationCent
// Scope the omnibar check to the shortcut's routed window context so a
// focused omnibar in another window does not suppress Cmd+P here.
let hasFocusedAddressBarInShortcutContext = focusedBrowserAddressBarPanelIdForShortcutEvent(event) != nil
let isCommandShiftP = matchShortcut(
event: event,
shortcut: StoredShortcut(key: "p", command: true, shift: true, option: false, control: false)
)
if isCommandShiftP {
let targetWindow = commandPaletteTargetWindow ?? event.window ?? NSApp.keyWindow ?? NSApp.mainWindow
requestCommandPaletteCommands(preferredWindow: targetWindow, source: "shortcut.cmdShiftP")
return true
}
let isCommandP = !hasFocusedAddressBarInShortcutContext
&& matchShortcut(
event: event,
@ -8061,16 +8073,6 @@ final class AppDelegate: NSObject, NSApplicationDelegate, UNUserNotificationCent
return true
}
let isCommandShiftP = matchShortcut(
event: event,
shortcut: StoredShortcut(key: "p", command: true, shift: true, option: false, control: false)
)
if isCommandShiftP {
let targetWindow = commandPaletteTargetWindow ?? event.window ?? NSApp.keyWindow ?? NSApp.mainWindow
requestCommandPaletteCommands(preferredWindow: targetWindow, source: "shortcut.cmdShiftP")
return true
}
if shouldConsumeShortcutWhileCommandPaletteVisible(
isCommandPaletteVisible: commandPaletteEffectiveInTargetWindow,
normalizedFlags: normalizedFlags,

View file

@ -3062,6 +3062,7 @@ struct ContentView: View {
private var commandPaletteCommandListView: some View {
let visibleResults = commandPaletteVisibleResults
let selectedIndex = commandPaletteSelectedIndex(resultCount: visibleResults.count)
let commandPaletteListIdentity = "\(commandPaletteListScope.rawValue):\(commandPaletteQuery)"
let commandPaletteListMaxHeight: CGFloat = 450
let commandPaletteRowHeight: CGFloat = 24
let commandPaletteEmptyStateHeight: CGFloat = 44
@ -3090,7 +3091,9 @@ struct ContentView: View {
Divider()
ScrollView {
LazyVStack(spacing: 0) {
// Rebuild the full results container on scope/query transitions so
// stale switcher rows cannot linger above command-mode results.
VStack(spacing: 0) {
if visibleResults.isEmpty {
if commandPaletteHasCurrentResolvedResults {
Text(commandPaletteEmptyStateText)
@ -3160,9 +3163,8 @@ struct ContentView: View {
}
}
.scrollTargetLayout()
// Force a fresh row tree per query so rendered labels/actions stay in lockstep.
.id(commandPaletteQuery)
}
.id(commandPaletteListIdentity)
.frame(height: commandPaletteListHeight)
.scrollPosition(
id: Binding(
@ -3329,6 +3331,8 @@ struct ContentView: View {
}
private final class CommandPaletteNativeTextField: NSTextField {
var onHandleKeyEvent: ((NSEvent, NSTextView?) -> Bool)?
override init(frame frameRect: NSRect) {
super.init(frame: frameRect)
isBordered = false
@ -3341,6 +3345,27 @@ struct ContentView: View {
required init?(coder: NSCoder) {
fatalError("init(coder:) has not been implemented")
}
override func keyDown(with event: NSEvent) {
if (currentEditor() as? NSTextView)?.hasMarkedText() == true {
super.keyDown(with: event)
return
}
if onHandleKeyEvent?(event, currentEditor() as? NSTextView) == true {
return
}
super.keyDown(with: event)
}
override func performKeyEquivalent(with event: NSEvent) -> Bool {
if (currentEditor() as? NSTextView)?.hasMarkedText() == true {
return super.performKeyEquivalent(with: event)
}
if onHandleKeyEvent?(event, currentEditor() as? NSTextView) == true {
return true
}
return super.performKeyEquivalent(with: event)
}
}
// Keep navigation on the AppKit field editor so deleting the ">" prefix
@ -3358,11 +3383,17 @@ struct ContentView: View {
var isProgrammaticMutation = false
weak var parentField: CommandPaletteNativeTextField?
var pendingFocusRequest: Bool?
var editorTextDidChangeObserver: NSObjectProtocol?
weak var observedEditor: NSTextView?
init(parent: CommandPaletteSearchFieldRepresentable) {
self.parent = parent
}
deinit {
detachEditorTextDidChangeObserver()
}
func controlTextDidChange(_ obj: Notification) {
guard !isProgrammaticMutation else { return }
guard let field = obj.object as? NSTextField else { return }
@ -3370,6 +3401,10 @@ struct ContentView: View {
}
func controlTextDidBeginEditing(_ obj: Notification) {
if let field = obj.object as? NSTextField,
let editor = field.currentEditor() as? NSTextView {
attachEditorTextDidChangeObserverIfNeeded(editor)
}
if !parent.isFocused {
DispatchQueue.main.async {
self.parent.isFocused = true
@ -3377,6 +3412,10 @@ struct ContentView: View {
}
}
func controlTextDidEndEditing(_ obj: Notification) {
detachEditorTextDidChangeObserver()
}
func control(_ control: NSControl, textView: NSTextView, doCommandBy commandSelector: Selector) -> Bool {
switch commandSelector {
case #selector(NSResponder.moveDown(_:)):
@ -3397,6 +3436,62 @@ struct ContentView: View {
return false
}
}
func handleKeyEvent(_ event: NSEvent, editor: NSTextView?) -> Bool {
guard !(editor?.hasMarkedText() ?? false) else { return false }
if let delta = commandPaletteSelectionDeltaForKeyboardNavigation(
flags: event.modifierFlags,
chars: event.characters ?? event.charactersIgnoringModifiers ?? "",
keyCode: event.keyCode
) {
parent.onMoveSelection(delta)
return true
}
if shouldSubmitCommandPaletteWithReturn(
keyCode: event.keyCode,
flags: event.modifierFlags
) {
parent.onSubmit()
return true
}
if event.keyCode == 53,
event.modifierFlags
.intersection(.deviceIndependentFlagsMask)
.subtracting([.numericPad, .function, .capsLock])
.isEmpty {
parent.onEscape()
return true
}
return false
}
func attachEditorTextDidChangeObserverIfNeeded(_ editor: NSTextView) {
if observedEditor !== editor {
detachEditorTextDidChangeObserver()
}
guard editorTextDidChangeObserver == nil else { return }
observedEditor = editor
editorTextDidChangeObserver = NotificationCenter.default.addObserver(
forName: NSText.didChangeNotification,
object: editor,
queue: .main
) { [weak self] _ in
guard let self, !self.isProgrammaticMutation else { return }
self.parent.text = editor.string
}
}
func detachEditorTextDidChangeObserver() {
if let editorTextDidChangeObserver {
NotificationCenter.default.removeObserver(editorTextDidChangeObserver)
self.editorTextDidChangeObserver = nil
}
observedEditor = nil
}
}
func makeCoordinator() -> Coordinator {
@ -3413,6 +3508,9 @@ struct ContentView: View {
field.isEditable = true
field.isSelectable = true
field.isEnabled = true
field.onHandleKeyEvent = { [weak coordinator = context.coordinator] event, editor in
coordinator?.handleKeyEvent(event, editor: editor) ?? false
}
context.coordinator.parentField = field
return field
}
@ -3423,6 +3521,7 @@ struct ContentView: View {
nsView.placeholderString = placeholder
if let editor = nsView.currentEditor() as? NSTextView {
context.coordinator.attachEditorTextDidChangeObserverIfNeeded(editor)
if editor.string != text, !editor.hasMarkedText() {
context.coordinator.isProgrammaticMutation = true
editor.string = text
@ -3430,7 +3529,10 @@ struct ContentView: View {
context.coordinator.isProgrammaticMutation = false
}
} else if nsView.stringValue != text {
context.coordinator.detachEditorTextDidChangeObserver()
nsView.stringValue = text
} else {
context.coordinator.detachEditorTextDidChangeObserver()
}
guard let window = nsView.window else { return }
@ -3459,6 +3561,8 @@ struct ContentView: View {
static func dismantleNSView(_ nsView: CommandPaletteNativeTextField, coordinator: Coordinator) {
nsView.delegate = nil
nsView.onHandleKeyEvent = nil
coordinator.detachEditorTextDidChangeObserver()
coordinator.parentField = nil
}
}
@ -5913,19 +6017,27 @@ struct ContentView: View {
}
private func openCommandPaletteCommands() {
toggleCommandPalette(initialQuery: Self.commandPaletteCommandsPrefix)
handleCommandPaletteListRequest(scope: .commands)
}
private func openCommandPaletteSwitcher() {
toggleCommandPalette(initialQuery: "")
handleCommandPaletteListRequest(scope: .switcher)
}
private func toggleCommandPalette(initialQuery: String) {
if isCommandPalettePresented {
dismissCommandPalette()
} else {
private func handleCommandPaletteListRequest(scope: CommandPaletteListScope) {
let initialQuery = (scope == .commands) ? Self.commandPaletteCommandsPrefix : ""
guard isCommandPalettePresented else {
presentCommandPalette(initialQuery: initialQuery)
return
}
if case .commands = commandPaletteMode,
commandPaletteListScope == scope {
dismissCommandPalette()
return
}
resetCommandPaletteListState(initialQuery: initialQuery)
}
private func openCommandPaletteRenameTabInput() {

View file

@ -955,6 +955,63 @@ final class AppDelegateShortcutRoutingTests: XCTestCase {
#endif
}
func testCmdShiftPRequestsCommandPaletteCommands() {
guard let appDelegate = AppDelegate.shared else {
XCTFail("Expected AppDelegate.shared")
return
}
let windowId = appDelegate.createMainWindow()
defer { closeWindow(withId: windowId) }
guard let window = window(withId: windowId) else {
XCTFail("Expected test window")
return
}
let paletteExpectation = expectation(description: "Expected command palette commands request for Cmd+Shift+P")
var observedPaletteWindow: NSWindow?
let paletteToken = NotificationCenter.default.addObserver(
forName: .commandPaletteRequested,
object: nil,
queue: nil
) { notification in
observedPaletteWindow = notification.object as? NSWindow
paletteExpectation.fulfill()
}
defer { NotificationCenter.default.removeObserver(paletteToken) }
let switcherExpectation = expectation(description: "Cmd+Shift+P should not request command palette switcher")
switcherExpectation.isInverted = true
let switcherToken = NotificationCenter.default.addObserver(
forName: .commandPaletteSwitcherRequested,
object: nil,
queue: nil
) { _ in
switcherExpectation.fulfill()
}
defer { NotificationCenter.default.removeObserver(switcherToken) }
guard let event = makeKeyDownEvent(
key: "P",
modifiers: [.command, .shift],
keyCode: 35,
windowNumber: window.windowNumber
) else {
XCTFail("Failed to construct Cmd+Shift+P event")
return
}
#if DEBUG
XCTAssertTrue(appDelegate.debugHandleCustomShortcut(event: event))
#else
XCTFail("debugHandleCustomShortcut is only available in DEBUG")
#endif
wait(for: [paletteExpectation, switcherExpectation], timeout: 1.0)
XCTAssertEqual(observedPaletteWindow?.windowNumber, window.windowNumber)
}
func testCmdPhysicalWWithDvorakCharactersDoesNotTriggerClosePanelShortcut() {
guard let appDelegate = AppDelegate.shared else {
XCTFail("Expected AppDelegate.shared")
@ -1712,6 +1769,77 @@ final class AppDelegateShortcutRoutingTests: XCTestCase {
XCTAssertEqual(observedDismissWindow?.windowNumber, window.windowNumber)
}
func testArrowNavigationRoutesWhileCommandPaletteOverlayIsInteractiveBeforeVisibilitySync() {
guard let appDelegate = AppDelegate.shared else {
XCTFail("Expected AppDelegate.shared")
return
}
let windowId = appDelegate.createMainWindow()
defer {
closeWindow(withId: windowId)
}
guard let window = window(withId: windowId),
let contentView = window.contentView else {
XCTFail("Expected test window")
return
}
let overlayContainer = NSView(frame: contentView.bounds)
overlayContainer.identifier = commandPaletteOverlayContainerIdentifier
overlayContainer.alphaValue = 1
overlayContainer.isHidden = false
contentView.addSubview(overlayContainer)
let fieldEditor = CommandPaletteMarkedTextFieldEditor(frame: NSRect(x: 0, y: 0, width: 200, height: 24))
fieldEditor.isFieldEditor = true
overlayContainer.addSubview(fieldEditor)
XCTAssertTrue(window.makeFirstResponder(fieldEditor))
appDelegate.setCommandPaletteVisible(false, for: window)
defer {
overlayContainer.removeFromSuperview()
fieldEditor.removeFromSuperview()
}
let moveExpectation = expectation(
description: "Expected command palette move-selection notification while overlay is interactive"
)
var observedDelta: Int?
var observedWindow: NSWindow?
let moveToken = NotificationCenter.default.addObserver(
forName: .commandPaletteMoveSelection,
object: nil,
queue: nil
) { notification in
observedWindow = notification.object as? NSWindow
observedDelta = notification.userInfo?["delta"] as? Int
moveExpectation.fulfill()
}
defer { NotificationCenter.default.removeObserver(moveToken) }
guard let downArrowEvent = makeKeyDownEvent(
key: String(UnicodeScalar(NSDownArrowFunctionKey)!),
modifiers: [],
keyCode: 125,
windowNumber: window.windowNumber
) else {
XCTFail("Failed to construct Down Arrow event")
return
}
#if DEBUG
XCTAssertTrue(appDelegate.debugHandleCustomShortcut(event: downArrowEvent))
#else
XCTFail("debugHandleCustomShortcut is only available in DEBUG")
#endif
wait(for: [moveExpectation], timeout: 1.0)
XCTAssertEqual(observedWindow?.windowNumber, window.windowNumber)
XCTAssertEqual(observedDelta, 1)
}
func testEscapeDismissesCommandPaletteWhenVisibilityStateStaysStalePastInitialPendingWindow() {
guard let appDelegate = AppDelegate.shared else {
XCTFail("Expected AppDelegate.shared")