fix(history): remove Ctrl+H fallback
- commit
- 0109d89385556cb20adec57873dad5e49f22ec33
- author
- brice · · 2026-06-13 05:57 UTC
- parent
- 5b002b0
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
5 files · +38 -105
M docs/reference/keybindings.md +2 -2
| @@ -83,7 +83,7 @@ These work outside modal modes: | |||
| 83 | 83 | | Action | Keys | | |
| 84 | 84 | |--------|------| | |
| 85 | 85 | | Toggle floating pane | `Alt+F` | | |
| 86 | - | | Toggle History sidebar (native GTK sidebar panel; fallback: opens dumb://history in a right split when sidebar is unavailable). Ctrl+H may conflict with the browser's default History shortcut; behavior can vary by browser. | `Ctrl+H` | | |
| 86 | + | | Toggle History sidebar (native GTK sidebar panel only). Ctrl+H may conflict with the browser's default History shortcut; behavior can vary by browser. | `Ctrl+H` | | |
| 87 | 87 | | Toggle Favorites system view in right split | unbound by default | | |
| 88 | 88 | | Toggle Config system view in right split | unbound by default | | |
| 89 | 89 | | Close pane (or release floating pane) | `Ctrl+W` | | |
| @@ -96,7 +96,7 @@ These work outside modal modes: | |||
| 96 | 96 | ||
| 97 | 97 | - `Alt+F` is the only floating-pane shortcut enabled by default. | |
| 98 | 98 | - `Alt+F` toggles floating visibility and keeps floating pane state intact. | |
| 99 | - | - `Ctrl+H` toggles the native GTK history sidebar when the history use case is available. The sidebar shows browsing history grouped by day with search/filter, keyboard navigation (arrows, Home/End, Ctrl+arrows for day jumps), and activation modes (Enter to navigate and close sidebar, Ctrl+Enter to navigate while keeping the sidebar open, Shift+Enter to open in a new split). When the history use case is unavailable (e.g., no database backend), Ctrl+H opens `dumb://history` in a right split as a fallback. | |
| 99 | + | - `Ctrl+H` toggles the native GTK history sidebar. The sidebar shows browsing history grouped by day with search/filter, keyboard navigation (arrows, Home/End, Ctrl+arrows for day jumps), and activation modes (Enter to navigate and close sidebar, Ctrl+Enter to navigate while keeping the sidebar open, Shift+Enter to open in a new split). If the native sidebar is unavailable, the shortcut returns an error instead of falling back to `dumb://history`. | |
| 100 | 100 | - `Ctrl+W` closes the active pane; when the floating pane is active, it fully releases that floating session. | |
| 101 | 101 | - Any URL shortcut (for example `Alt+G`) must be defined explicitly in `workspace.floating_pane.profiles`. | |
| 102 | 102 | - Floating profile shortcuts support modifier combos with `ctrl`, `shift`, and `alt` (for example `ctrl+shift+y` or `ctrl+alt+m`). | |
| @@ -83,7 +83,7 @@ These work outside modal modes: | |||||
| 83 | | Action | Keys | | 83 | | Action | Keys | | ||
| 84 | |--------|------| | 84 | |--------|------| | ||
| 85 | | Toggle floating pane | `Alt+F` | | 85 | | Toggle floating pane | `Alt+F` | | ||
| 86 | - | | Toggle History sidebar (native GTK sidebar panel; fallback: opens dumb://history in a right split when sidebar is unavailable). Ctrl+H may conflict with the browser's default History shortcut; behavior can vary by browser. | `Ctrl+H` | | 86 | + | | Toggle History sidebar (native GTK sidebar panel only). Ctrl+H may conflict with the browser's default History shortcut; behavior can vary by browser. | `Ctrl+H` | |
| 87 | | Toggle Favorites system view in right split | unbound by default | | 87 | | Toggle Favorites system view in right split | unbound by default | | ||
| 88 | | Toggle Config system view in right split | unbound by default | | 88 | | Toggle Config system view in right split | unbound by default | | ||
| 89 | | Close pane (or release floating pane) | `Ctrl+W` | | 89 | | Close pane (or release floating pane) | `Ctrl+W` | | ||
| @@ -96,7 +96,7 @@ These work outside modal modes: | |||||
| 96 | 96 | ||||
| 97 | - `Alt+F` is the only floating-pane shortcut enabled by default. | 97 | - `Alt+F` is the only floating-pane shortcut enabled by default. | ||
| 98 | - `Alt+F` toggles floating visibility and keeps floating pane state intact. | 98 | - `Alt+F` toggles floating visibility and keeps floating pane state intact. | ||
| 99 | - | - `Ctrl+H` toggles the native GTK history sidebar when the history use case is available. The sidebar shows browsing history grouped by day with search/filter, keyboard navigation (arrows, Home/End, Ctrl+arrows for day jumps), and activation modes (Enter to navigate and close sidebar, Ctrl+Enter to navigate while keeping the sidebar open, Shift+Enter to open in a new split). When the history use case is unavailable (e.g., no database backend), Ctrl+H opens `dumb://history` in a right split as a fallback. | 99 | + | - `Ctrl+H` toggles the native GTK history sidebar. The sidebar shows browsing history grouped by day with search/filter, keyboard navigation (arrows, Home/End, Ctrl+arrows for day jumps), and activation modes (Enter to navigate and close sidebar, Ctrl+Enter to navigate while keeping the sidebar open, Shift+Enter to open in a new split). If the native sidebar is unavailable, the shortcut returns an error instead of falling back to `dumb://history`. |
| 100 | - `Ctrl+W` closes the active pane; when the floating pane is active, it fully releases that floating session. | 100 | - `Ctrl+W` closes the active pane; when the floating pane is active, it fully releases that floating session. | ||
| 101 | - Any URL shortcut (for example `Alt+G`) must be defined explicitly in `workspace.floating_pane.profiles`. | 101 | - Any URL shortcut (for example `Alt+G`) must be defined explicitly in `workspace.floating_pane.profiles`. | ||
| 102 | - Floating profile shortcuts support modifier combos with `ctrl`, `shift`, and `alt` (for example `ctrl+shift+y` or `ctrl+alt+m`). | 102 | - Floating profile shortcuts support modifier combos with `ctrl`, `shift`, and `alt` (for example `ctrl+shift+y` or `ctrl+alt+m`). | ||
M internal/ui/app.go +5 -10
| @@ -3525,18 +3525,13 @@ func (a *App) wireKeyboardActions() { | |||
| 3525 | 3525 | a.kbDispatcher.SetOnToggleHistorySidebar(func(ctx context.Context) error { | |
| 3526 | 3526 | bw := a.lastFocusedBrowserWindow() | |
| 3527 | 3527 | if bw == nil { | |
| 3528 | - | return nil | |
| 3528 | + | return fmt.Errorf("history sidebar unavailable: no focused browser window") | |
| 3529 | 3529 | } | |
| 3530 | - | if bw.historySidebar != nil { | |
| 3531 | - | bw.toggleHistorySidebar() | |
| 3532 | - | return nil | |
| 3530 | + | if bw.historySidebar == nil { | |
| 3531 | + | return fmt.Errorf("history sidebar unavailable: native sidebar not initialized") | |
| 3533 | 3532 | } | |
| 3534 | - | if a.wsCoord == nil { | |
| 3535 | - | return nil | |
| 3536 | - | } | |
| 3537 | - | // Fall back to dumb://history system view when the native | |
| 3538 | - | // GTK sidebar is unavailable (HistoryUC nil or creation failed). | |
| 3539 | - | return a.wsCoord.ToggleSystemViewRight(ctx, "dumb://history") | |
| 3533 | + | bw.toggleHistorySidebar() | |
| 3534 | + | return nil | |
| 3540 | 3535 | }) | |
| 3541 | 3536 | a.kbDispatcher.SetOnToggleFloatingPane(func(ctx context.Context) error { | |
| 3542 | 3537 | return a.ToggleFloatingPane(ctx) | |
| @@ -3525,18 +3525,13 @@ func (a *App) wireKeyboardActions() { | |||||
| 3525 | a.kbDispatcher.SetOnToggleHistorySidebar(func(ctx context.Context) error { | 3525 | a.kbDispatcher.SetOnToggleHistorySidebar(func(ctx context.Context) error { | ||
| 3526 | bw := a.lastFocusedBrowserWindow() | 3526 | bw := a.lastFocusedBrowserWindow() | ||
| 3527 | if bw == nil { | 3527 | if bw == nil { | ||
| 3528 | - | return nil | 3528 | + | return fmt.Errorf("history sidebar unavailable: no focused browser window") |
| 3529 | } | 3529 | } | ||
| 3530 | - | if bw.historySidebar != nil { | 3530 | + | if bw.historySidebar == nil { |
| 3531 | - | bw.toggleHistorySidebar() | 3531 | + | return fmt.Errorf("history sidebar unavailable: native sidebar not initialized") |
| 3532 | - | return nil | |||
| 3533 | } | 3532 | } | ||
| 3534 | - | if a.wsCoord == nil { | 3533 | + | bw.toggleHistorySidebar() |
| 3535 | - | return nil | 3534 | + | return nil |
| 3536 | - | } | |||
| 3537 | - | // Fall back to dumb://history system view when the native | |||
| 3538 | - | // GTK sidebar is unavailable (HistoryUC nil or creation failed). | |||
| 3539 | - | return a.wsCoord.ToggleSystemViewRight(ctx, "dumb://history") | |||
| 3540 | }) | 3535 | }) | ||
| 3541 | a.kbDispatcher.SetOnToggleFloatingPane(func(ctx context.Context) error { | 3536 | a.kbDispatcher.SetOnToggleFloatingPane(func(ctx context.Context) error { | ||
| 3542 | return a.ToggleFloatingPane(ctx) | 3537 | return a.ToggleFloatingPane(ctx) | ||
M internal/ui/browser_window_test.go +14 -12
| @@ -3,6 +3,7 @@ package ui | |||
| 3 | 3 | import ( | |
| 4 | 4 | "context" | |
| 5 | 5 | "errors" | |
| 6 | + | "fmt" | |
| 6 | 7 | "reflect" | |
| 7 | 8 | "testing" | |
| 8 | 9 | "unsafe" | |
| @@ -1319,10 +1320,9 @@ func TestApp_HistorySidebar_ToggleThroughKeyboardDispatcher(t *testing.T) { | |||
| 1319 | 1320 | assert.False(t, focusedBW.sidebarVisible, "focused window sidebar must be hidden after second toggle") | |
| 1320 | 1321 | } | |
| 1321 | 1322 | ||
| 1322 | - | // TestApp_HistorySidebar_ToggleThroughDispatcher_FallbackPath verifies that | |
| 1323 | - | // when the focused window has no history sidebar, the dispatcher returns nil | |
| 1324 | - | // (no fallback system view in this test since we have no wsCoord). | |
| 1325 | - | func TestApp_HistorySidebar_ToggleThroughDispatcher_FallbackPath(t *testing.T) { | |
| 1323 | + | // TestApp_HistorySidebar_ToggleThroughDispatcher_UnavailableReturnsError verifies that | |
| 1324 | + | // when the focused window has no history sidebar, Ctrl+H returns a clean error. | |
| 1325 | + | func TestApp_HistorySidebar_ToggleThroughDispatcher_UnavailableReturnsError(t *testing.T) { | |
| 1326 | 1326 | ctx := context.Background() | |
| 1327 | 1327 | ||
| 1328 | 1328 | bw := &browserWindow{ | |
| @@ -1348,23 +1348,24 @@ func TestApp_HistorySidebar_ToggleThroughDispatcher_FallbackPath(t *testing.T) { | |||
| 1348 | 1348 | kbDispatcher.SetOnToggleHistorySidebar(func(ctx context.Context) error { | |
| 1349 | 1349 | bw := app.lastFocusedBrowserWindow() | |
| 1350 | 1350 | if bw == nil { | |
| 1351 | - | return nil | |
| 1351 | + | return fmt.Errorf("history sidebar unavailable: no focused browser window") | |
| 1352 | 1352 | } | |
| 1353 | 1353 | if bw.historySidebar != nil { | |
| 1354 | 1354 | bw.toggleHistorySidebar() | |
| 1355 | 1355 | return nil | |
| 1356 | 1356 | } | |
| 1357 | - | return nil | |
| 1357 | + | return fmt.Errorf("history sidebar unavailable: native sidebar not initialized") | |
| 1358 | 1358 | }) | |
| 1359 | 1359 | ||
| 1360 | 1360 | err := kbDispatcher.Dispatch(ctx, input.ActionToggleHistorySystemView) | |
| 1361 | - | require.NoError(t, err) | |
| 1361 | + | require.Error(t, err) | |
| 1362 | + | assert.ErrorContains(t, err, "history sidebar unavailable") | |
| 1362 | 1363 | assert.False(t, bw.sidebarVisible, "sidebar must remain invisible when not wired") | |
| 1363 | 1364 | } | |
| 1364 | 1365 | ||
| 1365 | - | // TestApp_HistorySidebar_ToggleThroughDispatcher_NilFocusedIsNoOp verifies | |
| 1366 | - | // that the toggle handler is a safe no-op when lastFocusedBrowserWindow returns nil. | |
| 1367 | - | func TestApp_HistorySidebar_ToggleThroughDispatcher_NilFocusedIsNoOp(t *testing.T) { | |
| 1366 | + | // TestApp_HistorySidebar_ToggleThroughDispatcher_NilFocusedReturnsError verifies | |
| 1367 | + | // that the toggle handler returns a clean error when there is no focused window. | |
| 1368 | + | func TestApp_HistorySidebar_ToggleThroughDispatcher_NilFocusedReturnsError(t *testing.T) { | |
| 1368 | 1369 | ctx := context.Background() | |
| 1369 | 1370 | ||
| 1370 | 1371 | app := &App{ | |
| @@ -1384,13 +1385,14 @@ func TestApp_HistorySidebar_ToggleThroughDispatcher_NilFocusedIsNoOp(t *testing. | |||
| 1384 | 1385 | kbDispatcher.SetOnToggleHistorySidebar(func(ctx context.Context) error { | |
| 1385 | 1386 | bw := app.lastFocusedBrowserWindow() | |
| 1386 | 1387 | if bw == nil { | |
| 1387 | - | return nil | |
| 1388 | + | return fmt.Errorf("history sidebar unavailable: no focused browser window") | |
| 1388 | 1389 | } | |
| 1389 | 1390 | return nil | |
| 1390 | 1391 | }) | |
| 1391 | 1392 | ||
| 1392 | 1393 | err := kbDispatcher.Dispatch(ctx, input.ActionToggleHistorySystemView) | |
| 1393 | - | require.NoError(t, err, "dispatch must not error when lastFocusedBrowserWindow returns nil") | |
| 1394 | + | require.Error(t, err) | |
| 1395 | + | assert.ErrorContains(t, err, "history sidebar unavailable") | |
| 1394 | 1396 | } | |
| 1395 | 1397 | ||
| 1396 | 1398 | // ============================================================================= | |
| @@ -3,6 +3,7 @@ package ui | |||||
| 3 | import ( | 3 | import ( | ||
| 4 | "context" | 4 | "context" | ||
| 5 | "errors" | 5 | "errors" | ||
| 6 | + | "fmt" | |||
| 6 | "reflect" | 7 | "reflect" | ||
| 7 | "testing" | 8 | "testing" | ||
| 8 | "unsafe" | 9 | "unsafe" | ||
| @@ -1319,10 +1320,9 @@ func TestApp_HistorySidebar_ToggleThroughKeyboardDispatcher(t *testing.T) { | |||||
| 1319 | assert.False(t, focusedBW.sidebarVisible, "focused window sidebar must be hidden after second toggle") | 1320 | assert.False(t, focusedBW.sidebarVisible, "focused window sidebar must be hidden after second toggle") | ||
| 1320 | } | 1321 | } | ||
| 1321 | 1322 | ||||
| 1322 | - | // TestApp_HistorySidebar_ToggleThroughDispatcher_FallbackPath verifies that | 1323 | + | // TestApp_HistorySidebar_ToggleThroughDispatcher_UnavailableReturnsError verifies that |
| 1323 | - | // when the focused window has no history sidebar, the dispatcher returns nil | 1324 | + | // when the focused window has no history sidebar, Ctrl+H returns a clean error. |
| 1324 | - | // (no fallback system view in this test since we have no wsCoord). | 1325 | + | func TestApp_HistorySidebar_ToggleThroughDispatcher_UnavailableReturnsError(t *testing.T) { |
| 1325 | - | func TestApp_HistorySidebar_ToggleThroughDispatcher_FallbackPath(t *testing.T) { | |||
| 1326 | ctx := context.Background() | 1326 | ctx := context.Background() | ||
| 1327 | 1327 | ||||
| 1328 | bw := &browserWindow{ | 1328 | bw := &browserWindow{ | ||
| @@ -1348,23 +1348,24 @@ func TestApp_HistorySidebar_ToggleThroughDispatcher_FallbackPath(t *testing.T) { | |||||
| 1348 | kbDispatcher.SetOnToggleHistorySidebar(func(ctx context.Context) error { | 1348 | kbDispatcher.SetOnToggleHistorySidebar(func(ctx context.Context) error { | ||
| 1349 | bw := app.lastFocusedBrowserWindow() | 1349 | bw := app.lastFocusedBrowserWindow() | ||
| 1350 | if bw == nil { | 1350 | if bw == nil { | ||
| 1351 | - | return nil | 1351 | + | return fmt.Errorf("history sidebar unavailable: no focused browser window") |
| 1352 | } | 1352 | } | ||
| 1353 | if bw.historySidebar != nil { | 1353 | if bw.historySidebar != nil { | ||
| 1354 | bw.toggleHistorySidebar() | 1354 | bw.toggleHistorySidebar() | ||
| 1355 | return nil | 1355 | return nil | ||
| 1356 | } | 1356 | } | ||
| 1357 | - | return nil | 1357 | + | return fmt.Errorf("history sidebar unavailable: native sidebar not initialized") |
| 1358 | }) | 1358 | }) | ||
| 1359 | 1359 | ||||
| 1360 | err := kbDispatcher.Dispatch(ctx, input.ActionToggleHistorySystemView) | 1360 | err := kbDispatcher.Dispatch(ctx, input.ActionToggleHistorySystemView) | ||
| 1361 | - | require.NoError(t, err) | 1361 | + | require.Error(t, err) |
| 1362 | + | assert.ErrorContains(t, err, "history sidebar unavailable") | |||
| 1362 | assert.False(t, bw.sidebarVisible, "sidebar must remain invisible when not wired") | 1363 | assert.False(t, bw.sidebarVisible, "sidebar must remain invisible when not wired") | ||
| 1363 | } | 1364 | } | ||
| 1364 | 1365 | ||||
| 1365 | - | // TestApp_HistorySidebar_ToggleThroughDispatcher_NilFocusedIsNoOp verifies | 1366 | + | // TestApp_HistorySidebar_ToggleThroughDispatcher_NilFocusedReturnsError verifies |
| 1366 | - | // that the toggle handler is a safe no-op when lastFocusedBrowserWindow returns nil. | 1367 | + | // that the toggle handler returns a clean error when there is no focused window. |
| 1367 | - | func TestApp_HistorySidebar_ToggleThroughDispatcher_NilFocusedIsNoOp(t *testing.T) { | 1368 | + | func TestApp_HistorySidebar_ToggleThroughDispatcher_NilFocusedReturnsError(t *testing.T) { |
| 1368 | ctx := context.Background() | 1369 | ctx := context.Background() | ||
| 1369 | 1370 | ||||
| 1370 | app := &App{ | 1371 | app := &App{ | ||
| @@ -1384,13 +1385,14 @@ func TestApp_HistorySidebar_ToggleThroughDispatcher_NilFocusedIsNoOp(t *testing. | |||||
| 1384 | kbDispatcher.SetOnToggleHistorySidebar(func(ctx context.Context) error { | 1385 | kbDispatcher.SetOnToggleHistorySidebar(func(ctx context.Context) error { | ||
| 1385 | bw := app.lastFocusedBrowserWindow() | 1386 | bw := app.lastFocusedBrowserWindow() | ||
| 1386 | if bw == nil { | 1387 | if bw == nil { | ||
| 1387 | - | return nil | 1388 | + | return fmt.Errorf("history sidebar unavailable: no focused browser window") |
| 1388 | } | 1389 | } | ||
| 1389 | return nil | 1390 | return nil | ||
| 1390 | }) | 1391 | }) | ||
| 1391 | 1392 | ||||
| 1392 | err := kbDispatcher.Dispatch(ctx, input.ActionToggleHistorySystemView) | 1393 | err := kbDispatcher.Dispatch(ctx, input.ActionToggleHistorySystemView) | ||
| 1393 | - | require.NoError(t, err, "dispatch must not error when lastFocusedBrowserWindow returns nil") | 1394 | + | require.Error(t, err) |
| 1395 | + | assert.ErrorContains(t, err, "history sidebar unavailable") | |||
| 1394 | } | 1396 | } | ||
| 1395 | 1397 | ||||
| 1396 | // ============================================================================= | 1398 | // ============================================================================= | ||
M internal/ui/dispatcher/keyboard.go +9 -20
| @@ -16,19 +16,9 @@ import ( | |||
| 16 | 16 | ||
| 17 | 17 | const ( | |
| 18 | 18 | // historySystemViewURL is the full-page/systemview history surface. | |
| 19 | - | // It is preserved for two scenarios: | |
| 20 | - | // 1. Direct navigation: the user can type "dumb://history" in the | |
| 21 | - | // omnibox to open a full-page history viewer in any pane. | |
| 22 | - | // 2. Fallback: if native sidebar SetOnToggleHistorySidebar is nil | |
| 23 | - | // (history usecase unavailable), Ctrl+H degenerates to opening | |
| 24 | - | // this URL in a right split. | |
| 25 | - | // | |
| 26 | - | // Ctrl+H always prefers the native sidebar path when wired (see | |
| 27 | - | // ActionToggleHistorySystemView handler). The native sidebar is a | |
| 28 | - | // GTK panel; dumb://history is an HTML system view with more | |
| 29 | - | // features (delete entries, domain filtering, CSV export). They | |
| 30 | - | // coexist: Ctrl+H opens the native sidebar; dumb://history is | |
| 31 | - | // reached by direct navigation. | |
| 19 | + | // It remains reachable by direct navigation (for example typing | |
| 20 | + | // dumb://history in the omnibox), but Ctrl+H no longer falls back to it. | |
| 21 | + | // Ctrl+H is reserved for the native GTK history sidebar only. | |
| 32 | 22 | historySystemViewURL = "dumb://history" | |
| 33 | 23 | favoritesSystemViewURL = "dumb://favorites" | |
| 34 | 24 | configSystemViewURL = "dumb://config" | |
| @@ -262,15 +252,14 @@ func (d *KeyboardDispatcher) initActionHandlers() { | |||
| 262 | 252 | } | |
| 263 | 253 | return d.logNoop(ctx, "toggle floating pane action (no handler)") | |
| 264 | 254 | }, | |
| 265 | - | // ActionToggleHistorySystemView (default Ctrl+H) prefers the native | |
| 266 | - | // GTK sidebar path (onToggleHistorySidebar). Falls back to the | |
| 267 | - | // dumb://history full-page systemview when no sidebar is wired. | |
| 268 | - | // See historySystemViewURL docstring for the coexistence story. | |
| 255 | + | // ActionToggleHistorySystemView (default Ctrl+H) is intentionally bound | |
| 256 | + | // to the native GTK history sidebar only. The dumb://history systemview | |
| 257 | + | // remains available through direct navigation, not shortcut fallback. | |
| 269 | 258 | input.ActionToggleHistorySystemView: func(ctx context.Context) error { | |
| 270 | - | if d.onToggleHistorySidebar != nil { | |
| 271 | - | return d.onToggleHistorySidebar(ctx) | |
| 259 | + | if d.onToggleHistorySidebar == nil { | |
| 260 | + | return fmt.Errorf("history sidebar unavailable: toggle handler not wired") | |
| 272 | 261 | } | |
| 273 | - | return d.wsCoord.ToggleSystemViewRight(ctx, historySystemViewURL) | |
| 262 | + | return d.onToggleHistorySidebar(ctx) | |
| 274 | 263 | }, | |
| 275 | 264 | input.ActionToggleFavoritesSystemView: func(ctx context.Context) error { | |
| 276 | 265 | return d.wsCoord.ToggleSystemViewRight(ctx, favoritesSystemViewURL) | |
| @@ -16,19 +16,9 @@ import ( | |||||
| 16 | 16 | ||||
| 17 | const ( | 17 | const ( | ||
| 18 | // historySystemViewURL is the full-page/systemview history surface. | 18 | // historySystemViewURL is the full-page/systemview history surface. | ||
| 19 | - | // It is preserved for two scenarios: | 19 | + | // It remains reachable by direct navigation (for example typing |
| 20 | - | // 1. Direct navigation: the user can type "dumb://history" in the | 20 | + | // dumb://history in the omnibox), but Ctrl+H no longer falls back to it. |
| 21 | - | // omnibox to open a full-page history viewer in any pane. | 21 | + | // Ctrl+H is reserved for the native GTK history sidebar only. |
| 22 | - | // 2. Fallback: if native sidebar SetOnToggleHistorySidebar is nil | |||
| 23 | - | // (history usecase unavailable), Ctrl+H degenerates to opening | |||
| 24 | - | // this URL in a right split. | |||
| 25 | - | // | |||
| 26 | - | // Ctrl+H always prefers the native sidebar path when wired (see | |||
| 27 | - | // ActionToggleHistorySystemView handler). The native sidebar is a | |||
| 28 | - | // GTK panel; dumb://history is an HTML system view with more | |||
| 29 | - | // features (delete entries, domain filtering, CSV export). They | |||
| 30 | - | // coexist: Ctrl+H opens the native sidebar; dumb://history is | |||
| 31 | - | // reached by direct navigation. | |||
| 32 | historySystemViewURL = "dumb://history" | 22 | historySystemViewURL = "dumb://history" | ||
| 33 | favoritesSystemViewURL = "dumb://favorites" | 23 | favoritesSystemViewURL = "dumb://favorites" | ||
| 34 | configSystemViewURL = "dumb://config" | 24 | configSystemViewURL = "dumb://config" | ||
| @@ -262,15 +252,14 @@ func (d *KeyboardDispatcher) initActionHandlers() { | |||||
| 262 | } | 252 | } | ||
| 263 | return d.logNoop(ctx, "toggle floating pane action (no handler)") | 253 | return d.logNoop(ctx, "toggle floating pane action (no handler)") | ||
| 264 | }, | 254 | }, | ||
| 265 | - | // ActionToggleHistorySystemView (default Ctrl+H) prefers the native | 255 | + | // ActionToggleHistorySystemView (default Ctrl+H) is intentionally bound |
| 266 | - | // GTK sidebar path (onToggleHistorySidebar). Falls back to the | 256 | + | // to the native GTK history sidebar only. The dumb://history systemview |
| 267 | - | // dumb://history full-page systemview when no sidebar is wired. | 257 | + | // remains available through direct navigation, not shortcut fallback. |
| 268 | - | // See historySystemViewURL docstring for the coexistence story. | |||
| 269 | input.ActionToggleHistorySystemView: func(ctx context.Context) error { | 258 | input.ActionToggleHistorySystemView: func(ctx context.Context) error { | ||
| 270 | - | if d.onToggleHistorySidebar != nil { | 259 | + | if d.onToggleHistorySidebar == nil { |
| 271 | - | return d.onToggleHistorySidebar(ctx) | 260 | + | return fmt.Errorf("history sidebar unavailable: toggle handler not wired") |
| 272 | } | 261 | } | ||
| 273 | - | return d.wsCoord.ToggleSystemViewRight(ctx, historySystemViewURL) | 262 | + | return d.onToggleHistorySidebar(ctx) |
| 274 | }, | 263 | }, | ||
| 275 | input.ActionToggleFavoritesSystemView: func(ctx context.Context) error { | 264 | input.ActionToggleFavoritesSystemView: func(ctx context.Context) error { | ||
| 276 | return d.wsCoord.ToggleSystemViewRight(ctx, favoritesSystemViewURL) | 265 | return d.wsCoord.ToggleSystemViewRight(ctx, favoritesSystemViewURL) | ||
M internal/ui/dispatcher/keyboard_test.go +8 -61
| @@ -5,9 +5,7 @@ import ( | |||
| 5 | 5 | "fmt" | |
| 6 | 6 | "testing" | |
| 7 | 7 | ||
| 8 | - | "github.com/bnema/dumber/internal/application/usecase" | |
| 9 | 8 | "github.com/bnema/dumber/internal/domain/entity" | |
| 10 | - | "github.com/bnema/dumber/internal/ui/component" | |
| 11 | 9 | "github.com/bnema/dumber/internal/ui/coordinator" | |
| 12 | 10 | "github.com/bnema/dumber/internal/ui/input" | |
| 13 | 11 | "github.com/stretchr/testify/assert" | |
| @@ -80,38 +78,13 @@ func TestKeyboardDispatcher_ToggleHistorySidebarCallsCallback(t *testing.T) { | |||
| 80 | 78 | assert.True(t, called, "onToggleHistorySidebar should have been called") | |
| 81 | 79 | } | |
| 82 | 80 | ||
| 83 | - | func TestKeyboardDispatcher_ToggleHistorySystemViewFallsBackToSystemView(t *testing.T) { | |
| 81 | + | func TestKeyboardDispatcher_ToggleHistorySystemViewReturnsErrorWhenHandlerMissing(t *testing.T) { | |
| 84 | 82 | ctx := context.Background() | |
| 85 | - | ids := []string{"pane-2", "split-1"} | |
| 86 | - | idx := 0 | |
| 87 | - | panesUC := usecase.NewManagePanesUseCase(func() string { | |
| 88 | - | id := ids[idx] | |
| 89 | - | idx++ | |
| 90 | - | return id | |
| 91 | - | }) | |
| 92 | - | ||
| 93 | - | initialPane := entity.NewPane("pane-1") | |
| 94 | - | initialPane.URI = "https://example.com" | |
| 95 | - | ws := entity.NewWorkspace("ws-1", initialPane) | |
| 96 | - | wsCoord := coordinator.NewWorkspaceCoordinator(ctx, coordinator.WorkspaceCoordinatorConfig{ | |
| 97 | - | PanesUC: panesUC, | |
| 98 | - | GetActiveWS: func() (*entity.Workspace, *component.WorkspaceView) { | |
| 99 | - | return ws, nil | |
| 100 | - | }, | |
| 101 | - | }) | |
| 102 | - | ||
| 103 | - | d := NewKeyboardDispatcher(ctx, wsCoord, &coordinator.NavigationCoordinator{}, nil, nil, KeyboardActions{}, func(context.Context) entity.PaneID { return "" }) | |
| 83 | + | d := NewKeyboardDispatcher(ctx, &coordinator.WorkspaceCoordinator{}, &coordinator.NavigationCoordinator{}, nil, nil, KeyboardActions{}, func(context.Context) entity.PaneID { return "" }) | |
| 104 | 84 | ||
| 105 | - | // No onToggleHistorySidebar set; should fall back to ToggleSystemViewRight | |
| 106 | 85 | err := d.Dispatch(ctx, input.ActionToggleHistorySystemView) | |
| 107 | - | require.NoError(t, err) | |
| 108 | - | ||
| 109 | - | require.Equal(t, 2, ws.PaneCount()) | |
| 110 | - | active := ws.ActivePane() | |
| 111 | - | require.NotNil(t, active) | |
| 112 | - | require.NotNil(t, active.Pane) | |
| 113 | - | assert.Equal(t, entity.PaneID("pane-2"), active.Pane.ID) | |
| 114 | - | assert.Equal(t, "dumb://history", active.Pane.URI) | |
| 86 | + | require.Error(t, err) | |
| 87 | + | assert.ErrorContains(t, err, "history sidebar unavailable") | |
| 115 | 88 | } | |
| 116 | 89 | ||
| 117 | 90 | func TestKeyboardDispatcher_ToggleHistorySidebarErrorPropagation(t *testing.T) { | |
| @@ -128,44 +101,18 @@ func TestKeyboardDispatcher_ToggleHistorySidebarErrorPropagation(t *testing.T) { | |||
| 128 | 101 | assert.ErrorIs(t, err, wantErr, "onToggleHistorySidebar error should propagate") | |
| 129 | 102 | } | |
| 130 | 103 | ||
| 131 | - | func TestKeyboardDispatcher_ToggleHistorySidebarSetThenUnsetFallsBack(t *testing.T) { | |
| 104 | + | func TestKeyboardDispatcher_ToggleHistorySidebarSetThenUnsetReturnsError(t *testing.T) { | |
| 132 | 105 | ctx := context.Background() | |
| 133 | - | ids := []string{"pane-3", "split-2"} | |
| 134 | - | idx := 0 | |
| 135 | - | panesUC := usecase.NewManagePanesUseCase(func() string { | |
| 136 | - | id := ids[idx] | |
| 137 | - | idx++ | |
| 138 | - | return id | |
| 139 | - | }) | |
| 140 | - | ||
| 141 | - | initialPane := entity.NewPane("pane-1") | |
| 142 | - | initialPane.URI = "https://example.com" | |
| 143 | - | ws := entity.NewWorkspace("ws-1", initialPane) | |
| 144 | - | wsCoord := coordinator.NewWorkspaceCoordinator(ctx, coordinator.WorkspaceCoordinatorConfig{ | |
| 145 | - | PanesUC: panesUC, | |
| 146 | - | GetActiveWS: func() (*entity.Workspace, *component.WorkspaceView) { | |
| 147 | - | return ws, nil | |
| 148 | - | }, | |
| 149 | - | }) | |
| 150 | - | ||
| 151 | - | d := NewKeyboardDispatcher(ctx, wsCoord, &coordinator.NavigationCoordinator{}, nil, nil, KeyboardActions{}, func(context.Context) entity.PaneID { return "" }) | |
| 106 | + | d := NewKeyboardDispatcher(ctx, &coordinator.WorkspaceCoordinator{}, &coordinator.NavigationCoordinator{}, nil, nil, KeyboardActions{}, func(context.Context) entity.PaneID { return "" }) | |
| 152 | 107 | ||
| 153 | - | // Set a callback that returns nil, then unset it by setting nil | |
| 154 | 108 | d.SetOnToggleHistorySidebar(func(context.Context) error { | |
| 155 | 109 | return nil | |
| 156 | 110 | }) | |
| 157 | - | // Setting to nil should clear the callback | |
| 158 | 111 | d.SetOnToggleHistorySidebar(nil) | |
| 159 | 112 | ||
| 160 | 113 | err := d.Dispatch(ctx, input.ActionToggleHistorySystemView) | |
| 161 | - | require.NoError(t, err) | |
| 162 | - | ||
| 163 | - | // Fallback path should have opened dumb://history in right split | |
| 164 | - | require.Equal(t, 2, ws.PaneCount()) | |
| 165 | - | active := ws.ActivePane() | |
| 166 | - | require.NotNil(t, active) | |
| 167 | - | require.NotNil(t, active.Pane) | |
| 168 | - | assert.Equal(t, "dumb://history", active.Pane.URI) | |
| 114 | + | require.Error(t, err) | |
| 115 | + | assert.ErrorContains(t, err, "history sidebar unavailable") | |
| 169 | 116 | } | |
| 170 | 117 | ||
| 171 | 118 | func TestKeyboardDispatcher_PassesActivePaneIDToShellCallbacks(t *testing.T) { | |
| @@ -5,9 +5,7 @@ import ( | |||||
| 5 | "fmt" | 5 | "fmt" | ||
| 6 | "testing" | 6 | "testing" | ||
| 7 | 7 | ||||
| 8 | - | "github.com/bnema/dumber/internal/application/usecase" | |||
| 9 | "github.com/bnema/dumber/internal/domain/entity" | 8 | "github.com/bnema/dumber/internal/domain/entity" | ||
| 10 | - | "github.com/bnema/dumber/internal/ui/component" | |||
| 11 | "github.com/bnema/dumber/internal/ui/coordinator" | 9 | "github.com/bnema/dumber/internal/ui/coordinator" | ||
| 12 | "github.com/bnema/dumber/internal/ui/input" | 10 | "github.com/bnema/dumber/internal/ui/input" | ||
| 13 | "github.com/stretchr/testify/assert" | 11 | "github.com/stretchr/testify/assert" | ||
| @@ -80,38 +78,13 @@ func TestKeyboardDispatcher_ToggleHistorySidebarCallsCallback(t *testing.T) { | |||||
| 80 | assert.True(t, called, "onToggleHistorySidebar should have been called") | 78 | assert.True(t, called, "onToggleHistorySidebar should have been called") | ||
| 81 | } | 79 | } | ||
| 82 | 80 | ||||
| 83 | - | func TestKeyboardDispatcher_ToggleHistorySystemViewFallsBackToSystemView(t *testing.T) { | 81 | + | func TestKeyboardDispatcher_ToggleHistorySystemViewReturnsErrorWhenHandlerMissing(t *testing.T) { |
| 84 | ctx := context.Background() | 82 | ctx := context.Background() | ||
| 85 | - | ids := []string{"pane-2", "split-1"} | 83 | + | d := NewKeyboardDispatcher(ctx, &coordinator.WorkspaceCoordinator{}, &coordinator.NavigationCoordinator{}, nil, nil, KeyboardActions{}, func(context.Context) entity.PaneID { return "" }) |
| 86 | - | idx := 0 | |||
| 87 | - | panesUC := usecase.NewManagePanesUseCase(func() string { | |||
| 88 | - | id := ids[idx] | |||
| 89 | - | idx++ | |||
| 90 | - | return id | |||
| 91 | - | }) | |||
| 92 | - | ||||
| 93 | - | initialPane := entity.NewPane("pane-1") | |||
| 94 | - | initialPane.URI = "https://example.com" | |||
| 95 | - | ws := entity.NewWorkspace("ws-1", initialPane) | |||
| 96 | - | wsCoord := coordinator.NewWorkspaceCoordinator(ctx, coordinator.WorkspaceCoordinatorConfig{ | |||
| 97 | - | PanesUC: panesUC, | |||
| 98 | - | GetActiveWS: func() (*entity.Workspace, *component.WorkspaceView) { | |||
| 99 | - | return ws, nil | |||
| 100 | - | }, | |||
| 101 | - | }) | |||
| 102 | - | ||||
| 103 | - | d := NewKeyboardDispatcher(ctx, wsCoord, &coordinator.NavigationCoordinator{}, nil, nil, KeyboardActions{}, func(context.Context) entity.PaneID { return "" }) | |||
| 104 | 84 | ||||
| 105 | - | // No onToggleHistorySidebar set; should fall back to ToggleSystemViewRight | |||
| 106 | err := d.Dispatch(ctx, input.ActionToggleHistorySystemView) | 85 | err := d.Dispatch(ctx, input.ActionToggleHistorySystemView) | ||
| 107 | - | require.NoError(t, err) | 86 | + | require.Error(t, err) |
| 108 | - | 87 | + | assert.ErrorContains(t, err, "history sidebar unavailable") | |
| 109 | - | require.Equal(t, 2, ws.PaneCount()) | |||
| 110 | - | active := ws.ActivePane() | |||
| 111 | - | require.NotNil(t, active) | |||
| 112 | - | require.NotNil(t, active.Pane) | |||
| 113 | - | assert.Equal(t, entity.PaneID("pane-2"), active.Pane.ID) | |||
| 114 | - | assert.Equal(t, "dumb://history", active.Pane.URI) | |||
| 115 | } | 88 | } | ||
| 116 | 89 | ||||
| 117 | func TestKeyboardDispatcher_ToggleHistorySidebarErrorPropagation(t *testing.T) { | 90 | func TestKeyboardDispatcher_ToggleHistorySidebarErrorPropagation(t *testing.T) { | ||
| @@ -128,44 +101,18 @@ func TestKeyboardDispatcher_ToggleHistorySidebarErrorPropagation(t *testing.T) { | |||||
| 128 | assert.ErrorIs(t, err, wantErr, "onToggleHistorySidebar error should propagate") | 101 | assert.ErrorIs(t, err, wantErr, "onToggleHistorySidebar error should propagate") | ||
| 129 | } | 102 | } | ||
| 130 | 103 | ||||
| 131 | - | func TestKeyboardDispatcher_ToggleHistorySidebarSetThenUnsetFallsBack(t *testing.T) { | 104 | + | func TestKeyboardDispatcher_ToggleHistorySidebarSetThenUnsetReturnsError(t *testing.T) { |
| 132 | ctx := context.Background() | 105 | ctx := context.Background() | ||
| 133 | - | ids := []string{"pane-3", "split-2"} | 106 | + | d := NewKeyboardDispatcher(ctx, &coordinator.WorkspaceCoordinator{}, &coordinator.NavigationCoordinator{}, nil, nil, KeyboardActions{}, func(context.Context) entity.PaneID { return "" }) |
| 134 | - | idx := 0 | |||
| 135 | - | panesUC := usecase.NewManagePanesUseCase(func() string { | |||
| 136 | - | id := ids[idx] | |||
| 137 | - | idx++ | |||
| 138 | - | return id | |||
| 139 | - | }) | |||
| 140 | - | ||||
| 141 | - | initialPane := entity.NewPane("pane-1") | |||
| 142 | - | initialPane.URI = "https://example.com" | |||
| 143 | - | ws := entity.NewWorkspace("ws-1", initialPane) | |||
| 144 | - | wsCoord := coordinator.NewWorkspaceCoordinator(ctx, coordinator.WorkspaceCoordinatorConfig{ | |||
| 145 | - | PanesUC: panesUC, | |||
| 146 | - | GetActiveWS: func() (*entity.Workspace, *component.WorkspaceView) { | |||
| 147 | - | return ws, nil | |||
| 148 | - | }, | |||
| 149 | - | }) | |||
| 150 | - | ||||
| 151 | - | d := NewKeyboardDispatcher(ctx, wsCoord, &coordinator.NavigationCoordinator{}, nil, nil, KeyboardActions{}, func(context.Context) entity.PaneID { return "" }) | |||
| 152 | 107 | ||||
| 153 | - | // Set a callback that returns nil, then unset it by setting nil | |||
| 154 | d.SetOnToggleHistorySidebar(func(context.Context) error { | 108 | d.SetOnToggleHistorySidebar(func(context.Context) error { | ||
| 155 | return nil | 109 | return nil | ||
| 156 | }) | 110 | }) | ||
| 157 | - | // Setting to nil should clear the callback | |||
| 158 | d.SetOnToggleHistorySidebar(nil) | 111 | d.SetOnToggleHistorySidebar(nil) | ||
| 159 | 112 | ||||
| 160 | err := d.Dispatch(ctx, input.ActionToggleHistorySystemView) | 113 | err := d.Dispatch(ctx, input.ActionToggleHistorySystemView) | ||
| 161 | - | require.NoError(t, err) | 114 | + | require.Error(t, err) |
| 162 | - | 115 | + | assert.ErrorContains(t, err, "history sidebar unavailable") | |
| 163 | - | // Fallback path should have opened dumb://history in right split | |||
| 164 | - | require.Equal(t, 2, ws.PaneCount()) | |||
| 165 | - | active := ws.ActivePane() | |||
| 166 | - | require.NotNil(t, active) | |||
| 167 | - | require.NotNil(t, active.Pane) | |||
| 168 | - | assert.Equal(t, "dumb://history", active.Pane.URI) | |||
| 169 | } | 116 | } | ||
| 170 | 117 | ||||
| 171 | func TestKeyboardDispatcher_PassesActivePaneIDToShellCallbacks(t *testing.T) { | 118 | func TestKeyboardDispatcher_PassesActivePaneIDToShellCallbacks(t *testing.T) { | ||