Merge pull request #990 from manaflow-ai/issue-983-browser-unfocusable-after-cmdr
Fix browser focus after dismissing the command palette by click
This commit is contained in:
commit
138ed0e2f0
3 changed files with 261 additions and 4 deletions
|
|
@ -1112,6 +1112,23 @@ private final class WindowCommandPaletteOverlayController: NSObject {
|
|||
containerView.isHidden = true
|
||||
}
|
||||
}
|
||||
|
||||
func underlyingResponder(atWindowPoint windowPoint: NSPoint) -> NSResponder? {
|
||||
guard let window,
|
||||
let contentView = window.contentView,
|
||||
let themeFrame = contentView.superview else {
|
||||
return nil
|
||||
}
|
||||
|
||||
let previousCapturesMouseEvents = containerView.capturesMouseEvents
|
||||
containerView.capturesMouseEvents = false
|
||||
defer {
|
||||
containerView.capturesMouseEvents = previousCapturesMouseEvents
|
||||
}
|
||||
|
||||
let pointInTheme = themeFrame.convert(windowPoint, from: nil)
|
||||
return themeFrame.hitTest(pointInTheme)
|
||||
}
|
||||
}
|
||||
|
||||
@MainActor
|
||||
|
|
@ -1124,6 +1141,40 @@ private func commandPaletteWindowOverlayController(for window: NSWindow) -> Wind
|
|||
return controller
|
||||
}
|
||||
|
||||
private func commandPaletteOwningWebView(for responder: NSResponder?) -> WKWebView? {
|
||||
guard let responder else { return nil }
|
||||
|
||||
if let webView = responder as? WKWebView {
|
||||
return webView
|
||||
}
|
||||
|
||||
if let view = responder as? NSView {
|
||||
var current: NSView? = view
|
||||
while let candidate = current {
|
||||
if let webView = candidate as? WKWebView {
|
||||
return webView
|
||||
}
|
||||
current = candidate.superview
|
||||
}
|
||||
}
|
||||
|
||||
if let textView = responder as? NSTextView,
|
||||
let delegateView = textView.delegate as? NSView,
|
||||
let webView = commandPaletteOwningWebView(for: delegateView) {
|
||||
return webView
|
||||
}
|
||||
|
||||
var currentResponder = responder.nextResponder
|
||||
while let next = currentResponder {
|
||||
if let webView = commandPaletteOwningWebView(for: next) {
|
||||
return webView
|
||||
}
|
||||
currentResponder = next.nextResponder
|
||||
}
|
||||
|
||||
return nil
|
||||
}
|
||||
|
||||
enum WorkspaceMountPolicy {
|
||||
// Keep only the selected workspace mounted to minimize layer-tree traversal.
|
||||
static let maxMountedWorkspaces = 1
|
||||
|
|
@ -2711,9 +2762,18 @@ struct ContentView: View {
|
|||
Color.clear
|
||||
.ignoresSafeArea()
|
||||
.contentShape(Rectangle())
|
||||
.onTapGesture {
|
||||
dismissCommandPalette()
|
||||
}
|
||||
.gesture(
|
||||
DragGesture(minimumDistance: 0)
|
||||
.onEnded { value in
|
||||
handleCommandPaletteBackdropClick(atContentPoint: value.location)
|
||||
}
|
||||
)
|
||||
|
||||
Color.clear
|
||||
.ignoresSafeArea()
|
||||
.contentShape(Rectangle())
|
||||
.allowsHitTesting(false)
|
||||
.accessibilityIdentifier("CommandPaletteBackdrop")
|
||||
|
||||
VStack(spacing: 0) {
|
||||
switch commandPaletteMode {
|
||||
|
|
@ -2762,6 +2822,7 @@ struct ContentView: View {
|
|||
.font(.system(size: 13, weight: .regular))
|
||||
.tint(Color(nsColor: sidebarActiveForegroundNSColor(opacity: 1.0)))
|
||||
.focused($isCommandPaletteSearchFocused)
|
||||
.accessibilityIdentifier("CommandPaletteSearchField")
|
||||
.onSubmit {
|
||||
runSelectedCommandPaletteResult(visibleResults: visibleResults)
|
||||
}
|
||||
|
|
@ -2915,6 +2976,7 @@ struct ContentView: View {
|
|||
.font(.system(size: 13, weight: .regular))
|
||||
.tint(Color(nsColor: sidebarActiveForegroundNSColor(opacity: 1.0)))
|
||||
.focused($isCommandPaletteRenameFocused)
|
||||
.accessibilityIdentifier("CommandPaletteRenameField")
|
||||
.backport.onKeyPress(.delete) { modifiers in
|
||||
handleCommandPaletteRenameDeleteBackward(modifiers: modifiers)
|
||||
}
|
||||
|
|
@ -4681,7 +4743,14 @@ struct ContentView: View {
|
|||
}
|
||||
|
||||
private func dismissCommandPalette(restoreFocus: Bool = true) {
|
||||
let focusTarget = commandPaletteRestoreFocusTarget
|
||||
dismissCommandPalette(restoreFocus: restoreFocus, preferredFocusTarget: nil)
|
||||
}
|
||||
|
||||
private func dismissCommandPalette(
|
||||
restoreFocus: Bool,
|
||||
preferredFocusTarget: CommandPaletteRestoreFocusTarget?
|
||||
) {
|
||||
let focusTarget = preferredFocusTarget ?? commandPaletteRestoreFocusTarget
|
||||
isCommandPalettePresented = false
|
||||
commandPaletteMode = .commands
|
||||
commandPaletteQuery = ""
|
||||
|
|
@ -4702,6 +4771,117 @@ struct ContentView: View {
|
|||
restoreCommandPaletteFocus(target: focusTarget, attemptsRemaining: 6)
|
||||
}
|
||||
|
||||
private func handleCommandPaletteBackdropClick(atContentPoint contentPoint: CGPoint) {
|
||||
let clickedFocusTarget = commandPaletteBackdropFocusTarget(atContentPoint: contentPoint)
|
||||
#if DEBUG
|
||||
if let clickedFocusTarget {
|
||||
dlog(
|
||||
"palette.dismiss.backdrop focusTarget panel=\(clickedFocusTarget.panelId.uuidString.prefix(5)) " +
|
||||
"workspace=\(clickedFocusTarget.workspaceId.uuidString.prefix(5)) intent=\(clickedFocusTarget.intent == .browserAddressBar ? "addressBar" : "panel")"
|
||||
)
|
||||
} else {
|
||||
dlog("palette.dismiss.backdrop focusTarget=nil")
|
||||
}
|
||||
#endif
|
||||
dismissCommandPalette(restoreFocus: true, preferredFocusTarget: clickedFocusTarget)
|
||||
}
|
||||
|
||||
private func commandPaletteBackdropFocusTarget(atContentPoint contentPoint: CGPoint) -> CommandPaletteRestoreFocusTarget? {
|
||||
guard let window = observedWindow,
|
||||
let contentView = window.contentView else {
|
||||
return nil
|
||||
}
|
||||
|
||||
let nsContentPoint = NSPoint(x: contentPoint.x, y: contentPoint.y)
|
||||
let windowPoint = contentView.convert(nsContentPoint, to: nil)
|
||||
return commandPaletteBackdropFocusTarget(atWindowPoint: windowPoint, in: window)
|
||||
}
|
||||
|
||||
private func commandPaletteBackdropFocusTarget(
|
||||
atWindowPoint windowPoint: NSPoint,
|
||||
in window: NSWindow
|
||||
) -> CommandPaletteRestoreFocusTarget? {
|
||||
let overlayController = commandPaletteWindowOverlayController(for: window)
|
||||
if let responder = overlayController.underlyingResponder(atWindowPoint: windowPoint),
|
||||
let target = commandPaletteBackdropFocusTarget(for: responder) {
|
||||
return target
|
||||
}
|
||||
|
||||
if let webView = BrowserWindowPortalRegistry.webViewAtWindowPoint(windowPoint, in: window),
|
||||
let target = commandPaletteBrowserFocusTarget(for: webView) {
|
||||
return target
|
||||
}
|
||||
|
||||
if let terminalView = TerminalWindowPortalRegistry.terminalViewAtWindowPoint(windowPoint, in: window),
|
||||
let workspaceId = terminalView.tabId,
|
||||
let panelId = terminalView.terminalSurface?.id,
|
||||
tabManager.tabs.contains(where: { $0.id == workspaceId }) {
|
||||
return CommandPaletteRestoreFocusTarget(
|
||||
workspaceId: workspaceId,
|
||||
panelId: panelId,
|
||||
intent: .panel
|
||||
)
|
||||
}
|
||||
|
||||
return nil
|
||||
}
|
||||
|
||||
private func commandPaletteBackdropFocusTarget(for responder: NSResponder) -> CommandPaletteRestoreFocusTarget? {
|
||||
if let terminalView = cmuxOwningGhosttyView(for: responder),
|
||||
let workspaceId = terminalView.tabId,
|
||||
let panelId = terminalView.terminalSurface?.id,
|
||||
tabManager.tabs.contains(where: { $0.id == workspaceId }) {
|
||||
return CommandPaletteRestoreFocusTarget(
|
||||
workspaceId: workspaceId,
|
||||
panelId: panelId,
|
||||
intent: .panel
|
||||
)
|
||||
}
|
||||
|
||||
if let webView = commandPaletteOwningWebView(for: responder),
|
||||
let target = commandPaletteBrowserFocusTarget(for: webView) {
|
||||
return target
|
||||
}
|
||||
|
||||
return nil
|
||||
}
|
||||
|
||||
private func commandPaletteBrowserFocusTarget(for webView: WKWebView) -> CommandPaletteRestoreFocusTarget? {
|
||||
if let selectedWorkspace = tabManager.selectedWorkspace,
|
||||
let target = commandPaletteBrowserFocusTarget(in: selectedWorkspace, for: webView) {
|
||||
return target
|
||||
}
|
||||
|
||||
let selectedWorkspaceId = tabManager.selectedTabId
|
||||
for workspace in tabManager.tabs where workspace.id != selectedWorkspaceId {
|
||||
if let target = commandPaletteBrowserFocusTarget(in: workspace, for: webView) {
|
||||
return target
|
||||
}
|
||||
}
|
||||
|
||||
return nil
|
||||
}
|
||||
|
||||
private func commandPaletteBrowserFocusTarget(
|
||||
in workspace: Workspace,
|
||||
for webView: WKWebView
|
||||
) -> CommandPaletteRestoreFocusTarget? {
|
||||
for (panelId, panel) in workspace.panels {
|
||||
guard let browserPanel = panel as? BrowserPanel,
|
||||
browserPanel.webView === webView else {
|
||||
continue
|
||||
}
|
||||
|
||||
return CommandPaletteRestoreFocusTarget(
|
||||
workspaceId: workspace.id,
|
||||
panelId: panelId,
|
||||
intent: .panel
|
||||
)
|
||||
}
|
||||
|
||||
return nil
|
||||
}
|
||||
|
||||
private func restoreCommandPaletteFocus(
|
||||
target: CommandPaletteRestoreFocusTarget,
|
||||
attemptsRemaining: Int
|
||||
|
|
|
|||
|
|
@ -298,6 +298,10 @@ struct BrowserPanelView: View {
|
|||
)
|
||||
}
|
||||
|
||||
private var browserContentAccessibilityIdentifier: String {
|
||||
"BrowserPanelContent.\(panel.id.uuidString)"
|
||||
}
|
||||
|
||||
private var omnibarPillBackgroundColor: NSColor {
|
||||
resolvedBrowserOmnibarPillBackgroundColor(
|
||||
for: browserChromeColorScheme,
|
||||
|
|
@ -749,6 +753,7 @@ struct BrowserPanelView: View {
|
|||
// BrowserPanel replaces its underlying WKWebView after process termination.
|
||||
.id(panel.webViewInstanceID)
|
||||
.contentShape(Rectangle())
|
||||
.accessibilityIdentifier(browserContentAccessibilityIdentifier)
|
||||
.simultaneousGesture(TapGesture().onEnded {
|
||||
// Chrome-like behavior: clicking web content while editing the
|
||||
// omnibar should commit blur and revert transient edits.
|
||||
|
|
@ -759,6 +764,7 @@ struct BrowserPanelView: View {
|
|||
} else {
|
||||
Color(nsColor: browserChromeBackgroundColor)
|
||||
.contentShape(Rectangle())
|
||||
.accessibilityIdentifier(browserContentAccessibilityIdentifier)
|
||||
.onTapGesture {
|
||||
onRequestPanelFocus()
|
||||
if addressBarFocused {
|
||||
|
|
|
|||
|
|
@ -275,6 +275,71 @@ final class BrowserPaneNavigationKeybindUITests: XCTestCase {
|
|||
)
|
||||
}
|
||||
|
||||
func testClickingBrowserDismissesCommandPaletteAndKeepsBrowserFocus() {
|
||||
let app = XCUIApplication()
|
||||
app.launchEnvironment["CMUX_SOCKET_PATH"] = socketPath
|
||||
app.launchEnvironment["CMUX_UI_TEST_GOTO_SPLIT_SETUP"] = "1"
|
||||
app.launchEnvironment["CMUX_UI_TEST_GOTO_SPLIT_PATH"] = dataPath
|
||||
app.launchEnvironment["CMUX_UI_TEST_FOCUS_SHORTCUTS"] = "1"
|
||||
launchAndEnsureForeground(app)
|
||||
|
||||
XCTAssertTrue(
|
||||
waitForData(keys: ["browserPanelId", "terminalPaneId", "webViewFocused"], timeout: 10.0),
|
||||
"Expected goto_split setup data to be written"
|
||||
)
|
||||
|
||||
guard let setup = loadData() else {
|
||||
XCTFail("Missing goto_split setup data")
|
||||
return
|
||||
}
|
||||
|
||||
guard let expectedBrowserPanelId = setup["browserPanelId"] else {
|
||||
XCTFail("Missing browserPanelId in goto_split setup data")
|
||||
return
|
||||
}
|
||||
|
||||
guard let expectedTerminalPaneId = setup["terminalPaneId"] else {
|
||||
XCTFail("Missing terminalPaneId in goto_split setup data")
|
||||
return
|
||||
}
|
||||
|
||||
// Move focus away from browser to terminal first so Cmd+R opens the rename overlay.
|
||||
app.typeKey("h", modifierFlags: [.command, .control])
|
||||
XCTAssertTrue(
|
||||
waitForDataMatch(timeout: 5.0) { data in
|
||||
data["lastMoveDirection"] == "left" && data["focusedPaneId"] == expectedTerminalPaneId
|
||||
},
|
||||
"Expected Cmd+Ctrl+H to move focus to left pane (terminal)"
|
||||
)
|
||||
|
||||
let renameField = app.textFields["CommandPaletteRenameField"].firstMatch
|
||||
app.typeKey("r", modifierFlags: [.command])
|
||||
XCTAssertTrue(
|
||||
renameField.waitForExistence(timeout: 5.0),
|
||||
"Expected Cmd+R to open the rename command palette while terminal is focused"
|
||||
)
|
||||
|
||||
let browserPane = app.otherElements["BrowserPanelContent.\(expectedBrowserPanelId)"].firstMatch
|
||||
XCTAssertTrue(browserPane.waitForExistence(timeout: 5.0), "Expected browser pane content for click target")
|
||||
browserPane.coordinate(withNormalizedOffset: CGVector(dx: 0.5, dy: 0.5)).click()
|
||||
XCTAssertTrue(
|
||||
waitForNonExistence(renameField, timeout: 5.0),
|
||||
"Expected clicking the browser pane to dismiss the command palette"
|
||||
)
|
||||
|
||||
// Cmd+L behavior is context-aware:
|
||||
// - If terminal is still focused: opens a new browser in that pane.
|
||||
// - If the original browser took focus: focuses that existing browser's omnibar.
|
||||
app.typeKey("l", modifierFlags: [.command])
|
||||
XCTAssertTrue(
|
||||
waitForDataMatch(timeout: 5.0) { data in
|
||||
guard data["webViewFocusedAfterAddressBarFocus"] == "false" else { return false }
|
||||
return data["webViewFocusedAfterAddressBarFocusPanelId"] == expectedBrowserPanelId
|
||||
},
|
||||
"Expected clicking browser content to dismiss the palette and keep focus on the existing browser pane"
|
||||
)
|
||||
}
|
||||
|
||||
func testCmdDSplitsRightWhenWebViewFocused() {
|
||||
let app = XCUIApplication()
|
||||
app.launchEnvironment["CMUX_SOCKET_PATH"] = socketPath
|
||||
|
|
@ -644,6 +709,12 @@ final class BrowserPaneNavigationKeybindUITests: XCTestCase {
|
|||
return false
|
||||
}
|
||||
|
||||
private func waitForNonExistence(_ element: XCUIElement, timeout: TimeInterval) -> Bool {
|
||||
let predicate = NSPredicate(format: "exists == false")
|
||||
let expectation = XCTNSPredicateExpectation(predicate: predicate, object: element)
|
||||
return XCTWaiter().wait(for: [expectation], timeout: timeout) == .completed
|
||||
}
|
||||
|
||||
private func loadData() -> [String: String]? {
|
||||
guard let data = try? Data(contentsOf: URL(fileURLWithPath: dataPath)) else {
|
||||
return nil
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue