From 50c59babc1473948135465399c61e6bea6aa40f4 Mon Sep 17 00:00:00 2001 From: austinpower1258 Date: Fri, 13 Mar 2026 20:14:01 -0700 Subject: [PATCH 1/2] Add regression tests for command palette shortcut routing --- .../AppDelegateShortcutRoutingTests.swift | 128 ++++++++++++++++++ 1 file changed, 128 insertions(+) diff --git a/cmuxTests/AppDelegateShortcutRoutingTests.swift b/cmuxTests/AppDelegateShortcutRoutingTests.swift index 26c25bf2..05062a22 100644 --- a/cmuxTests/AppDelegateShortcutRoutingTests.swift +++ b/cmuxTests/AppDelegateShortcutRoutingTests.swift @@ -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") From c1e264f325f60b377a38108508ee1283b7edc8bb Mon Sep 17 00:00:00 2001 From: austinpower1258 Date: Fri, 13 Mar 2026 20:14:03 -0700 Subject: [PATCH 2/2] Fix command palette command mode navigation --- Sources/AppDelegate.swift | 30 +++++---- Sources/ContentView.swift | 130 +++++++++++++++++++++++++++++++++++--- 2 files changed, 137 insertions(+), 23 deletions(-) diff --git a/Sources/AppDelegate.swift b/Sources/AppDelegate.swift index fa269175..71ca25a5 100644 --- a/Sources/AppDelegate.swift +++ b/Sources/AppDelegate.swift @@ -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, diff --git a/Sources/ContentView.swift b/Sources/ContentView.swift index 8a07eaaa..2a6fdb3c 100644 --- a/Sources/ContentView.swift +++ b/Sources/ContentView.swift @@ -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() {