Merge pull request #99 from manaflow-ai/fix/omnibar-focus-intent
Fix omnibar focus intent races for Cmd+L / Cmd+Shift+L
This commit is contained in:
commit
3193e602d4
4 changed files with 111 additions and 24 deletions
|
|
@ -1872,7 +1872,9 @@ final class AppDelegate: NSObject, NSApplicationDelegate, UNUserNotificationCent
|
|||
|
||||
// Open browser: Cmd+Shift+L
|
||||
if matchShortcut(event: event, shortcut: KeyboardShortcutSettings.shortcut(for: .openBrowser)) {
|
||||
tabManager?.openBrowser(insertAtEnd: true)
|
||||
if let panelId = tabManager?.openBrowser(insertAtEnd: true) {
|
||||
focusBrowserAddressBar(panelId: panelId)
|
||||
}
|
||||
return true
|
||||
}
|
||||
|
||||
|
|
@ -1884,17 +1886,12 @@ final class AppDelegate: NSObject, NSApplicationDelegate, UNUserNotificationCent
|
|||
}
|
||||
|
||||
if let browserAddressBarFocusedPanelId,
|
||||
let tabManager,
|
||||
let workspace = tabManager.selectedWorkspace,
|
||||
let panel = workspace.browserPanel(for: browserAddressBarFocusedPanelId) {
|
||||
workspace.focusPanel(panel.id)
|
||||
focusBrowserAddressBar(in: panel)
|
||||
focusBrowserAddressBar(panelId: browserAddressBarFocusedPanelId) {
|
||||
return true
|
||||
}
|
||||
|
||||
tabManager?.openBrowser(insertAtEnd: true)
|
||||
if let focusedPanel = tabManager?.focusedBrowserPanel {
|
||||
focusBrowserAddressBar(in: focusedPanel)
|
||||
if let panelId = tabManager?.openBrowser(insertAtEnd: true) {
|
||||
focusBrowserAddressBar(panelId: panelId)
|
||||
return true
|
||||
}
|
||||
}
|
||||
|
|
@ -1914,8 +1911,20 @@ final class AppDelegate: NSObject, NSApplicationDelegate, UNUserNotificationCent
|
|||
return false
|
||||
}
|
||||
|
||||
@discardableResult
|
||||
private func focusBrowserAddressBar(panelId: UUID) -> Bool {
|
||||
guard let tabManager,
|
||||
let workspace = tabManager.selectedWorkspace,
|
||||
let panel = workspace.browserPanel(for: panelId) else {
|
||||
return false
|
||||
}
|
||||
workspace.focusPanel(panel.id)
|
||||
focusBrowserAddressBar(in: panel)
|
||||
return true
|
||||
}
|
||||
|
||||
private func focusBrowserAddressBar(in panel: BrowserPanel) {
|
||||
panel.beginSuppressWebViewFocusForAddressBar()
|
||||
_ = panel.requestAddressBarFocus()
|
||||
browserAddressBarFocusedPanelId = panel.id
|
||||
NotificationCenter.default.post(name: .browserFocusAddressBar, object: panel.id)
|
||||
}
|
||||
|
|
|
|||
|
|
@ -805,6 +805,10 @@ final class BrowserPanel: Panel, ObservableObject {
|
|||
/// Increment to request a UI-only flash highlight (e.g. from a keyboard shortcut).
|
||||
@Published private(set) var focusFlashToken: Int = 0
|
||||
|
||||
/// Sticky omnibar-focus intent. This survives view mount timing races and is
|
||||
/// cleared only after BrowserPanelView acknowledges handling it.
|
||||
@Published private(set) var pendingAddressBarFocusRequestId: UUID?
|
||||
|
||||
private var cancellables = Set<AnyCancellable>()
|
||||
private var navigationDelegate: BrowserNavigationDelegate?
|
||||
private var uiDelegate: BrowserUIDelegate?
|
||||
|
|
@ -1362,6 +1366,22 @@ extension BrowserPanel {
|
|||
suppressWebViewFocusForAddressBar = false
|
||||
}
|
||||
|
||||
@discardableResult
|
||||
func requestAddressBarFocus() -> UUID {
|
||||
beginSuppressWebViewFocusForAddressBar()
|
||||
if let pendingAddressBarFocusRequestId {
|
||||
return pendingAddressBarFocusRequestId
|
||||
}
|
||||
let requestId = UUID()
|
||||
pendingAddressBarFocusRequestId = requestId
|
||||
return requestId
|
||||
}
|
||||
|
||||
func acknowledgeAddressBarFocusRequest(_ requestId: UUID) {
|
||||
guard pendingAddressBarFocusRequestId == requestId else { return }
|
||||
pendingAddressBarFocusRequestId = nil
|
||||
}
|
||||
|
||||
/// Returns the most reliable URL string for omnibar-related matching and UI decisions.
|
||||
/// `currentURL` can lag behind navigation changes, so prefer the live WKWebView URL.
|
||||
func preferredURLStringForOmnibar() -> String? {
|
||||
|
|
|
|||
|
|
@ -36,6 +36,7 @@ struct BrowserPanelView: View {
|
|||
@State private var focusFlashOpacity: Double = 0.0
|
||||
@State private var focusFlashFadeWorkItem: DispatchWorkItem?
|
||||
@State private var omnibarPillFrame: CGRect = .zero
|
||||
@State private var lastHandledAddressBarFocusRequestId: UUID?
|
||||
private let omnibarPillCornerRadius: CGFloat = 12
|
||||
|
||||
private var searchEngine: BrowserSearchEngine {
|
||||
|
|
@ -111,6 +112,7 @@ struct BrowserPanelView: View {
|
|||
BrowserSearchSettings.searchEngineKey: BrowserSearchSettings.defaultSearchEngine.rawValue,
|
||||
BrowserSearchSettings.searchSuggestionsEnabledKey: BrowserSearchSettings.defaultSearchSuggestionsEnabled,
|
||||
])
|
||||
applyPendingAddressBarFocusRequestIfNeeded()
|
||||
syncURLFromPanel()
|
||||
// If the browser surface is focused but has no URL loaded yet, auto-focus the omnibar.
|
||||
autoFocusOmnibarIfBlank()
|
||||
|
|
@ -131,9 +133,13 @@ struct BrowserPanelView: View {
|
|||
addressBarFocused = false
|
||||
}
|
||||
}
|
||||
.onChange(of: panel.pendingAddressBarFocusRequestId) { _ in
|
||||
applyPendingAddressBarFocusRequestIfNeeded()
|
||||
}
|
||||
.onChange(of: isFocused) { focused in
|
||||
// Ensure this view doesn't retain focus while hidden (bonsplit keepAllAlive).
|
||||
if focused {
|
||||
applyPendingAddressBarFocusRequestIfNeeded()
|
||||
autoFocusOmnibarIfBlank()
|
||||
} else {
|
||||
hideSuggestions()
|
||||
|
|
@ -167,20 +173,6 @@ struct BrowserPanelView: View {
|
|||
inlineCompletion = nil
|
||||
}
|
||||
}
|
||||
.onReceive(NotificationCenter.default.publisher(for: .browserFocusAddressBar)) { notification in
|
||||
guard let panelId = notification.object as? UUID, panelId == panel.id else { return }
|
||||
panel.beginSuppressWebViewFocusForAddressBar()
|
||||
if addressBarFocused {
|
||||
// Cmd+L should always refresh omnibar state/select-all, even when the
|
||||
// field already has focus.
|
||||
let urlString = panel.preferredURLStringForOmnibar() ?? ""
|
||||
let effects = omnibarReduce(state: &omnibarState, event: .focusGained(currentURLString: urlString))
|
||||
applyOmnibarEffects(effects)
|
||||
refreshInlineCompletion()
|
||||
} else {
|
||||
addressBarFocused = true
|
||||
}
|
||||
}
|
||||
.onReceive(NotificationCenter.default.publisher(for: .browserMoveOmnibarSelection)) { notification in
|
||||
guard let panelId = notification.object as? UUID, panelId == panel.id else { return }
|
||||
guard addressBarFocused, !omnibarState.suggestions.isEmpty else { return }
|
||||
|
|
@ -406,6 +398,26 @@ struct BrowserPanelView: View {
|
|||
applyOmnibarEffects(effects)
|
||||
}
|
||||
|
||||
private func applyPendingAddressBarFocusRequestIfNeeded() {
|
||||
guard let requestId = panel.pendingAddressBarFocusRequestId else { return }
|
||||
guard lastHandledAddressBarFocusRequestId != requestId else { return }
|
||||
lastHandledAddressBarFocusRequestId = requestId
|
||||
panel.beginSuppressWebViewFocusForAddressBar()
|
||||
|
||||
if addressBarFocused {
|
||||
// Re-run focus behavior (select-all/refresh suggestions) when focus is
|
||||
// explicitly requested again while already focused.
|
||||
let urlString = panel.preferredURLStringForOmnibar() ?? ""
|
||||
let effects = omnibarReduce(state: &omnibarState, event: .focusGained(currentURLString: urlString))
|
||||
applyOmnibarEffects(effects)
|
||||
refreshInlineCompletion()
|
||||
} else {
|
||||
addressBarFocused = true
|
||||
}
|
||||
|
||||
panel.acknowledgeAddressBarFocusRequest(requestId)
|
||||
}
|
||||
|
||||
/// Treat a WebView with no URL (or about:blank) as "blank" for UX purposes.
|
||||
private func isWebViewBlank() -> Bool {
|
||||
guard let url = panel.webView.url else { return true }
|
||||
|
|
|
|||
|
|
@ -510,6 +510,52 @@ final class TabManagerSurfaceCreationTests: XCTestCase {
|
|||
}
|
||||
}
|
||||
|
||||
@MainActor
|
||||
final class BrowserPanelAddressBarFocusRequestTests: XCTestCase {
|
||||
func testRequestPersistsUntilAcknowledged() {
|
||||
let panel = BrowserPanel(workspaceId: UUID())
|
||||
XCTAssertNil(panel.pendingAddressBarFocusRequestId)
|
||||
|
||||
let requestId = panel.requestAddressBarFocus()
|
||||
XCTAssertEqual(panel.pendingAddressBarFocusRequestId, requestId)
|
||||
XCTAssertTrue(panel.shouldSuppressWebViewFocus())
|
||||
|
||||
panel.acknowledgeAddressBarFocusRequest(requestId)
|
||||
XCTAssertNil(panel.pendingAddressBarFocusRequestId)
|
||||
|
||||
// Acknowledgement only clears the durable request; focus suppression follows
|
||||
// explicit blur state transitions.
|
||||
XCTAssertTrue(panel.shouldSuppressWebViewFocus())
|
||||
panel.endSuppressWebViewFocusForAddressBar()
|
||||
XCTAssertFalse(panel.shouldSuppressWebViewFocus())
|
||||
}
|
||||
|
||||
func testRequestCoalescesWhilePending() {
|
||||
let panel = BrowserPanel(workspaceId: UUID())
|
||||
let firstRequest = panel.requestAddressBarFocus()
|
||||
let secondRequest = panel.requestAddressBarFocus()
|
||||
|
||||
XCTAssertEqual(firstRequest, secondRequest)
|
||||
XCTAssertEqual(panel.pendingAddressBarFocusRequestId, firstRequest)
|
||||
}
|
||||
|
||||
func testStaleAcknowledgementDoesNotClearNewestRequest() {
|
||||
let panel = BrowserPanel(workspaceId: UUID())
|
||||
let firstRequest = panel.requestAddressBarFocus()
|
||||
panel.acknowledgeAddressBarFocusRequest(firstRequest)
|
||||
let secondRequest = panel.requestAddressBarFocus()
|
||||
|
||||
XCTAssertNotEqual(firstRequest, secondRequest)
|
||||
XCTAssertEqual(panel.pendingAddressBarFocusRequestId, secondRequest)
|
||||
|
||||
panel.acknowledgeAddressBarFocusRequest(firstRequest)
|
||||
XCTAssertEqual(panel.pendingAddressBarFocusRequestId, secondRequest)
|
||||
|
||||
panel.acknowledgeAddressBarFocusRequest(secondRequest)
|
||||
XCTAssertNil(panel.pendingAddressBarFocusRequestId)
|
||||
}
|
||||
}
|
||||
|
||||
final class SidebarDropPlannerTests: XCTestCase {
|
||||
func testNoIndicatorForNoOpEdges() {
|
||||
let first = UUID()
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue