diff --git a/Sources/AppDelegate.swift b/Sources/AppDelegate.swift index 542add0d4360..c2693bc8f9a0 100644 --- a/Sources/AppDelegate.swift +++ b/Sources/AppDelegate.swift @@ -7684,6 +7684,18 @@ final class AppDelegate: NSObject, NSApplicationDelegate, UNUserNotificationCent keyboardFocusCoordinator(for: window)?.syncAfterResponderChange() } + /// Hands keyboard focus from the right sidebar to the workspace's focused + /// panel, when the sidebar currently owns it. Used after a sidebar-origin + /// action opens a panel in the main area (for example a file drag-drop): + /// the drag never resigns the sidebar's first responder, so without this + /// the find/shortcut router keeps targeting the sidebar. No-op when the + /// sidebar does not own focus. + @discardableResult + func restoreMainPanelKeyboardFocusFromRightSidebar(in window: NSWindow?) -> Bool { + keyboardFocusCoordinator(for: window)? + .restoreFocusedPanelFocusFromRightSidebarIfNeeded() ?? false + } + @discardableResult func focusRightSidebarInActiveMainWindow( mode requestedMode: RightSidebarMode? = nil, diff --git a/Sources/Find/MarkdownFindWebViewEvaluator.swift b/Sources/Find/MarkdownFindWebViewEvaluator.swift index 9512cb8a81b2..b30b2e14eb4e 100644 --- a/Sources/Find/MarkdownFindWebViewEvaluator.swift +++ b/Sources/Find/MarkdownFindWebViewEvaluator.swift @@ -20,7 +20,7 @@ final class MarkdownFindWebViewEvaluator: BrowserFindScriptEvaluating { } func evaluate(_ script: BrowserFindScript) async throws -> Any? { - guard let webView = panel?.rendererSession.findScriptWebView else { return nil } + guard let webView = panel?.rendererSession.webView else { return nil } return try await webView.evaluateJavaScript(script.source) } } diff --git a/Sources/Panels/FilePreviewWorkspaceOpenSupport.swift b/Sources/Panels/FilePreviewWorkspaceOpenSupport.swift index 2d921c389c5d..114d9ef055bc 100644 --- a/Sources/Panels/FilePreviewWorkspaceOpenSupport.swift +++ b/Sources/Panels/FilePreviewWorkspaceOpenSupport.swift @@ -14,6 +14,19 @@ extension Workspace { let shouldFocusNewTabs = focus ?? (bonsplitController.focusedPaneId == paneId) var nextIndex = targetIndex var openedPanels: [any Panel] = [] + defer { + // Shared across every focused open entrypoint (sidebar click, + // sidebar drag-drop, CLI/socket open, workspace actions): when + // the right sidebar owns keyboard focus, hand it to the opened + // panel so the find/shortcut router targets the document. A + // freshly created panel's view mounts a runloop turn later and + // cannot take first responder during activation, so this happens + // at the coordinator level. No-op when the sidebar does not own + // focus. + if shouldFocusNewTabs, let firstPanel = openedPanels.first { + handKeyboardFocusFromRightSidebarAfterFileOpen(to: firstPanel) + } + } for filePath in filePaths { let panel: (any Panel)? diff --git a/Sources/Panels/MarkdownPanel.swift b/Sources/Panels/MarkdownPanel.swift index 34f05773294a..3f75eed015f0 100644 --- a/Sources/Panels/MarkdownPanel.swift +++ b/Sources/Panels/MarkdownPanel.swift @@ -103,6 +103,9 @@ final class MarkdownPanel: Panel, ObservableObject, FilePreviewTextEditingPanel private var saveGeneration: Int = 0 private var activeSaveGeneration: Int? private var pendingSearchNeedle: String? + /// Set when activation asks a preview panel to focus before SwiftUI has + /// mounted its WKWebView. The renderer fulfills this at window attach. + private var pendingPreviewFocus = false private weak var textView: NSTextView? private var isClosed: Bool = false // NotificationCenter token; removal is thread-safe so deinit can drop it. @@ -138,6 +141,7 @@ final class MarkdownPanel: Panel, ObservableObject, FilePreviewTextEditingPanel startWatching() observeTypographyDefaults() rendererSession.onMarkdownRendered = { [weak self] in + self?.replayPendingPreviewFocusAfterWindowAttach() self?.replayActiveFindAfterRender() } } @@ -371,17 +375,43 @@ final class MarkdownPanel: Panel, ObservableObject, FilePreviewTextEditingPanel // MARK: - Panel protocol func focus() { - guard displayMode == .text else { return } - _ = textView?.window?.makeFirstResponder(textView) - applyPendingSearchNeedleIfPossible() + if displayMode == .text { + pendingPreviewFocus = false + _ = textView?.window?.makeFirstResponder(textView) + applyPendingSearchNeedleIfPossible() + return + } + // Preview mode: the rendered web view is the panel's keyboard + // surface. Taking first responder on activation is what moves the + // keyboard out of wherever it was (for example the right-sidebar + // file list after a click- or drag-open), so the find/shortcut + // router targets this panel — the same behavior terminal and + // browser panels have. No-op while the web view is not mounted; + // the drop/open paths also hand off focus at the coordinator level. + guard let webView = rendererSession.webView, let window = webView.window else { + pendingPreviewFocus = true + return + } + let didBecomeFirstResponder = window.makeFirstResponder(webView) + && window.firstResponder === webView + pendingPreviewFocus = !didBecomeFirstResponder + } + + /// Completes a preview focus request recorded before the renderer view was + /// attached to its window. The callback is event-driven, so it cannot + /// steal focus after this panel has been unfocused in the meantime. + func replayPendingPreviewFocusAfterWindowAttach() { + guard pendingPreviewFocus, displayMode == .preview else { return } + focus() } func unfocus() { - // No-op for read-only panel. + pendingPreviewFocus = false } func close() { isClosed = true + pendingPreviewFocus = false searchState = nil rendererSession.close() GlobalSearchCoordinator.shared.purgePanel(id: id) diff --git a/Sources/Panels/MarkdownPanelView.swift b/Sources/Panels/MarkdownPanelView.swift index ee6efbeded2e..5ee5d120951e 100644 --- a/Sources/Panels/MarkdownPanelView.swift +++ b/Sources/Panels/MarkdownPanelView.swift @@ -85,7 +85,10 @@ struct MarkdownPanelView: View { fontFamily: panel.fontFamily, maxContentWidth: panel.maxContentWidth, session: panel.rendererSession, - onRequestPanelFocus: onRequestPanelFocus + onRequestPanelFocus: onRequestPanelFocus, + onViewAttachedToWindow: { [weak panel] in + panel?.replayPendingPreviewFocusAfterWindowAttach() + } ) .frame(maxWidth: .infinity, maxHeight: .infinity) .opacity(panel.displayMode == .preview ? 1 : 0) diff --git a/Sources/Panels/MarkdownWebRenderer.swift b/Sources/Panels/MarkdownWebRenderer.swift index c95dc753187f..4d9d3ba9a33f 100644 --- a/Sources/Panels/MarkdownWebRenderer.swift +++ b/Sources/Panels/MarkdownWebRenderer.swift @@ -23,6 +23,10 @@ struct MarkdownWebRenderer: NSViewRepresentable { let maxContentWidth: Double let session: MarkdownRendererSession let onRequestPanelFocus: () -> Void + /// Called after the renderer view is attached to a window. A panel can + /// request focus before SwiftUI mounts its WebKit view, so the panel uses + /// this lifecycle signal to complete that request without polling. + let onViewAttachedToWindow: () -> Void = {} func makeCoordinator() -> Coordinator { session.coordinator(panelId: panelId, workspaceId: workspaceId, filePath: filePath) @@ -34,6 +38,7 @@ struct MarkdownWebRenderer: NSViewRepresentable { webView.removeFromSuperview() } webView.onPointerDown = onRequestPanelFocus + webView.onAttachToWindow = onViewAttachedToWindow webView.setVisibleInUI(isVisibleInUI) webView.onLeaveWindow = { [weak coordinator = context.coordinator] in coordinator?.handleViewLeftWindow() @@ -67,6 +72,7 @@ struct MarkdownWebRenderer: NSViewRepresentable { ) let webView = MarkdownWebView(frame: .zero, configuration: config) webView.onPointerDown = onRequestPanelFocus + webView.onAttachToWindow = onViewAttachedToWindow webView.setVisibleInUI(isVisibleInUI) webView.onLeaveWindow = { [weak coordinator = context.coordinator] in coordinator?.handleViewLeftWindow() @@ -119,6 +125,7 @@ struct MarkdownWebRenderer: NSViewRepresentable { nsView.navigationDelegate = nil nsView.uiDelegate = nil (nsView as? MarkdownWebView)?.onPointerDown = nil + (nsView as? MarkdownWebView)?.onAttachToWindow = nil (nsView as? MarkdownWebView)?.onLeaveWindow = nil (nsView as? MarkdownWebView)?.onReenterWindow = nil coordinator.cancelImageLoads() @@ -268,6 +275,7 @@ struct MarkdownWebRenderer: NSViewRepresentable { webView.navigationDelegate = nil webView.uiDelegate = nil webView.onPointerDown = nil + webView.onAttachToWindow = nil webView.onLeaveWindow = nil webView.onReenterWindow = nil } diff --git a/Sources/Panels/MarkdownWebSupport.swift b/Sources/Panels/MarkdownWebSupport.swift index bfb85d7f59bb..24d7878b7610 100644 --- a/Sources/Panels/MarkdownWebSupport.swift +++ b/Sources/Panels/MarkdownWebSupport.swift @@ -176,6 +176,10 @@ final class MarkdownWebRenderingCoordinator { @MainActor final class MarkdownWebView: WKWebView { var onPointerDown: (() -> Void)? + /// Invoked after this view is attached to a window. Keep this separate + /// from pointer focus so a panel can complete a focus request made before + /// SwiftUI mounted the WebKit view. + var onAttachToWindow: (() -> Void)? /// Invoked when the view leaves its window (the detach half of a pane /// re-parent). Lets the renderer coordinator record whether the document /// was healthy at detach time so re-entry recovery can tell a detach @@ -302,6 +306,8 @@ final class MarkdownWebView: WKWebView { // This callback only records renderer health. All WebKit lifecycle // selectors and layout/display work stay on the deferred path. onLeaveWindow?() + } else { + onAttachToWindow?() } } @@ -421,9 +427,10 @@ struct MarkdownWebTheme: Equatable { final class MarkdownRendererSession { private let ownedCoordinator = MarkdownWebRenderer.Coordinator() - /// The live preview web view, for find-in-page script evaluation. - /// `nil` until the renderer has been mounted once. - var findScriptWebView: WKWebView? { + /// The live preview web view — the panel's keyboard surface in preview + /// mode, and the evaluation target for find-in-page scripts. `nil` until + /// the renderer has been mounted once. + var webView: WKWebView? { ownedCoordinator.webView } diff --git a/Sources/Workspace.swift b/Sources/Workspace.swift index 786e04403288..d5a37a4f2184 100644 --- a/Sources/Workspace.swift +++ b/Sources/Workspace.swift @@ -12308,12 +12308,16 @@ final class Workspace: Identifiable, ObservableObject, FilePreviewTabMetadataHos targetIndex: index ).isEmpty case .split(let paneId, let orientation, let insertFirst): - return splitPaneWithFileSurface( + guard let panel = splitPaneWithFileSurface( targetPane: paneId, orientation: orientation, insertFirst: insertFirst, filePath: entry.filePath - ) != nil + ) else { + return false + } + handKeyboardFocusFromRightSidebarAfterFileOpen(to: panel) + return true } } @@ -12330,6 +12334,8 @@ final class Workspace: Identifiable, ObservableObject, FilePreviewTabMetadataHos switch request.destination { case .insert(let paneId, let index): + // openFileSurfaces(focus: true) performs the sidebar focus + // handoff itself. return !openFileSurfaces( inPane: paneId, filePaths: entries.map(\.filePath), @@ -12354,10 +12360,27 @@ final class Workspace: Identifiable, ObservableObject, FilePreviewTabMetadataHos filePaths: entries.dropFirst().map(\.filePath), focus: true ) + handKeyboardFocusFromRightSidebarAfterFileOpen(to: firstPanel) return true } } + /// A sidebar-initiated open (click on a file row, or a drag whose + /// mouse-down made the sidebar first responder) never resigns the + /// sidebar's keyboard focus by itself, and a freshly created panel's + /// view may not be mounted yet when activation asks it to take first + /// responder. Without this handoff the find/shortcut router keeps + /// targeting the sidebar (Cmd+F lands in its file search instead of the + /// just-opened document). Hand keyboard focus to the opened panel the + /// same way the text-drop path does; the call is a no-op when the + /// sidebar does not own focus (opens from Finder, the CLI, or between + /// panes). + func handKeyboardFocusFromRightSidebarAfterFileOpen(to panel: any Panel) { + _ = AppDelegate.shared?.restoreMainPanelKeyboardFocusFromRightSidebar( + in: activationWindow(for: panel) + ) + } + @discardableResult private func splitPaneWithFileSurface( targetPane paneId: PaneID, diff --git a/cmux.xcodeproj/project.pbxproj b/cmux.xcodeproj/project.pbxproj index 5afc1ed7edfa..621ad575fad5 100644 --- a/cmux.xcodeproj/project.pbxproj +++ b/cmux.xcodeproj/project.pbxproj @@ -2052,6 +2052,7 @@ C0DE71B10000000000000001 /* AppDelegate+AgentChatNotifications.swift in Sources D7AB34300000000000000003 /* SidebarBonsplitTabWorkspaceDropOverlay.swift in Sources */ = {isa = PBXBuildFile; fileRef = D7AB34300000000000000004 /* SidebarBonsplitTabWorkspaceDropOverlay.swift */; }; EA1F00000000000000000003 /* SidebarDirectoryText.swift in Sources */ = {isa = PBXBuildFile; fileRef = EA1F00000000000000000004 /* SidebarDirectoryText.swift */; }; B804A0270000000000000027 /* SidebarDividerTrackingView.swift in Sources */ = {isa = PBXBuildFile; fileRef = B804B0270000000000000027 /* SidebarDividerTrackingView.swift */; }; + A7FD1002 /* SidebarFileDropFindRoutingTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = A7FD1001 /* SidebarFileDropFindRoutingTests.swift */; }; B8624C060000000000000006 /* SidebarFocusBoundaryLifecycleTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = B8624D060000000000000006 /* SidebarFocusBoundaryLifecycleTests.swift */; }; A8624F0C0000000000000001 /* SidebarFocusBoundaryReference.swift in Sources */ = {isa = PBXBuildFile; fileRef = A8624F0C0000000000000002 /* SidebarFocusBoundaryReference.swift */; }; 8175A0010000000000000001 /* SidebarGitProcessCompositionTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 8175A0010000000000000002 /* SidebarGitProcessCompositionTests.swift */; }; @@ -4945,6 +4946,7 @@ B8B056D80000000000000002 /* MobileHostIdentityTests.swift */ = {isa = PBXFileRef D7AB34300000000000000004 /* SidebarBonsplitTabWorkspaceDropOverlay.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = Sidebar/SidebarBonsplitTabWorkspaceDropOverlay.swift; sourceTree = ""; }; EA1F00000000000000000004 /* SidebarDirectoryText.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = Sidebar/SidebarDirectoryText.swift; sourceTree = ""; }; B804B0270000000000000027 /* SidebarDividerTrackingView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = Sidebar/SidebarDividerTrackingView.swift; sourceTree = ""; }; + A7FD1001 /* SidebarFileDropFindRoutingTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SidebarFileDropFindRoutingTests.swift; sourceTree = ""; }; B8624D060000000000000006 /* SidebarFocusBoundaryLifecycleTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SidebarFocusBoundaryLifecycleTests.swift; sourceTree = ""; }; A8624F0C0000000000000002 /* SidebarFocusBoundaryReference.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = Sidebar/SidebarFocusBoundaryReference.swift; sourceTree = ""; }; 8175A0010000000000000002 /* SidebarGitProcessCompositionTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SidebarGitProcessCompositionTests.swift; sourceTree = ""; }; @@ -8262,6 +8264,7 @@ B8B056D80000000000000002 /* MobileHostIdentityTests.swift */ = {isa = PBXFileRef 6419B0026419B0026419B002 /* AppDelegateShortcutRoutingRepairProbe.swift */, 4E6A6F5C1D2B4980A1234567 /* AppDelegateSurfaceShortcutRoutingTests.swift */, 6512F0C06512F0C06512F001 /* MainWindowFocusRestoreTests.swift */, + A7FD1001 /* SidebarFileDropFindRoutingTests.swift */, C3467AB10000000000000002 /* ShortcutWhenClauseTests.swift */, 5330A0025330A0025330A002 /* TextBoxInlineAttachmentRenderingTests.swift */, C0DE7B300000000000000002 /* TextBoxMentionCompletionTests.swift */, @@ -11993,6 +11996,7 @@ B8B056D80000000000000002 /* MobileHostIdentityTests.swift */ = {isa = PBXFileRef F6001000A1B2C3D4E5F60718 /* ShortcutUnbindingTests.swift in Sources */, C3467AB10000000000000001 /* ShortcutWhenClauseTests.swift in Sources */, B804C0030000000000000003 /* SidebarAppKitRowCellTests.swift in Sources */, + A7FD1002 /* SidebarFileDropFindRoutingTests.swift in Sources */, B8624C060000000000000006 /* SidebarFocusBoundaryLifecycleTests.swift in Sources */, 8175A0010000000000000001 /* SidebarGitProcessCompositionTests.swift in Sources */, B8624C010000000000000001 /* SidebarHiddenPresentationTests.swift in Sources */, diff --git a/cmuxTests/SidebarFileDropFindRoutingTests.swift b/cmuxTests/SidebarFileDropFindRoutingTests.swift new file mode 100644 index 000000000000..1bade3cc5cd6 --- /dev/null +++ b/cmuxTests/SidebarFileDropFindRoutingTests.swift @@ -0,0 +1,132 @@ +import AppKit +import Testing +import WebKit + +#if canImport(cmux_DEV) +@testable import cmux_DEV +#elseif canImport(cmux) +@testable import cmux +#endif + +/// Regression coverage for https://github.com/manaflow-ai/cmux issue reported +/// after PR #11039: shift-dragging a file from the right-sidebar file +/// explorer into the workspace opens the file as a panel, but the drag never +/// resigns the sidebar's first responder, so Cmd+F kept routing to the +/// sidebar's file search instead of the just-opened document's find bar. +@MainActor +@Suite(.serialized) +struct SidebarFileDropFindRoutingTests { + @Test func sidebarFileDropHandsFindShortcutToOpenedMarkdownPanel() throws { + let appDelegate = try #require(AppDelegate.shared) + + let windowId = appDelegate.createMainWindow() + defer { closeWindow(withId: windowId) } + + let window = try #require(mainWindow(for: windowId)) + window.makeKeyAndOrderFront(nil) + let manager = try #require(appDelegate.tabManagerFor(windowId: windowId)) + let workspace = try #require(manager.selectedWorkspace) + let focusController = try #require(appDelegate.keyboardFocusCoordinator(for: window)) + + let fileManager = FileManager.default + let directoryURL = fileManager.temporaryDirectory + .appendingPathComponent("cmux-sidebar-drop-find-\(UUID().uuidString)", isDirectory: true) + try fileManager.createDirectory(at: directoryURL, withIntermediateDirectories: true) + let fileURL = directoryURL.appendingPathComponent("README.md") + try "# needle".write(to: fileURL, atomically: true, encoding: .utf8) + defer { try? fileManager.removeItem(at: directoryURL) } + + // Simulate the drag-origin state: the right-sidebar file area owns + // the window's first responder (a drag session never resigns it). + let sidebarResponder = RightSidebarKeyboardFocusView( + frame: NSRect(x: 0, y: 0, width: 24, height: 24) + ) + (window.contentView?.superview ?? window.contentView)?.addSubview(sidebarResponder) + defer { sidebarResponder.removeFromSuperview() } + sidebarResponder.registerWithKeyboardFocusCoordinatorIfNeeded() + #expect(window.makeFirstResponder(sidebarResponder), "Expected sidebar responder to take focus") + #expect( + focusController.findShortcutTarget(currentResponder: window.firstResponder) + == .rightSidebarFileSearch, + "Precondition: with the sidebar owning focus, Cmd+F targets the sidebar file search" + ) + + // The drop that a shift-drag from the sidebar performs. + let paneId = try #require(workspace.bonsplitController.focusedPaneId) + #expect( + workspace.handleExternalFileDrop( + BonsplitController.ExternalFileDropRequest( + urls: [fileURL], + destination: .insert(targetPane: paneId, targetIndex: nil) + ) + ), + "Expected the markdown file drop to open a panel" + ) + + let focusedPanelId = try #require(workspace.focusedPanelId) + #expect( + workspace.panels[focusedPanelId] is MarkdownPanel, + "Expected the dropped markdown file to be the focused panel" + ) + #expect( + window.firstResponder !== sidebarResponder, + "The drop must hand keyboard focus away from the sidebar" + ) + #expect( + focusController.findShortcutTarget(currentResponder: window.firstResponder) + == .mainPanelFind, + "After the drop opens a document, Cmd+F must target the opened panel, not the sidebar" + ) + } + + @Test func markdownPreviewFocusWaitsForWebViewAttachment() throws { + let directoryURL = FileManager.default.temporaryDirectory + .appendingPathComponent("cmux-markdown-focus-\(UUID().uuidString)", isDirectory: true) + try FileManager.default.createDirectory(at: directoryURL, withIntermediateDirectories: true) + defer { try? FileManager.default.removeItem(at: directoryURL) } + + let fileURL = directoryURL.appendingPathComponent("README.md") + try "# Focus".write(to: fileURL, atomically: true, encoding: .utf8) + let panel = MarkdownPanel(workspaceId: UUID(), filePath: fileURL.path) + defer { panel.close() } + + // Activation can happen before SwiftUI has created the representable. + // The request must remain pending instead of being silently dropped. + panel.focus() + + let coordinator = panel.rendererSession.coordinator( + panelId: panel.id, + workspaceId: panel.workspaceId, + filePath: panel.filePath + ) + let webView = MarkdownWebView(frame: .zero, configuration: WKWebViewConfiguration()) + coordinator.webView = webView + webView.onAttachToWindow = { [weak panel] in + panel?.replayPendingPreviewFocusAfterWindowAttach() + } + + let window = NSWindow( + contentRect: NSRect(x: 0, y: 0, width: 320, height: 240), + styleMask: [.borderless], + backing: .buffered, + defer: false + ) + window.contentView = webView + window.makeKeyAndOrderFront(nil) + defer { window.close() } + + #expect( + window.firstResponder === webView, + "A preview focus request made before mounting must be completed when the WebView enters its window." + ) + } + + private func mainWindow(for windowId: UUID) -> NSWindow? { + AppDelegate.shared?.windowForMainWindowId(windowId) + } + + private func closeWindow(withId windowId: UUID) { + guard let window = mainWindow(for: windowId) else { return } + window.close() + } +}