diff --git a/Sources/AppDelegate.swift b/Sources/AppDelegate.swift index ef7215c0..28c72ae1 100644 --- a/Sources/AppDelegate.swift +++ b/Sources/AppDelegate.swift @@ -315,6 +315,42 @@ func commandPaletteSelectionDeltaForKeyboardNavigation( return nil } +func shouldConsumeShortcutWhileCommandPaletteVisible( + isCommandPaletteVisible: Bool, + normalizedFlags: NSEvent.ModifierFlags, + chars: String, + keyCode: UInt16 +) -> Bool { + guard isCommandPaletteVisible else { return false } + guard normalizedFlags.contains(.command) else { return false } + + let normalizedChars = chars.lowercased() + + if normalizedFlags == [.command] { + if normalizedChars == "a" + || normalizedChars == "c" + || normalizedChars == "v" + || normalizedChars == "x" + || normalizedChars == "z" + || normalizedChars == "y" { + return false + } + + switch keyCode { + case 51, 117, 123, 124: + return false + default: + break + } + } + + if normalizedFlags == [.command, .shift], normalizedChars == "z" { + return false + } + + return true +} + enum BrowserZoomShortcutAction: Equatable { case zoomIn case zoomOut @@ -2578,6 +2614,15 @@ final class AppDelegate: NSObject, NSApplicationDelegate, UNUserNotificationCent return true } + if shouldConsumeShortcutWhileCommandPaletteVisible( + isCommandPaletteVisible: activeCommandPaletteWindow() != nil, + normalizedFlags: normalizedFlags, + chars: chars, + keyCode: event.keyCode + ) { + return true + } + if normalizedFlags == [.command], chars == "q" { return handleQuitShortcutWarning() } @@ -3100,6 +3145,15 @@ final class AppDelegate: NSObject, NSApplicationDelegate, UNUserNotificationCent NotificationCenter.default.post(name: .browserFocusAddressBar, object: panel.id) } + func focusedBrowserAddressBarPanelId() -> UUID? { + browserAddressBarFocusedPanelId + } + + @discardableResult + func requestBrowserAddressBarFocus(panelId: UUID) -> Bool { + focusBrowserAddressBar(panelId: panelId) + } + private func shouldBypassAppShortcutForFocusedBrowserAddressBar( flags: NSEvent.ModifierFlags, chars: String diff --git a/Sources/ContentView.swift b/Sources/ContentView.swift index 55e8ec22..dc0adaaf 100644 --- a/Sources/ContentView.swift +++ b/Sources/ContentView.swift @@ -28,6 +28,16 @@ private func coloredCircleImage(color: NSColor) -> NSImage { return image } +func sidebarActiveForegroundNSColor( + opacity: CGFloat, + appAppearance: NSAppearance? = NSApp?.effectiveAppearance +) -> NSColor { + let clampedOpacity = max(0, min(opacity, 1)) + let bestMatch = appAppearance?.bestMatch(from: [.darkAqua, .aqua]) + let baseColor: NSColor = (bestMatch == .darkAqua) ? .white : .black + return baseColor.withAlphaComponent(clampedOpacity) +} + struct ShortcutHintPillBackground: View { var emphasis: Double = 1.0 @@ -1169,9 +1179,15 @@ struct ContentView: View { } } + private enum CommandPaletteRestoreFocusIntent { + case panel + case browserAddressBar + } + private struct CommandPaletteRestoreFocusTarget { let workspaceId: UUID let panelId: UUID + let intent: CommandPaletteRestoreFocusIntent } private enum CommandPaletteInputFocusTarget { @@ -2449,7 +2465,7 @@ struct ContentView: View { TextField(commandPaletteSearchPlaceholder, text: $commandPaletteQuery) .textFieldStyle(.plain) .font(.system(size: 13, weight: .regular)) - .tint(.white) + .tint(Color(nsColor: sidebarActiveForegroundNSColor(opacity: 1.0))) .focused($isCommandPaletteSearchFocused) .onSubmit { runSelectedCommandPaletteResult(visibleResults: visibleResults) @@ -2602,7 +2618,7 @@ struct ContentView: View { TextField(target.placeholder, text: $commandPaletteRenameDraft) .textFieldStyle(.plain) .font(.system(size: 13, weight: .regular)) - .tint(.white) + .tint(Color(nsColor: sidebarActiveForegroundNSColor(opacity: 1.0))) .focused($isCommandPaletteRenameFocused) .backport.onKeyPress(.delete) { modifiers in handleCommandPaletteRenameDeleteBackward(modifiers: modifiers) @@ -4137,6 +4153,14 @@ struct ContentView: View { return false } + static func shouldRestoreBrowserAddressBarAfterCommandPaletteDismiss( + focusedPanelIsBrowser: Bool, + focusedBrowserAddressBarPanelId: UUID?, + focusedPanelId: UUID + ) -> Bool { + focusedPanelIsBrowser && focusedBrowserAddressBarPanelId == focusedPanelId + } + private func syncCommandPaletteDebugStateForObservedWindow() { guard let window = observedWindow ?? NSApp.keyWindow ?? NSApp.mainWindow else { return } AppDelegate.shared?.setCommandPaletteVisible(isCommandPalettePresented, for: window) @@ -4178,9 +4202,15 @@ struct ContentView: View { private func presentCommandPalette(initialQuery: String) { if let panelContext = focusedPanelContext { + let shouldRestoreBrowserAddressBar = Self.shouldRestoreBrowserAddressBarAfterCommandPaletteDismiss( + focusedPanelIsBrowser: panelContext.panel.panelType == .browser, + focusedBrowserAddressBarPanelId: AppDelegate.shared?.focusedBrowserAddressBarPanelId(), + focusedPanelId: panelContext.panelId + ) commandPaletteRestoreFocusTarget = CommandPaletteRestoreFocusTarget( workspaceId: panelContext.workspace.id, - panelId: panelContext.panelId + panelId: panelContext.panelId, + intent: shouldRestoreBrowserAddressBar ? .browserAddressBar : .panel ) } else { commandPaletteRestoreFocusTarget = nil @@ -4236,18 +4266,47 @@ struct ContentView: View { } tabManager.focusTab(target.workspaceId, surfaceId: target.panelId, suppressFlash: true) + if let context = focusedPanelContext, + context.workspace.id == target.workspaceId, + context.panelId == target.panelId { + restoreCommandPaletteInputFocusIfNeeded(target: target, attemptsRemaining: 6) + return + } + guard attemptsRemaining > 0 else { return } DispatchQueue.main.asyncAfter(deadline: .now() + 0.03) { guard !isCommandPalettePresented else { return } if let context = focusedPanelContext, context.workspace.id == target.workspaceId, context.panelId == target.panelId { + restoreCommandPaletteInputFocusIfNeeded(target: target, attemptsRemaining: 6) return } restoreCommandPaletteFocus(target: target, attemptsRemaining: attemptsRemaining - 1) } } + private func restoreCommandPaletteInputFocusIfNeeded( + target: CommandPaletteRestoreFocusTarget, + attemptsRemaining: Int + ) { + guard !isCommandPalettePresented else { return } + guard target.intent == .browserAddressBar else { return } + guard attemptsRemaining > 0 else { return } + guard let appDelegate = AppDelegate.shared else { return } + + if appDelegate.requestBrowserAddressBarFocus(panelId: target.panelId) { + return + } + + DispatchQueue.main.asyncAfter(deadline: .now() + 0.03) { + restoreCommandPaletteInputFocusIfNeeded( + target: target, + attemptsRemaining: attemptsRemaining - 1 + ) + } + } + private func resetCommandPaletteSearchFocus() { applyCommandPaletteInputFocusPolicy(.search) } @@ -5867,11 +5926,13 @@ private struct TabItemView: View { } private var activePrimaryTextColor: Color { - usesInvertedActiveForeground ? .white : .primary + usesInvertedActiveForeground ? Color(nsColor: sidebarActiveForegroundNSColor(opacity: 1.0)) : .primary } private func activeSecondaryColor(_ opacity: Double = 0.75) -> Color { - usesInvertedActiveForeground ? .white.opacity(opacity) : .secondary + usesInvertedActiveForeground + ? Color(nsColor: sidebarActiveForegroundNSColor(opacity: CGFloat(opacity))) + : .secondary } private var activeUnreadBadgeFillColor: Color { @@ -6614,11 +6675,16 @@ private struct TabItemView: View { private func logLevelColor(_ level: SidebarLogLevel, isActive: Bool) -> Color { if isActive { switch level { - case .info: return .white.opacity(0.5) - case .progress: return .white.opacity(0.8) - case .success: return .white.opacity(0.9) - case .warning: return .white.opacity(0.9) - case .error: return .white.opacity(0.9) + case .info: + return Color(nsColor: sidebarActiveForegroundNSColor(opacity: 0.5)) + case .progress: + return Color(nsColor: sidebarActiveForegroundNSColor(opacity: 0.8)) + case .success: + return Color(nsColor: sidebarActiveForegroundNSColor(opacity: 0.9)) + case .warning: + return Color(nsColor: sidebarActiveForegroundNSColor(opacity: 0.9)) + case .error: + return Color(nsColor: sidebarActiveForegroundNSColor(opacity: 0.9)) } } switch level { @@ -6724,7 +6790,7 @@ private struct SidebarStatusPillsRow: View { VStack(alignment: .leading, spacing: 2) { Text(statusText) .font(.system(size: 10)) - .foregroundColor(isActive ? .white.opacity(0.8) : .secondary) + .foregroundColor(isActive ? activePrimaryTextColor : .secondary) .lineLimit(isExpanded ? nil : 3) .truncationMode(.tail) .multilineTextAlignment(.leading) @@ -6747,13 +6813,21 @@ private struct SidebarStatusPillsRow: View { } .buttonStyle(.plain) .font(.system(size: 10, weight: .semibold)) - .foregroundColor(isActive ? .white.opacity(0.65) : .secondary.opacity(0.9)) + .foregroundColor(isActive ? activeSecondaryTextColor : .secondary.opacity(0.9)) .frame(maxWidth: .infinity, alignment: .leading) } } .help(statusText) } + private var activePrimaryTextColor: Color { + Color(nsColor: sidebarActiveForegroundNSColor(opacity: 0.8)) + } + + private var activeSecondaryTextColor: Color { + Color(nsColor: sidebarActiveForegroundNSColor(opacity: 0.65)) + } + private var statusText: String { entries .map { entry in diff --git a/Sources/Panels/BrowserPanelView.swift b/Sources/Panels/BrowserPanelView.swift index f91b71e8..71518994 100644 --- a/Sources/Panels/BrowserPanelView.swift +++ b/Sources/Panels/BrowserPanelView.swift @@ -2181,6 +2181,13 @@ struct OmnibarSuggestion: Identifiable, Hashable { } } +func browserOmnibarShouldReacquireFocusAfterEndEditing( + suppressWebViewFocus: Bool, + nextResponderIsOtherTextField: Bool +) -> Bool { + suppressWebViewFocus && !nextResponderIsOtherTextField +} + private final class OmnibarNativeTextField: NSTextField { var onPointerDown: (() -> Void)? var onHandleKeyEvent: ((NSEvent, NSTextView?) -> Bool)? @@ -2293,6 +2300,29 @@ private struct OmnibarTextFieldRepresentable: NSViewRepresentable { } } + private func nextResponderIsOtherTextField(window: NSWindow?) -> Bool { + guard let window, let field = parentField else { return false } + let responder = window.firstResponder + + if let editor = responder as? NSTextView, + let delegateField = editor.delegate as? NSTextField { + return delegateField !== field + } + + if let textField = responder as? NSTextField { + return textField !== field + } + + return false + } + + private func shouldReacquireFocusAfterEndEditing(window: NSWindow?) -> Bool { + return browserOmnibarShouldReacquireFocusAfterEndEditing( + suppressWebViewFocus: parent.shouldSuppressWebViewFocus(), + nextResponderIsOtherTextField: nextResponderIsOtherTextField(window: window) + ) + } + func controlTextDidBeginEditing(_ obj: Notification) { if !parent.isFocused { DispatchQueue.main.async { @@ -2305,15 +2335,18 @@ private struct OmnibarTextFieldRepresentable: NSViewRepresentable { func controlTextDidEndEditing(_ obj: Notification) { if parent.isFocused { - if parent.shouldSuppressWebViewFocus() { + if shouldReacquireFocusAfterEndEditing(window: parentField?.window) { guard pendingFocusRequest != true else { return } pendingFocusRequest = true DispatchQueue.main.async { [weak self] in guard let self else { return } self.pendingFocusRequest = nil guard self.parent.isFocused else { return } - guard self.parent.shouldSuppressWebViewFocus() else { return } guard let field = self.parentField, let window = field.window else { return } + guard self.shouldReacquireFocusAfterEndEditing(window: window) else { + self.parent.onFieldLostFocus() + return + } // Check both the field itself AND its field editor (which becomes // the actual first responder when the text field is being edited). let fr = window.firstResponder diff --git a/cmuxTests/CmuxWebViewKeyEquivalentTests.swift b/cmuxTests/CmuxWebViewKeyEquivalentTests.swift index c875cf11..8484c957 100644 --- a/cmuxTests/CmuxWebViewKeyEquivalentTests.swift +++ b/cmuxTests/CmuxWebViewKeyEquivalentTests.swift @@ -773,6 +773,40 @@ final class BrowserPanelOmnibarPillBackgroundColorTests: XCTestCase { } } +final class SidebarActiveForegroundColorTests: XCTestCase { + func testLightAppearanceUsesBlackWithRequestedOpacity() { + guard let lightAppearance = NSAppearance(named: .aqua), + let color = sidebarActiveForegroundNSColor( + opacity: 0.8, + appAppearance: lightAppearance + ).usingColorSpace(.sRGB) else { + XCTFail("Expected sRGB-convertible color") + return + } + + XCTAssertEqual(color.redComponent, 0, accuracy: 0.001) + XCTAssertEqual(color.greenComponent, 0, accuracy: 0.001) + XCTAssertEqual(color.blueComponent, 0, accuracy: 0.001) + XCTAssertEqual(color.alphaComponent, 0.8, accuracy: 0.001) + } + + func testDarkAppearanceUsesWhiteWithRequestedOpacity() { + guard let darkAppearance = NSAppearance(named: .darkAqua), + let color = sidebarActiveForegroundNSColor( + opacity: 0.65, + appAppearance: darkAppearance + ).usingColorSpace(.sRGB) else { + XCTFail("Expected sRGB-convertible color") + return + } + + XCTAssertEqual(color.redComponent, 1, accuracy: 0.001) + XCTAssertEqual(color.greenComponent, 1, accuracy: 0.001) + XCTAssertEqual(color.blueComponent, 1, accuracy: 0.001) + XCTAssertEqual(color.alphaComponent, 0.65, accuracy: 0.001) + } +} + final class BrowserDeveloperToolsShortcutDefaultsTests: XCTestCase { func testSafariDefaultShortcutForToggleDeveloperTools() { let shortcut = KeyboardShortcutSettings.Action.toggleBrowserDeveloperTools.defaultShortcut @@ -1581,6 +1615,126 @@ final class CommandPaletteKeyboardNavigationTests: XCTestCase { } } +final class CommandPaletteOpenShortcutConsumptionTests: XCTestCase { + func testDoesNotConsumeWhenPaletteIsNotVisible() { + XCTAssertFalse( + shouldConsumeShortcutWhileCommandPaletteVisible( + isCommandPaletteVisible: false, + normalizedFlags: [.command], + chars: "n", + keyCode: 45 + ) + ) + } + + func testConsumesAppCommandShortcutsWhenPaletteIsVisible() { + XCTAssertTrue( + shouldConsumeShortcutWhileCommandPaletteVisible( + isCommandPaletteVisible: true, + normalizedFlags: [.command], + chars: "n", + keyCode: 45 + ) + ) + XCTAssertTrue( + shouldConsumeShortcutWhileCommandPaletteVisible( + isCommandPaletteVisible: true, + normalizedFlags: [.command], + chars: "t", + keyCode: 17 + ) + ) + XCTAssertTrue( + shouldConsumeShortcutWhileCommandPaletteVisible( + isCommandPaletteVisible: true, + normalizedFlags: [.command, .shift], + chars: ",", + keyCode: 43 + ) + ) + } + + func testAllowsClipboardAndUndoShortcutsForPaletteTextEditing() { + XCTAssertFalse( + shouldConsumeShortcutWhileCommandPaletteVisible( + isCommandPaletteVisible: true, + normalizedFlags: [.command], + chars: "v", + keyCode: 9 + ) + ) + XCTAssertFalse( + shouldConsumeShortcutWhileCommandPaletteVisible( + isCommandPaletteVisible: true, + normalizedFlags: [.command], + chars: "z", + keyCode: 6 + ) + ) + XCTAssertFalse( + shouldConsumeShortcutWhileCommandPaletteVisible( + isCommandPaletteVisible: true, + normalizedFlags: [.command, .shift], + chars: "z", + keyCode: 6 + ) + ) + } + + func testAllowsArrowAndDeleteEditingCommandsForPaletteTextEditing() { + XCTAssertFalse( + shouldConsumeShortcutWhileCommandPaletteVisible( + isCommandPaletteVisible: true, + normalizedFlags: [.command], + chars: "", + keyCode: 123 + ) + ) + XCTAssertFalse( + shouldConsumeShortcutWhileCommandPaletteVisible( + isCommandPaletteVisible: true, + normalizedFlags: [.command], + chars: "", + keyCode: 51 + ) + ) + } +} + +final class CommandPaletteRestoreFocusStateMachineTests: XCTestCase { + func testRestoresBrowserAddressBarWhenPaletteOpenedFromFocusedAddressBar() { + let panelId = UUID() + XCTAssertTrue( + ContentView.shouldRestoreBrowserAddressBarAfterCommandPaletteDismiss( + focusedPanelIsBrowser: true, + focusedBrowserAddressBarPanelId: panelId, + focusedPanelId: panelId + ) + ) + } + + func testDoesNotRestoreBrowserAddressBarWhenFocusedPanelIsNotBrowser() { + let panelId = UUID() + XCTAssertFalse( + ContentView.shouldRestoreBrowserAddressBarAfterCommandPaletteDismiss( + focusedPanelIsBrowser: false, + focusedBrowserAddressBarPanelId: panelId, + focusedPanelId: panelId + ) + ) + } + + func testDoesNotRestoreBrowserAddressBarWhenAnotherPanelHadAddressBarFocus() { + XCTAssertFalse( + ContentView.shouldRestoreBrowserAddressBarAfterCommandPaletteDismiss( + focusedPanelIsBrowser: true, + focusedBrowserAddressBarPanelId: UUID(), + focusedPanelId: UUID() + ) + ) + } +} + final class CommandPaletteRenameSelectionSettingsTests: XCTestCase { private let suiteName = "cmux.tests.commandPaletteRenameSelection.\(UUID().uuidString)" @@ -6012,3 +6166,32 @@ final class TerminalControllerSocketTextChunkTests: XCTestCase { ) } } + +final class BrowserOmnibarFocusPolicyTests: XCTestCase { + func testReacquiresFocusWhenWebViewSuppressionIsActiveAndNextResponderIsNotAnotherTextField() { + XCTAssertTrue( + browserOmnibarShouldReacquireFocusAfterEndEditing( + suppressWebViewFocus: true, + nextResponderIsOtherTextField: false + ) + ) + } + + func testDoesNotReacquireFocusWhenAnotherTextFieldAlreadyTookFocus() { + XCTAssertFalse( + browserOmnibarShouldReacquireFocusAfterEndEditing( + suppressWebViewFocus: true, + nextResponderIsOtherTextField: true + ) + ) + } + + func testDoesNotReacquireFocusWhenWebViewSuppressionIsInactive() { + XCTAssertFalse( + browserOmnibarShouldReacquireFocusAfterEndEditing( + suppressWebViewFocus: false, + nextResponderIsOtherTextField: false + ) + ) + } +}