Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 24 additions & 36 deletions Sources/AppDelegate.swift
Original file line number Diff line number Diff line change
Expand Up @@ -5496,32 +5496,6 @@ final class AppDelegate: NSObject, NSApplicationDelegate, UNUserNotificationCent
return false
}

@discardableResult
func toggleRightSidebarInActiveMainWindow(preferredWindow: NSWindow? = nil) -> Bool {
guard let context = preferredRegisteredMainWindowContext(preferredWindow: preferredWindow) else {
if let fileExplorerState {
fileExplorerState.toggle()
return true
}
return false
}

let window = context.window ?? windowForMainWindowId(context.windowId)
if let window {
setActiveMainWindow(window)
}

guard let state = context.fileExplorerState ?? fileExplorerState else {
return false
}
let wasVisible = state.isVisible
state.toggle()
if wasVisible && !state.isVisible {
_ = context.keyboardFocusCoordinator.restoreTerminalFocusAfterRightSidebarHiddenIfNeeded()
}
return true
}

@discardableResult
func restoreTerminalFocusAfterRightSidebarHidden(in window: NSWindow?) -> Bool {
let context = preferredRegisteredMainWindowContext(preferredWindow: window)
Expand Down Expand Up @@ -5823,13 +5797,13 @@ final class AppDelegate: NSObject, NSApplicationDelegate, UNUserNotificationCent
}

@discardableResult
func toggleRightSidebarKeyboardFocusInActiveMainWindow(preferredWindow: NSWindow? = nil) -> Bool {
func toggleRightSidebarVisibilityInActiveMainWindow(preferredWindow: NSWindow? = nil) -> Bool {
let context = preferredRegisteredMainWindowContext(preferredWindow: preferredWindow)

guard let context else {
#if DEBUG
dlog(
"rs.focus.toggle.abort reason=noContext preferred={\(debugWindowToken(preferredWindow))} " +
"rs.visibility.toggle.abort reason=noContext preferred={\(debugWindowToken(preferredWindow))} " +
"\(debugShortcutRouteSnapshot())"
)
#endif
Expand All @@ -5839,19 +5813,29 @@ final class AppDelegate: NSObject, NSApplicationDelegate, UNUserNotificationCent
#if DEBUG
let beforeResponder = window?.firstResponder.map { String(describing: type(of: $0)) } ?? "nil"
dlog(
"rs.focus.toggle.begin preferred={\(debugWindowToken(preferredWindow))} " +
"rs.visibility.toggle.begin preferred={\(debugWindowToken(preferredWindow))} " +
"context={\(debugContextToken(context))} targetWin={\(debugWindowToken(window))} " +
"fr=\(beforeResponder)"
)
#endif
guard let state = context.fileExplorerState ?? fileExplorerState else {
return false
}

if state.isVisible {
state.setVisible(false)
_ = context.keyboardFocusCoordinator.restoreTerminalFocusAfterRightSidebarHiddenIfNeeded()
return true
}

if let window {
mainWindowVisibilityController.focusForInWindowCommand(window, reason: .rightSidebarToggle)
}
let result = context.keyboardFocusCoordinator.toggleRightSidebarOrTerminalFocus()
let result = context.keyboardFocusCoordinator.focusRightSidebar()
#if DEBUG
let afterResponder = window?.firstResponder.map { String(describing: type(of: $0)) } ?? "nil"
dlog(
"rs.focus.toggle.end result=\(result ? 1 : 0) " +
"rs.visibility.toggle.end result=\(result ? 1 : 0) " +
"targetWin={\(debugWindowToken(window))} fr=\(afterResponder)"
)
#endif
Expand Down Expand Up @@ -11105,11 +11089,15 @@ final class AppDelegate: NSObject, NSApplicationDelegate, UNUserNotificationCent

if matchConfiguredShortcut(event: event, action: .toggleFileExplorer) {
// Escape AppKit's performKeyEquivalent animation context. Without
// deferring the toggle, NSAnimationContext implicitly animates the
// deferring the reveal, NSAnimationContext implicitly animates the
// layout change.
let preferredWindow = mainWindowForShortcutEvent(event) ?? event.window ?? NSApp.keyWindow ?? NSApp.mainWindow
Task { @MainActor [weak self, weak preferredWindow] in
_ = self?.toggleRightSidebarInActiveMainWindow(preferredWindow: preferredWindow)
_ = self?.focusRightSidebarInActiveMainWindow(
mode: .files,
focusFirstItem: true,
preferredWindow: preferredWindow
)
}
return true
}
Expand All @@ -11121,18 +11109,18 @@ final class AppDelegate: NSObject, NSApplicationDelegate, UNUserNotificationCent
?? NSApp.keyWindow?.firstResponder
?? NSApp.mainWindow?.firstResponder
dlog(
"rs.focus.toggle.shortcut.begin event=\(NSWindow.keyDescription(event)) " +
"rs.visibility.toggle.shortcut.begin event=\(NSWindow.keyDescription(event)) " +
"preferred={\(debugWindowToken(preferredWindow))} fr=\(beforeResponder.map { String(describing: type(of: $0)) } ?? "nil") " +
"\(debugShortcutRouteSnapshot(event: event))"
)
#endif
let result = toggleRightSidebarKeyboardFocusInActiveMainWindow(preferredWindow: preferredWindow)
let result = toggleRightSidebarVisibilityInActiveMainWindow(preferredWindow: preferredWindow)
#if DEBUG
let afterResponder = preferredWindow?.firstResponder
?? NSApp.keyWindow?.firstResponder
?? NSApp.mainWindow?.firstResponder
dlog(
"rs.focus.toggle.shortcut.end result=\(result ? 1 : 0) " +
"rs.visibility.toggle.shortcut.end result=\(result ? 1 : 0) " +
"preferred={\(debugWindowToken(preferredWindow))} fr=\(afterResponder.map { String(describing: type(of: $0)) } ?? "nil") " +
"\(debugShortcutRouteSnapshot(event: event))"
)
Expand Down
2 changes: 1 addition & 1 deletion Sources/cmuxApp.swift
Original file line number Diff line number Diff line change
Expand Up @@ -645,7 +645,7 @@ struct cmuxApp: App {
}

splitCommandButton(title: String(localized: "menu.view.focusRightSidebar", defaultValue: "Toggle Right Sidebar"), shortcut: menuShortcut(for: .focusRightSidebar)) {
if AppDelegate.shared?.toggleRightSidebarKeyboardFocusInActiveMainWindow() != true {
if AppDelegate.shared?.toggleRightSidebarVisibilityInActiveMainWindow() != true {
if AppDelegate.shared?.focusRightSidebarInActiveMainWindow(
preferredWindow: NSApp.keyWindow ?? NSApp.mainWindow
) != true {
Expand Down
125 changes: 125 additions & 0 deletions cmuxTests/AppDelegateShortcutRoutingTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -4484,6 +4484,131 @@ final class AppDelegateShortcutRoutingTests: XCTestCase {
}
}

func testFocusRightSidebarShortcutTogglesRightSidebarVisibility() {
guard let appDelegate = AppDelegate.shared else {
XCTFail("Expected AppDelegate.shared")
return
}

let originalVisibility = UserDefaults.standard.object(forKey: "fileExplorer.isVisible")
defer {
restoreDefaultsValue(originalVisibility, forKey: "fileExplorer.isVisible", defaults: .standard)
}

let windowId = appDelegate.createMainWindow()
defer { closeWindow(withId: windowId) }

guard let window = window(withId: windowId),
let state = appDelegate.fileExplorerState else {
XCTFail("Expected test window and right sidebar state")
return
}

window.makeKeyAndOrderFront(nil)
RunLoop.main.run(until: Date(timeIntervalSinceNow: 0.05))
Comment on lines +4507 to +4508

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Test covers only two of three sidebar states

testFocusRightSidebarShortcutTogglesRightSidebarVisibility always begins with state.setVisible(false) before driving the shortcut, so it verifies the "hidden → show" and "visible → hide" transitions only. The previous toggleRightSidebarOrTerminalFocus() had a third state: sidebar visible with keyboard focus on the terminal, where the old behavior would have moved focus to the sidebar. The new toggleRightSidebarVisibilityInActiveMainWindow unconditionally hides the sidebar in that state instead — a user-visible change that has no test coverage. If that semantic change is intentional, a comment or a third test case would prevent a future accidental reversion.

state.setVisible(false)

guard let showEvent = makeKeyDownEvent(
key: "b",
modifiers: [.command, .option],
keyCode: 11,
windowNumber: window.windowNumber
), let hideEvent = makeKeyDownEvent(
key: "b",
modifiers: [.command, .option],
keyCode: 11,
windowNumber: window.windowNumber
) else {
XCTFail("Failed to construct right sidebar toggle shortcut events")
return
}

#if DEBUG
XCTAssertTrue(appDelegate.debugHandleCustomShortcut(event: showEvent))
#else
XCTFail("debugHandleCustomShortcut is only available in DEBUG")
#endif
XCTAssertTrue(state.isVisible, "First shortcut press should show the right sidebar")

#if DEBUG
XCTAssertTrue(appDelegate.debugHandleCustomShortcut(event: hideEvent))
#else
XCTFail("debugHandleCustomShortcut is only available in DEBUG")
#endif
XCTAssertFalse(state.isVisible, "Second shortcut press should hide the right sidebar")
}

func testOpenFileExplorerShortcutSelectsFilesModeWhenFindWasActive() {
guard let appDelegate = AppDelegate.shared else {
XCTFail("Expected AppDelegate.shared")
return
}

let originalVisibility = UserDefaults.standard.object(forKey: "fileExplorer.isVisible")
let originalMode = UserDefaults.standard.object(forKey: "rightSidebar.mode")
defer {
restoreDefaultsValue(originalVisibility, forKey: "fileExplorer.isVisible", defaults: .standard)
restoreDefaultsValue(originalMode, forKey: "rightSidebar.mode", defaults: .standard)
}

let windowId = appDelegate.createMainWindow()
defer { closeWindow(withId: windowId) }

guard let window = window(withId: windowId),
let state = appDelegate.fileExplorerState else {
XCTFail("Expected test window and right sidebar state")
return
}

window.makeKeyAndOrderFront(nil)
RunLoop.main.run(until: Date(timeIntervalSinceNow: 0.05))
state.mode = .find
state.setVisible(false)

guard let hiddenEvent = makeKeyDownEvent(
key: "e",
modifiers: [.command, .shift],
keyCode: 14,
windowNumber: window.windowNumber
) else {
XCTFail("Failed to construct Open File Explorer shortcut event")
return
}

#if DEBUG
XCTAssertTrue(appDelegate.debugHandleCustomShortcut(event: hiddenEvent))
#else
XCTFail("debugHandleCustomShortcut is only available in DEBUG")
#endif
RunLoop.main.run(until: Date(timeIntervalSinceNow: 0.05))

XCTAssertTrue(state.isVisible, "Open File Explorer should reveal the right sidebar")
XCTAssertEqual(state.mode, .files, "Open File Explorer must select Files, not the previously active Find mode")

state.mode = .find
state.setVisible(true)

guard let visibleEvent = makeKeyDownEvent(
key: "e",
modifiers: [.command, .shift],
keyCode: 14,
windowNumber: window.windowNumber
) else {
XCTFail("Failed to construct second Open File Explorer shortcut event")
return
}

#if DEBUG
XCTAssertTrue(appDelegate.debugHandleCustomShortcut(event: visibleEvent))
#else
XCTFail("debugHandleCustomShortcut is only available in DEBUG")
#endif
RunLoop.main.run(until: Date(timeIntervalSinceNow: 0.05))

XCTAssertTrue(state.isVisible, "Open File Explorer should keep the right sidebar open")
XCTAssertEqual(state.mode, .files, "Open File Explorer should switch an already visible right sidebar to Files")
}

func testBrowserFindCommandPreflightConsultsConfiguredFindFamilyShortcuts() {
#if DEBUG
let cases: [(action: KeyboardShortcutSettings.Action, modifiers: NSEvent.ModifierFlags, key: String, keyCode: UInt16)] = [
Expand Down
Loading