From ec6d8cec5406c36252e439f782a8910b7a25f658 Mon Sep 17 00:00:00 2001 From: Leo Li Date: Wed, 28 Oct 2026 13:50:14 -0700 Subject: [PATCH] fix(next palette): the focus overlay goes on the window the palette opens over The palette panel is a child of the window it opens over, but its focus overlay went on `windows.active`: the window an input event came from, else the key window's, else the last active one. Those disagree with the panel's parent whenever the palette opens over a window that is not the active one (an explicit `relativeTo:`, or a press in one window while another holds the keys). The overlay then landed on a window the palette was not over, and the window under the panel kept an empty overlay stack, so it went on treating its terminal as the keyboard target (input-spec.md W5, W6, F1). The palette now reports the window it parented to, and the app opens the overlay on that window's controller. Co-Authored-By: Claude Opus 5 Agent-Session: https://cmux.com/deeplink/agent?id=aid-c8241b2aa98359e57bc6 --- .../CmuxNextApp/Focus/AppServices+Focus.swift | 19 +++++++-- .../CmuxNextPalette/PaletteController.swift | 10 +++-- .../PaletteOverlayWindowTests.swift | 42 +++++++++++++++++++ 3 files changed, 64 insertions(+), 7 deletions(-) create mode 100644 Packages/macOS/CmuxNext/Tests/CmuxNextAppTests/PaletteOverlayWindowTests.swift diff --git a/Packages/macOS/CmuxNext/Sources/CmuxNextApp/Focus/AppServices+Focus.swift b/Packages/macOS/CmuxNext/Sources/CmuxNextApp/Focus/AppServices+Focus.swift index 8df58528f4b6..fd812aa57d10 100644 --- a/Packages/macOS/CmuxNext/Sources/CmuxNextApp/Focus/AppServices+Focus.swift +++ b/Packages/macOS/CmuxNext/Sources/CmuxNextApp/Focus/AppServices+Focus.swift @@ -5,15 +5,26 @@ import CmuxNextFocus // App-wide inputs to the per-window focus coordinators. extension AppServices { - /// The palette is a panel above the active window: an overlay of that - /// window's focus while it is open. Reported synchronously by the + /// The palette is a panel above the window it opens over: an overlay of + /// that window's focus while it is open. Reported synchronously by the /// palette, before the key-window change it causes, so a click that /// closes it is judged with the overlay already gone (input-spec.md B7). + /// + /// The overlay goes on the window the panel is a child of, not on + /// `windows.active`: the two disagree whenever the palette opens over a + /// window that is not the active one (an explicit `relativeTo:`, or a + /// press in one window while another holds the keys, where `active` is + /// the event's window and the palette parents to the key window). The + /// overlay then landed on a window the palette was not over, leaving the + /// window under the panel with an empty stack: it goes on applying focus + /// to its terminal while the palette holds the keyboard (input-spec.md + /// W5, W6, and F1's "an overlay is the keyboard target"). func observePaletteForFocus() { - palette.onVisibilityChange = { [weak self] open in + palette.onVisibilityChange = { [weak self] open, host in guard let self else { return } if open { - windows.active?.focus.send(.overlayOpened(.palette)) + let over = host.flatMap { windows.owner(of: $0) } ?? windows.active + over?.focus.send(.overlayOpened(.palette)) } else { for controller in windows.controllers where controller.focus.state.overlays.contains(.palette) { controller.focus.send(.overlayClosed(.palette)) diff --git a/Packages/macOS/CmuxNext/Sources/CmuxNextPalette/PaletteController.swift b/Packages/macOS/CmuxNext/Sources/CmuxNextPalette/PaletteController.swift index 84daba606144..8c367399c3a3 100644 --- a/Packages/macOS/CmuxNext/Sources/CmuxNextPalette/PaletteController.swift +++ b/Packages/macOS/CmuxNext/Sources/CmuxNextPalette/PaletteController.swift @@ -30,7 +30,11 @@ public final class PaletteController { /// Called synchronously when the panel opens (true) or closes (false), /// before any key-window change it causes, so the owner's focus overlay /// stack never lags the panel (plans/cmux-next/input-spec.md bug B7). - public var onVisibilityChange: ((Bool) -> Void)? + /// The window is the one the panel is a child of, so the owner puts the + /// overlay on the window the palette is actually over (W5, W6) instead + /// of guessing it from the active window, which the palette's own + /// `relativeTo:` and the key window can both disagree with. + public var onVisibilityChange: ((Bool, NSWindow?) -> Void)? /// Main-thread phases of each open, request to committed first frame /// (the stall bench, `debug.timings`). public var onPresented: ((PaletteOpenTiming) -> Void)? @@ -272,8 +276,8 @@ public final class PaletteController { return } isVisible = true - onVisibilityChange?(true) parentWindow = parent + onVisibilityChange?(true, parent) // The room (theme) of the window it opens over. (parent?.themeScope ?? .app).adopt(panel) let size = PaletteLayout.windowSize @@ -331,7 +335,7 @@ public final class PaletteController { guard isVisible, let panel else { return } isVisible = false registry.context.remove(.paletteOpen) - onVisibilityChange?(false) + onVisibilityChange?(false, parentWindow) model.closeActionsMenu() shortcutRecorder.abandon() model.shortcutRecorder = nil diff --git a/Packages/macOS/CmuxNext/Tests/CmuxNextAppTests/PaletteOverlayWindowTests.swift b/Packages/macOS/CmuxNext/Tests/CmuxNextAppTests/PaletteOverlayWindowTests.swift new file mode 100644 index 000000000000..cd1c305e7c3e --- /dev/null +++ b/Packages/macOS/CmuxNext/Tests/CmuxNextAppTests/PaletteOverlayWindowTests.swift @@ -0,0 +1,42 @@ +import AppKit +@testable import CmuxNextApp +import CmuxNextFocus +import Testing + +/// The palette's focus overlay goes on the window the panel opens over +/// (#4303: "the palette UI is invoked, but the next character typed lands in +/// the terminal"). The overlay used to go on `windows.active`, which is the +/// window an input event came from, else the key window's, else the last +/// active one: none of those is the window the palette parents to when the +/// two disagree. The overlay then sat on a window the palette was not over, +/// and the window under the panel kept an empty overlay stack, so it went on +/// treating its terminal as the keyboard target (input-spec.md W5, W6, F1). +@MainActor @Suite(.serialized) +struct PaletteOverlayWindowTests { + @Test func theOverlayGoesOnTheWindowThePaletteOpensOver() throws { + _ = NSApplication.shared + let services = AppServices(environment: AppEnvironment.current([:])) + services.windows.ordersWindowsIn = false + let over = try #require(services.windows.openWindow(workspaces: ["palette-overlay-over"])) + let elsewhere = try #require(services.windows.openWindow(workspaces: ["palette-overlay-elsewhere"])) + defer { + services.palette.hide() + for controller in services.windows.controllers { controller.window?.close() } + } + let overWindow = try #require(over.window) + // The active window is the other one, as it is for a press in one + // window while another holds the keys. + services.keyWindowSource = { elsewhere.window } + try #require(services.windows.active === elsewhere, "the active window is not the other one") + + services.palette.show(.commands, relativeTo: overWindow) + + #expect(over.focus.state.overlays.contains(.palette), + "the window the palette opened over has no palette overlay") + #expect(!elsewhere.focus.state.overlays.contains(.palette), + "the palette overlay landed on a window the palette is not over") + + services.palette.hide() + #expect(!over.focus.state.overlays.contains(.palette), "the overlay outlived the palette") + } +}