Fix Cmd+P/Cmd+Shift+P window routing (#413)
* Fix command palette shortcuts to stay window-scoped * Fix cross-window command palette typing focus lock
This commit is contained in:
parent
30398a0af9
commit
3c1650d3e0
3 changed files with 191 additions and 11 deletions
|
|
@ -1535,6 +1535,13 @@ final class AppDelegate: NSObject, NSApplicationDelegate, UNUserNotificationCent
|
|||
return nil
|
||||
}
|
||||
|
||||
private func commandPaletteWindowForShortcutEvent(_ event: NSEvent) -> NSWindow? {
|
||||
if let scopedWindow = mainWindowForShortcutEvent(event) {
|
||||
return scopedWindow
|
||||
}
|
||||
return activeCommandPaletteWindow()
|
||||
}
|
||||
|
||||
private func contextForMainWindow(_ window: NSWindow?) -> MainWindowContext? {
|
||||
guard let window, isMainTerminalWindow(window) else { return nil }
|
||||
return mainWindowContexts[ObjectIdentifier(window)]
|
||||
|
|
@ -2950,13 +2957,18 @@ final class AppDelegate: NSObject, NSApplicationDelegate, UNUserNotificationCent
|
|||
}
|
||||
|
||||
let normalizedFlags = flags.subtracting([.numericPad, .function, .capsLock])
|
||||
let commandPaletteTargetWindow = commandPaletteWindowForShortcutEvent(event)
|
||||
let commandPaletteVisibleInTargetWindow = commandPaletteTargetWindow.map {
|
||||
isCommandPaletteVisible(for: $0)
|
||||
} ?? false
|
||||
|
||||
if let delta = commandPaletteSelectionDeltaForKeyboardNavigation(
|
||||
flags: event.modifierFlags,
|
||||
chars: chars,
|
||||
keyCode: event.keyCode
|
||||
),
|
||||
let paletteWindow = activeCommandPaletteWindow() {
|
||||
commandPaletteVisibleInTargetWindow,
|
||||
let paletteWindow = commandPaletteTargetWindow {
|
||||
NotificationCenter.default.post(
|
||||
name: .commandPaletteMoveSelection,
|
||||
object: paletteWindow,
|
||||
|
|
@ -2967,20 +2979,20 @@ final class AppDelegate: NSObject, NSApplicationDelegate, UNUserNotificationCent
|
|||
|
||||
let isCommandP = normalizedFlags == [.command] && (chars == "p" || event.keyCode == 35)
|
||||
if isCommandP {
|
||||
let targetWindow = activeCommandPaletteWindow() ?? event.window ?? NSApp.keyWindow ?? NSApp.mainWindow
|
||||
let targetWindow = commandPaletteTargetWindow ?? event.window ?? NSApp.keyWindow ?? NSApp.mainWindow
|
||||
NotificationCenter.default.post(name: .commandPaletteSwitcherRequested, object: targetWindow)
|
||||
return true
|
||||
}
|
||||
|
||||
let isCommandShiftP = normalizedFlags == [.command, .shift] && (chars == "p" || event.keyCode == 35)
|
||||
if isCommandShiftP {
|
||||
let targetWindow = activeCommandPaletteWindow() ?? event.window ?? NSApp.keyWindow ?? NSApp.mainWindow
|
||||
let targetWindow = commandPaletteTargetWindow ?? event.window ?? NSApp.keyWindow ?? NSApp.mainWindow
|
||||
NotificationCenter.default.post(name: .commandPaletteRequested, object: targetWindow)
|
||||
return true
|
||||
}
|
||||
|
||||
if shouldConsumeShortcutWhileCommandPaletteVisible(
|
||||
isCommandPaletteVisible: activeCommandPaletteWindow() != nil,
|
||||
isCommandPaletteVisible: commandPaletteVisibleInTargetWindow,
|
||||
normalizedFlags: normalizedFlags,
|
||||
chars: chars,
|
||||
keyCode: event.keyCode
|
||||
|
|
@ -3184,7 +3196,7 @@ final class AppDelegate: NSObject, NSApplicationDelegate, UNUserNotificationCent
|
|||
if tabManager?.focusedBrowserPanel != nil {
|
||||
return false
|
||||
}
|
||||
let targetWindow = activeCommandPaletteWindow() ?? event.window ?? NSApp.keyWindow ?? NSApp.mainWindow
|
||||
let targetWindow = commandPaletteTargetWindow ?? event.window ?? NSApp.keyWindow ?? NSApp.mainWindow
|
||||
NotificationCenter.default.post(name: .commandPaletteRenameTabRequested, object: targetWindow)
|
||||
return true
|
||||
}
|
||||
|
|
|
|||
|
|
@ -753,6 +753,9 @@ private final class WindowCommandPaletteOverlayController: NSObject {
|
|||
private weak var installedThemeFrame: NSView?
|
||||
private var focusLockTimer: DispatchSourceTimer?
|
||||
private var scheduledFocusWorkItem: DispatchWorkItem?
|
||||
private var isPaletteVisible = false
|
||||
private var windowDidBecomeKeyObserver: NSObjectProtocol?
|
||||
private var windowDidResignKeyObserver: NSObjectProtocol?
|
||||
|
||||
init(window: NSWindow) {
|
||||
self.window = window
|
||||
|
|
@ -775,6 +778,7 @@ private final class WindowCommandPaletteOverlayController: NSObject {
|
|||
hostingView.trailingAnchor.constraint(equalTo: containerView.trailingAnchor),
|
||||
])
|
||||
_ = ensureInstalled()
|
||||
installWindowKeyObservers()
|
||||
}
|
||||
|
||||
@discardableResult
|
||||
|
|
@ -906,6 +910,52 @@ private final class WindowCommandPaletteOverlayController: NSObject {
|
|||
}
|
||||
}
|
||||
|
||||
private func installWindowKeyObservers() {
|
||||
guard let window else { return }
|
||||
windowDidBecomeKeyObserver = NotificationCenter.default.addObserver(
|
||||
forName: NSWindow.didBecomeKeyNotification,
|
||||
object: window,
|
||||
queue: .main
|
||||
) { [weak self] _ in
|
||||
Task { @MainActor [weak self] in
|
||||
self?.updateFocusLockForWindowState()
|
||||
}
|
||||
}
|
||||
windowDidResignKeyObserver = NotificationCenter.default.addObserver(
|
||||
forName: NSWindow.didResignKeyNotification,
|
||||
object: window,
|
||||
queue: .main
|
||||
) { [weak self] _ in
|
||||
Task { @MainActor [weak self] in
|
||||
self?.updateFocusLockForWindowState()
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
private func updateFocusLockForWindowState() {
|
||||
guard let window else {
|
||||
stopFocusLockTimer()
|
||||
return
|
||||
}
|
||||
guard isPaletteVisible else {
|
||||
stopFocusLockTimer()
|
||||
return
|
||||
}
|
||||
|
||||
guard window.isKeyWindow else {
|
||||
stopFocusLockTimer()
|
||||
if isPaletteResponder(window.firstResponder) {
|
||||
_ = window.makeFirstResponder(nil)
|
||||
}
|
||||
return
|
||||
}
|
||||
|
||||
startFocusLockTimer()
|
||||
if !isPaletteTextInputFirstResponder(window.firstResponder) {
|
||||
scheduleFocusIntoPalette(retries: 8)
|
||||
}
|
||||
}
|
||||
|
||||
private func startFocusLockTimer() {
|
||||
guard focusLockTimer == nil else { return }
|
||||
let timer = DispatchSource.makeTimerSource(queue: .main)
|
||||
|
|
@ -952,6 +1002,7 @@ private final class WindowCommandPaletteOverlayController: NSObject {
|
|||
|
||||
func update(rootView: AnyView, isVisible: Bool) {
|
||||
guard ensureInstalled() else { return }
|
||||
isPaletteVisible = isVisible
|
||||
if isVisible {
|
||||
hostingView.rootView = rootView
|
||||
containerView.capturesMouseEvents = true
|
||||
|
|
@ -960,10 +1011,7 @@ private final class WindowCommandPaletteOverlayController: NSObject {
|
|||
if let themeFrame = installedThemeFrame, themeFrame.subviews.last !== containerView {
|
||||
themeFrame.addSubview(containerView, positioned: .above, relativeTo: nil)
|
||||
}
|
||||
startFocusLockTimer()
|
||||
if let window, !isPaletteTextInputFirstResponder(window.firstResponder) {
|
||||
scheduleFocusIntoPalette(retries: 8)
|
||||
}
|
||||
updateFocusLockForWindowState()
|
||||
} else {
|
||||
stopFocusLockTimer()
|
||||
if let window, isPaletteResponder(window.firstResponder) {
|
||||
|
|
|
|||
|
|
@ -32,6 +32,10 @@ def _palette_visible(client: cmux, window_id: str) -> bool:
|
|||
return bool(res.get("visible"))
|
||||
|
||||
|
||||
def _palette_results(client: cmux, window_id: str, limit: int = 20) -> dict:
|
||||
return client.command_palette_results(window_id=window_id, limit=limit)
|
||||
|
||||
|
||||
def _set_palette_visible(client: cmux, window_id: str, visible: bool) -> None:
|
||||
if _palette_visible(client, window_id) == visible:
|
||||
return
|
||||
|
|
@ -43,6 +47,116 @@ def _set_palette_visible(client: cmux, window_id: str, visible: bool) -> None:
|
|||
)
|
||||
|
||||
|
||||
def _focus_window(client: cmux, window_id: str) -> None:
|
||||
client.focus_window(window_id)
|
||||
client.activate_app()
|
||||
_wait_until(
|
||||
lambda: client.current_window().lower() == window_id.lower(),
|
||||
timeout_s=3.0,
|
||||
message=f"failed to focus window {window_id}",
|
||||
)
|
||||
time.sleep(0.15)
|
||||
|
||||
|
||||
def _assert_shortcut_window_scoped(client: cmux, shortcut: str, w1: str, w2: str) -> None:
|
||||
_set_palette_visible(client, w1, False)
|
||||
_set_palette_visible(client, w2, False)
|
||||
|
||||
_focus_window(client, w1)
|
||||
client.simulate_shortcut(shortcut)
|
||||
_wait_until(
|
||||
lambda: _palette_visible(client, w1),
|
||||
timeout_s=3.0,
|
||||
message=f"{shortcut} did not open palette in window1",
|
||||
)
|
||||
if _palette_visible(client, w2):
|
||||
raise cmuxError(f"{shortcut} in window1 incorrectly opened palette in window2")
|
||||
|
||||
_focus_window(client, w2)
|
||||
client.simulate_shortcut(shortcut)
|
||||
_wait_until(
|
||||
lambda: _palette_visible(client, w2),
|
||||
timeout_s=3.0,
|
||||
message=f"{shortcut} did not open palette in window2",
|
||||
)
|
||||
if not _palette_visible(client, w1):
|
||||
raise cmuxError(
|
||||
f"{shortcut} in window2 incorrectly toggled window1 palette off "
|
||||
"(cross-window routing regression)"
|
||||
)
|
||||
|
||||
client.simulate_shortcut(shortcut)
|
||||
_wait_until(
|
||||
lambda: not _palette_visible(client, w2),
|
||||
timeout_s=3.0,
|
||||
message=f"second {shortcut} did not close palette in window2",
|
||||
)
|
||||
if not _palette_visible(client, w1):
|
||||
raise cmuxError(
|
||||
f"second {shortcut} in window2 incorrectly changed window1 palette visibility"
|
||||
)
|
||||
|
||||
_focus_window(client, w1)
|
||||
client.simulate_shortcut(shortcut)
|
||||
_wait_until(
|
||||
lambda: not _palette_visible(client, w1),
|
||||
timeout_s=3.0,
|
||||
message=f"second {shortcut} did not close palette in window1",
|
||||
)
|
||||
|
||||
|
||||
def _assert_cross_window_typing_after_mixed_shortcuts(client: cmux, w1: str, w2: str) -> None:
|
||||
_set_palette_visible(client, w1, False)
|
||||
_set_palette_visible(client, w2, False)
|
||||
|
||||
_focus_window(client, w1)
|
||||
client.simulate_shortcut("cmd+shift+p")
|
||||
_wait_until(
|
||||
lambda: _palette_visible(client, w1),
|
||||
timeout_s=3.0,
|
||||
message="cmd+shift+p did not open palette in window1",
|
||||
)
|
||||
_wait_until(
|
||||
lambda: str(_palette_results(client, w1).get("mode") or "") == "commands",
|
||||
timeout_s=3.0,
|
||||
message="window1 palette did not enter commands mode",
|
||||
)
|
||||
window1_query_before = str(_palette_results(client, w1).get("query") or "")
|
||||
|
||||
_focus_window(client, w2)
|
||||
client.simulate_shortcut("cmd+p")
|
||||
_wait_until(
|
||||
lambda: _palette_visible(client, w2),
|
||||
timeout_s=3.0,
|
||||
message="cmd+p did not open palette in window2",
|
||||
)
|
||||
_wait_until(
|
||||
lambda: str(_palette_results(client, w2).get("mode") or "") == "switcher",
|
||||
timeout_s=3.0,
|
||||
message="window2 palette did not enter switcher mode",
|
||||
)
|
||||
|
||||
typed = ""
|
||||
for ch in "crosswindow":
|
||||
typed += ch
|
||||
client.simulate_type(ch)
|
||||
_wait_until(
|
||||
lambda expected=typed: str(_palette_results(client, w2).get("query") or "").lower() == expected,
|
||||
timeout_s=1.8,
|
||||
message=(
|
||||
"typing into window2 palette did not accumulate query text "
|
||||
f"(expected {typed!r})"
|
||||
),
|
||||
)
|
||||
|
||||
window1_query_now = str(_palette_results(client, w1).get("query") or "")
|
||||
if window1_query_now != window1_query_before:
|
||||
raise cmuxError(
|
||||
"typing in window2 changed window1 command-palette query "
|
||||
f"(before={window1_query_before!r}, now={window1_query_now!r})"
|
||||
)
|
||||
|
||||
|
||||
def main() -> int:
|
||||
with cmux(SOCKET_PATH) as client:
|
||||
client.activate_app()
|
||||
|
|
@ -51,8 +165,8 @@ def main() -> int:
|
|||
w2 = client.new_window()
|
||||
time.sleep(0.25)
|
||||
|
||||
ws1 = client.new_workspace(window_id=w1)
|
||||
ws2 = client.new_workspace(window_id=w2)
|
||||
_ = client.new_workspace(window_id=w1)
|
||||
_ = client.new_workspace(window_id=w2)
|
||||
time.sleep(0.25)
|
||||
_set_palette_visible(client, w1, False)
|
||||
_set_palette_visible(client, w2, False)
|
||||
|
|
@ -91,6 +205,12 @@ def main() -> int:
|
|||
message="window2 command palette did not close",
|
||||
)
|
||||
|
||||
# Reproduce keyboard-shortcut window-scoping path:
|
||||
# opening from window2 must not jump back and toggle window1.
|
||||
_assert_shortcut_window_scoped(client, "cmd+shift+p", w1, w2)
|
||||
_assert_shortcut_window_scoped(client, "cmd+p", w1, w2)
|
||||
_assert_cross_window_typing_after_mixed_shortcuts(client, w1, w2)
|
||||
|
||||
print("PASS: command palette is scoped to active window")
|
||||
return 0
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue