fix(history): resolve post-commit review findings
- commit
- 5b002b0d0997a40cfe6060f817df9f3941c4d9e7
- author
- brice · · 2026-06-12 20:08 UTC
- parent
- c5c9793
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
9 files · +229 -500
M internal/ui/app.go +4 -1
| @@ -3531,6 +3531,9 @@ func (a *App) wireKeyboardActions() { | |||
| 3531 | 3531 | bw.toggleHistorySidebar() | |
| 3532 | 3532 | return nil | |
| 3533 | 3533 | } | |
| 3534 | + | if a.wsCoord == nil { | |
| 3535 | + | return nil | |
| 3536 | + | } | |
| 3534 | 3537 | // Fall back to dumb://history system view when the native | |
| 3535 | 3538 | // GTK sidebar is unavailable (HistoryUC nil or creation failed). | |
| 3536 | 3539 | return a.wsCoord.ToggleSystemViewRight(ctx, "dumb://history") | |
| @@ -5064,7 +5067,7 @@ func (a *App) initConfigWatcher(ctx context.Context) { | |||
| 5064 | 5067 | if bw == nil { | |
| 5065 | 5068 | continue | |
| 5066 | 5069 | } | |
| 5067 | - | // Reapply sidebar width from live config (reloads after sidebare_width | |
| 5070 | + | // Reapply sidebar width from live config (reloads after sidebar_width | |
| 5068 | 5071 | // changes in the config file). | |
| 5069 | 5072 | bw.applySidebarWidthConfig(a) | |
| 5070 | 5073 | if bw.keyboardHandler != nil { | |
| @@ -3531,6 +3531,9 @@ func (a *App) wireKeyboardActions() { | |||||
| 3531 | bw.toggleHistorySidebar() | 3531 | bw.toggleHistorySidebar() | ||
| 3532 | return nil | 3532 | return nil | ||
| 3533 | } | 3533 | } | ||
| 3534 | + | if a.wsCoord == nil { | |||
| 3535 | + | return nil | |||
| 3536 | + | } | |||
| 3534 | // Fall back to dumb://history system view when the native | 3537 | // Fall back to dumb://history system view when the native | ||
| 3535 | // GTK sidebar is unavailable (HistoryUC nil or creation failed). | 3538 | // GTK sidebar is unavailable (HistoryUC nil or creation failed). | ||
| 3536 | return a.wsCoord.ToggleSystemViewRight(ctx, "dumb://history") | 3539 | return a.wsCoord.ToggleSystemViewRight(ctx, "dumb://history") | ||
| @@ -5064,7 +5067,7 @@ func (a *App) initConfigWatcher(ctx context.Context) { | |||||
| 5064 | if bw == nil { | 5067 | if bw == nil { | ||
| 5065 | continue | 5068 | continue | ||
| 5066 | } | 5069 | } | ||
| 5067 | - | // Reapply sidebar width from live config (reloads after sidebare_width | 5070 | + | // Reapply sidebar width from live config (reloads after sidebar_width |
| 5068 | // changes in the config file). | 5071 | // changes in the config file). | ||
| 5069 | bw.applySidebarWidthConfig(a) | 5072 | bw.applySidebarWidthConfig(a) | ||
| 5070 | if bw.keyboardHandler != nil { | 5073 | if bw.keyboardHandler != nil { | ||
M internal/ui/browser_window_test.go +1 -67
| @@ -1010,69 +1010,6 @@ func TestHistorySidebarConfig_OnNavigateKeepOpenNavigatesWithoutClosing(t *testi | |||
| 1010 | 1010 | assert.Equal(t, navigateURL, fakeWv.loadURILastURI, "Ctrl+Enter navigation should go to the URL") | |
| 1011 | 1011 | } | |
| 1012 | 1012 | ||
| 1013 | - | // TestHistorySidebarConfig_OnOpenInNewPaneCreatesSplit verifies that the | |
| 1014 | - | // OnOpenInNewPane callback (Shift+Enter) activates the owning browser window | |
| 1015 | - | // and calls wsCoord.SplitWithURL to open the URL in a right split. | |
| 1016 | - | func TestHistorySidebarConfig_OnOpenInNewPaneCreatesSplit(t *testing.T) { | |
| 1017 | - | ctx := context.Background() | |
| 1018 | - | ||
| 1019 | - | tab := entity.NewTab(entity.TabID("tab-1"), entity.WorkspaceID("ws-1"), entity.NewPane(entity.PaneID("pane-1"))) | |
| 1020 | - | bwTabs := entity.NewTabList() | |
| 1021 | - | bwTabs.Add(tab) | |
| 1022 | - | bwTabs.SetActive(tab.ID) | |
| 1023 | - | ||
| 1024 | - | // Create a workspace with one pane; SplitWithURL should create a second pane. | |
| 1025 | - | ws := entity.NewWorkspace("ws-1", entity.NewPane("pane-1")) | |
| 1026 | - | ws.ActivePaneID = "pane-1" | |
| 1027 | - | tab.Workspace = ws | |
| 1028 | - | ||
| 1029 | - | bw := &browserWindow{id: "window-1", tabs: bwTabs} | |
| 1030 | - | ||
| 1031 | - | // WorkspaceCoordinator needs a PanesUC and GetActiveWS. | |
| 1032 | - | panesUC := usecase.NewManagePanesUseCase(func() string { return "pane-2" }) | |
| 1033 | - | wsCoord := coordinator.NewWorkspaceCoordinator(ctx, coordinator.WorkspaceCoordinatorConfig{ | |
| 1034 | - | PanesUC: panesUC, | |
| 1035 | - | GetActiveWS: func() (*entity.Workspace, *component.WorkspaceView) { | |
| 1036 | - | return ws, nil | |
| 1037 | - | }, | |
| 1038 | - | }) | |
| 1039 | - | ||
| 1040 | - | splitCalled := false | |
| 1041 | - | var splitURL string | |
| 1042 | - | // We wrap wsCoord.SplitWithURL to verify it was called with the right URL. | |
| 1043 | - | // The real initHistorySidebar OnOpenInNewPane calls a.wsCoord.SplitWithURL(…). | |
| 1044 | - | // We use a test wrapper here to assert the call. | |
| 1045 | - | _ = splitCalled | |
| 1046 | - | _ = splitURL | |
| 1047 | - | ||
| 1048 | - | // Simulate the OnOpenInNewPane callback from initHistorySidebar. | |
| 1049 | - | // This activates the owning window and calls wsCoord.SplitWithURL. | |
| 1050 | - | _ = bw | |
| 1051 | - | _ = wsCoord | |
| 1052 | - | ||
| 1053 | - | // Direct test: call the app-level SplitWithURL through the real wsCoord. | |
| 1054 | - | // This verifies the complete path: activateBrowserWindow + SplitWithURL. | |
| 1055 | - | if err := wsCoord.SplitWithURL(ctx, usecase.SplitRight, "https://shift-enter.com"); err != nil { | |
| 1056 | - | t.Fatalf("SplitWithURL failed: %v", err) | |
| 1057 | - | } | |
| 1058 | - | ||
| 1059 | - | // After split, the workspace should have 2 panes. | |
| 1060 | - | require.Equal(t, 2, ws.PaneCount(), "workspace should have 2 panes after split") | |
| 1061 | - | ||
| 1062 | - | // The new pane should have the URL. | |
| 1063 | - | // Find the new pane (not the first one). | |
| 1064 | - | allPanes := ws.AllPanes() | |
| 1065 | - | var newPane *entity.Pane | |
| 1066 | - | for _, p := range allPanes { | |
| 1067 | - | if p != nil && p.ID != "pane-1" { | |
| 1068 | - | newPane = p | |
| 1069 | - | break | |
| 1070 | - | } | |
| 1071 | - | } | |
| 1072 | - | require.NotNil(t, newPane, "new pane should exist") | |
| 1073 | - | assert.Equal(t, "https://shift-enter.com", newPane.URI, "new pane should have the URL") | |
| 1074 | - | } | |
| 1075 | - | ||
| 1076 | 1013 | // TestHistorySidebar_OwnershipOnMultiWindowNavigation verifies that when | |
| 1077 | 1014 | // multiple browser windows have history sidebars, navigation targets the | |
| 1078 | 1015 | // correct owning window's active pane. This tests the stale-focus scenario | |
| @@ -1691,8 +1628,7 @@ func TestApp_HistorySidebarConfig_CloseCallback(t *testing.T) { | |||
| 1691 | 1628 | sidebarVisible: true, | |
| 1692 | 1629 | } | |
| 1693 | 1630 | ||
| 1694 | - | // Create a minimal workspace view that records focus calls. | |
| 1695 | - | focusCalled := false | |
| 1631 | + | // Create a minimal workspace view. | |
| 1696 | 1632 | wsView := &component.WorkspaceView{} | |
| 1697 | 1633 | // We set up the app so that hideAndRestoreFocusForBrowserWindow | |
| 1698 | 1634 | // can find the wsView and call FocusPane on it. | |
| @@ -1707,8 +1643,6 @@ func TestApp_HistorySidebarConfig_CloseCallback(t *testing.T) { | |||
| 1707 | 1643 | tab.ID: wsView, | |
| 1708 | 1644 | }, | |
| 1709 | 1645 | } | |
| 1710 | - | _ = focusCalled | |
| 1711 | - | ||
| 1712 | 1646 | cfg := app.buildHistorySidebarConfig(ctx, bw) | |
| 1713 | 1647 | ||
| 1714 | 1648 | // OnClose hides the sidebar. | |
| @@ -1010,69 +1010,6 @@ func TestHistorySidebarConfig_OnNavigateKeepOpenNavigatesWithoutClosing(t *testi | |||||
| 1010 | assert.Equal(t, navigateURL, fakeWv.loadURILastURI, "Ctrl+Enter navigation should go to the URL") | 1010 | assert.Equal(t, navigateURL, fakeWv.loadURILastURI, "Ctrl+Enter navigation should go to the URL") | ||
| 1011 | } | 1011 | } | ||
| 1012 | 1012 | ||||
| 1013 | - | // TestHistorySidebarConfig_OnOpenInNewPaneCreatesSplit verifies that the | |||
| 1014 | - | // OnOpenInNewPane callback (Shift+Enter) activates the owning browser window | |||
| 1015 | - | // and calls wsCoord.SplitWithURL to open the URL in a right split. | |||
| 1016 | - | func TestHistorySidebarConfig_OnOpenInNewPaneCreatesSplit(t *testing.T) { | |||
| 1017 | - | ctx := context.Background() | |||
| 1018 | - | ||||
| 1019 | - | tab := entity.NewTab(entity.TabID("tab-1"), entity.WorkspaceID("ws-1"), entity.NewPane(entity.PaneID("pane-1"))) | |||
| 1020 | - | bwTabs := entity.NewTabList() | |||
| 1021 | - | bwTabs.Add(tab) | |||
| 1022 | - | bwTabs.SetActive(tab.ID) | |||
| 1023 | - | ||||
| 1024 | - | // Create a workspace with one pane; SplitWithURL should create a second pane. | |||
| 1025 | - | ws := entity.NewWorkspace("ws-1", entity.NewPane("pane-1")) | |||
| 1026 | - | ws.ActivePaneID = "pane-1" | |||
| 1027 | - | tab.Workspace = ws | |||
| 1028 | - | ||||
| 1029 | - | bw := &browserWindow{id: "window-1", tabs: bwTabs} | |||
| 1030 | - | ||||
| 1031 | - | // WorkspaceCoordinator needs a PanesUC and GetActiveWS. | |||
| 1032 | - | panesUC := usecase.NewManagePanesUseCase(func() string { return "pane-2" }) | |||
| 1033 | - | wsCoord := coordinator.NewWorkspaceCoordinator(ctx, coordinator.WorkspaceCoordinatorConfig{ | |||
| 1034 | - | PanesUC: panesUC, | |||
| 1035 | - | GetActiveWS: func() (*entity.Workspace, *component.WorkspaceView) { | |||
| 1036 | - | return ws, nil | |||
| 1037 | - | }, | |||
| 1038 | - | }) | |||
| 1039 | - | ||||
| 1040 | - | splitCalled := false | |||
| 1041 | - | var splitURL string | |||
| 1042 | - | // We wrap wsCoord.SplitWithURL to verify it was called with the right URL. | |||
| 1043 | - | // The real initHistorySidebar OnOpenInNewPane calls a.wsCoord.SplitWithURL(…). | |||
| 1044 | - | // We use a test wrapper here to assert the call. | |||
| 1045 | - | _ = splitCalled | |||
| 1046 | - | _ = splitURL | |||
| 1047 | - | ||||
| 1048 | - | // Simulate the OnOpenInNewPane callback from initHistorySidebar. | |||
| 1049 | - | // This activates the owning window and calls wsCoord.SplitWithURL. | |||
| 1050 | - | _ = bw | |||
| 1051 | - | _ = wsCoord | |||
| 1052 | - | ||||
| 1053 | - | // Direct test: call the app-level SplitWithURL through the real wsCoord. | |||
| 1054 | - | // This verifies the complete path: activateBrowserWindow + SplitWithURL. | |||
| 1055 | - | if err := wsCoord.SplitWithURL(ctx, usecase.SplitRight, "https://shift-enter.com"); err != nil { | |||
| 1056 | - | t.Fatalf("SplitWithURL failed: %v", err) | |||
| 1057 | - | } | |||
| 1058 | - | ||||
| 1059 | - | // After split, the workspace should have 2 panes. | |||
| 1060 | - | require.Equal(t, 2, ws.PaneCount(), "workspace should have 2 panes after split") | |||
| 1061 | - | ||||
| 1062 | - | // The new pane should have the URL. | |||
| 1063 | - | // Find the new pane (not the first one). | |||
| 1064 | - | allPanes := ws.AllPanes() | |||
| 1065 | - | var newPane *entity.Pane | |||
| 1066 | - | for _, p := range allPanes { | |||
| 1067 | - | if p != nil && p.ID != "pane-1" { | |||
| 1068 | - | newPane = p | |||
| 1069 | - | break | |||
| 1070 | - | } | |||
| 1071 | - | } | |||
| 1072 | - | require.NotNil(t, newPane, "new pane should exist") | |||
| 1073 | - | assert.Equal(t, "https://shift-enter.com", newPane.URI, "new pane should have the URL") | |||
| 1074 | - | } | |||
| 1075 | - | ||||
| 1076 | // TestHistorySidebar_OwnershipOnMultiWindowNavigation verifies that when | 1013 | // TestHistorySidebar_OwnershipOnMultiWindowNavigation verifies that when | ||
| 1077 | // multiple browser windows have history sidebars, navigation targets the | 1014 | // multiple browser windows have history sidebars, navigation targets the | ||
| 1078 | // correct owning window's active pane. This tests the stale-focus scenario | 1015 | // correct owning window's active pane. This tests the stale-focus scenario | ||
| @@ -1691,8 +1628,7 @@ func TestApp_HistorySidebarConfig_CloseCallback(t *testing.T) { | |||||
| 1691 | sidebarVisible: true, | 1628 | sidebarVisible: true, | ||
| 1692 | } | 1629 | } | ||
| 1693 | 1630 | ||||
| 1694 | - | // Create a minimal workspace view that records focus calls. | 1631 | + | // Create a minimal workspace view. |
| 1695 | - | focusCalled := false | |||
| 1696 | wsView := &component.WorkspaceView{} | 1632 | wsView := &component.WorkspaceView{} | ||
| 1697 | // We set up the app so that hideAndRestoreFocusForBrowserWindow | 1633 | // We set up the app so that hideAndRestoreFocusForBrowserWindow | ||
| 1698 | // can find the wsView and call FocusPane on it. | 1634 | // can find the wsView and call FocusPane on it. | ||
| @@ -1707,8 +1643,6 @@ func TestApp_HistorySidebarConfig_CloseCallback(t *testing.T) { | |||||
| 1707 | tab.ID: wsView, | 1643 | tab.ID: wsView, | ||
| 1708 | }, | 1644 | }, | ||
| 1709 | } | 1645 | } | ||
| 1710 | - | _ = focusCalled | |||
| 1711 | - | ||||
| 1712 | cfg := app.buildHistorySidebarConfig(ctx, bw) | 1646 | cfg := app.buildHistorySidebarConfig(ctx, bw) | ||
| 1713 | 1647 | ||||
| 1714 | // OnClose hides the sidebar. | 1648 | // OnClose hides the sidebar. | ||
M internal/ui/component/history_model.go +0 -46
| @@ -82,8 +82,6 @@ func dayLabelForKey(key dayKey, todayStart time.Time, now time.Time) string { | |||
| 82 | 82 | return dayLabelYesterday | |
| 83 | 83 | case key.year == now.Year(): | |
| 84 | 84 | return dayStart.Format("Monday, January 2") | |
| 85 | - | case key.year == now.Year()-1: | |
| 86 | - | return dayStart.Format("Monday, January 2") | |
| 87 | 85 | default: | |
| 88 | 86 | return dayStart.Format(dayLabelOtherYearFormat) | |
| 89 | 87 | } | |
| @@ -327,47 +325,3 @@ func (m keyboardNavModel) entryCount() int { | |||
| 327 | 325 | } | |
| 328 | 326 | return n | |
| 329 | 327 | } | |
| 330 | - | ||
| 331 | - | // searchStateSnapshot captures the observable state of a history search | |
| 332 | - | // at a point in time. This is a pure-data type for testing search | |
| 333 | - | // transitions without GTK dependencies. | |
| 334 | - | type searchStateSnapshot struct { | |
| 335 | - | Query string | |
| 336 | - | HasSearchDone bool | |
| 337 | - | HasResults bool | |
| 338 | - | ResultCount int | |
| 339 | - | } | |
| 340 | - | ||
| 341 | - | // transitionSearchState models a search state transition: given a current | |
| 342 | - | // snapshot and a new query, what should the next snapshot look like? | |
| 343 | - | // This pure function encodes the expected transitions without GTK. | |
| 344 | - | func transitionSearchState(_ searchStateSnapshot, newQuery string, resultCount int) searchStateSnapshot { | |
| 345 | - | return searchStateSnapshot{ | |
| 346 | - | Query: newQuery, | |
| 347 | - | HasSearchDone: newQuery != "", | |
| 348 | - | HasResults: newQuery != "" && resultCount > 0, | |
| 349 | - | ResultCount: resultCount, | |
| 350 | - | } | |
| 351 | - | } | |
| 352 | - | ||
| 353 | - | // reloadPreservationSnapshot captures the preserved state during a reload. | |
| 354 | - | type reloadPreservationSnapshot struct { | |
| 355 | - | PreservedQuery string | |
| 356 | - | ResetBrowse bool | |
| 357 | - | ClearSearch bool | |
| 358 | - | } | |
| 359 | - | ||
| 360 | - | // applyReloadState computes the expected state after a reload given the | |
| 361 | - | // current query. When query is empty browse state is reset; when query is | |
| 362 | - | // non-empty search is cleared and re-triggered. | |
| 363 | - | func applyReloadState(currentQuery string) reloadPreservationSnapshot { | |
| 364 | - | s := reloadPreservationSnapshot{ | |
| 365 | - | PreservedQuery: currentQuery, | |
| 366 | - | } | |
| 367 | - | if currentQuery == "" { | |
| 368 | - | s.ResetBrowse = true | |
| 369 | - | } else { | |
| 370 | - | s.ClearSearch = true | |
| 371 | - | } | |
| 372 | - | return s | |
| 373 | - | } | |
| @@ -82,8 +82,6 @@ func dayLabelForKey(key dayKey, todayStart time.Time, now time.Time) string { | |||||
| 82 | return dayLabelYesterday | 82 | return dayLabelYesterday | ||
| 83 | case key.year == now.Year(): | 83 | case key.year == now.Year(): | ||
| 84 | return dayStart.Format("Monday, January 2") | 84 | return dayStart.Format("Monday, January 2") | ||
| 85 | - | case key.year == now.Year()-1: | |||
| 86 | - | return dayStart.Format("Monday, January 2") | |||
| 87 | default: | 85 | default: | ||
| 88 | return dayStart.Format(dayLabelOtherYearFormat) | 86 | return dayStart.Format(dayLabelOtherYearFormat) | ||
| 89 | } | 87 | } | ||
| @@ -327,47 +325,3 @@ func (m keyboardNavModel) entryCount() int { | |||||
| 327 | } | 325 | } | ||
| 328 | return n | 326 | return n | ||
| 329 | } | 327 | } | ||
| 330 | - | ||||
| 331 | - | // searchStateSnapshot captures the observable state of a history search | |||
| 332 | - | // at a point in time. This is a pure-data type for testing search | |||
| 333 | - | // transitions without GTK dependencies. | |||
| 334 | - | type searchStateSnapshot struct { | |||
| 335 | - | Query string | |||
| 336 | - | HasSearchDone bool | |||
| 337 | - | HasResults bool | |||
| 338 | - | ResultCount int | |||
| 339 | - | } | |||
| 340 | - | ||||
| 341 | - | // transitionSearchState models a search state transition: given a current | |||
| 342 | - | // snapshot and a new query, what should the next snapshot look like? | |||
| 343 | - | // This pure function encodes the expected transitions without GTK. | |||
| 344 | - | func transitionSearchState(_ searchStateSnapshot, newQuery string, resultCount int) searchStateSnapshot { | |||
| 345 | - | return searchStateSnapshot{ | |||
| 346 | - | Query: newQuery, | |||
| 347 | - | HasSearchDone: newQuery != "", | |||
| 348 | - | HasResults: newQuery != "" && resultCount > 0, | |||
| 349 | - | ResultCount: resultCount, | |||
| 350 | - | } | |||
| 351 | - | } | |||
| 352 | - | ||||
| 353 | - | // reloadPreservationSnapshot captures the preserved state during a reload. | |||
| 354 | - | type reloadPreservationSnapshot struct { | |||
| 355 | - | PreservedQuery string | |||
| 356 | - | ResetBrowse bool | |||
| 357 | - | ClearSearch bool | |||
| 358 | - | } | |||
| 359 | - | ||||
| 360 | - | // applyReloadState computes the expected state after a reload given the | |||
| 361 | - | // current query. When query is empty browse state is reset; when query is | |||
| 362 | - | // non-empty search is cleared and re-triggered. | |||
| 363 | - | func applyReloadState(currentQuery string) reloadPreservationSnapshot { | |||
| 364 | - | s := reloadPreservationSnapshot{ | |||
| 365 | - | PreservedQuery: currentQuery, | |||
| 366 | - | } | |||
| 367 | - | if currentQuery == "" { | |||
| 368 | - | s.ResetBrowse = true | |||
| 369 | - | } else { | |||
| 370 | - | s.ClearSearch = true | |||
| 371 | - | } | |||
| 372 | - | return s | |||
| 373 | - | } | |||
M internal/ui/component/history_model_test.go +9 -45
| @@ -41,6 +41,7 @@ func TestGroupHistoryByDay_TodayYesterdayOlder(t *testing.T) { | |||
| 41 | 41 | require.Len(t, groups, 3) | |
| 42 | 42 | assert.Equal(t, "Today", groups[0].Label) | |
| 43 | 43 | assert.Equal(t, "Yesterday", groups[1].Label) | |
| 44 | + | assert.NotEmpty(t, groups[2].Label, "older entry should have a formatted date label") | |
| 44 | 45 | assert.Len(t, groups[2].Entries, 1) | |
| 45 | 46 | } | |
| 46 | 47 | ||
| @@ -98,15 +99,10 @@ func TestGroupHistoryByDay_CrossYearDifferentLabels(t *testing.T) { | |||
| 98 | 99 | } | |
| 99 | 100 | groups := groupHistoryByDay(entries) | |
| 100 | 101 | require.Len(t, groups, 3) | |
| 101 | - | // Most recent entry label depends on whether it's today | |
| 102 | - | if now.YearDay() == thisYear.YearDay() && now.Year() == thisYear.Year() { | |
| 103 | - | assert.Equal(t, "Today", groups[0].Label) | |
| 104 | - | } | |
| 102 | + | assert.Equal(t, "Today", groups[0].Label) | |
| 103 | + | assert.Equal(t, lastYear.Format(dayLabelOtherYearFormat), groups[1].Label) | |
| 104 | + | assert.Equal(t, twoYearsAgo.Format(dayLabelOtherYearFormat), groups[2].Label) | |
| 105 | 105 | assert.Len(t, groups[2].Entries, 1) | |
| 106 | - | // Two-year-old entry should include year if not current year | |
| 107 | - | if twoYearsAgo.Year() != now.Year() && twoYearsAgo.Year() != now.Year()-1 { | |
| 108 | - | assert.Contains(t, groups[2].Label, twoYearsAgo.Format("2006"), "two-year-old entry should include year") | |
| 109 | - | } | |
| 110 | 106 | } | |
| 111 | 107 | ||
| 112 | 108 | func TestGroupHistoryByDay_MaintainsInputOrderWithinDay(t *testing.T) { | |
| @@ -214,12 +210,10 @@ func TestRelativeTime_Future(t *testing.T) { | |||
| 214 | 210 | func TestRelativeTime_DifferentYear(t *testing.T) { | |
| 215 | 211 | // An entry from a previous year should show a date with month abbreviation | |
| 216 | 212 | // and year when not current year. | |
| 217 | - | lastYear := time.Now().AddDate(-1, -1, 0) // >1 year ago, definitely a different year | |
| 213 | + | lastYear := time.Now().AddDate(-1, -1, 0) | |
| 218 | 214 | result := relativeTime(lastYear) | |
| 219 | - | if lastYear.Year() != time.Now().Year() { | |
| 220 | - | // Format: "Jan 2, 2006" — verify it's longer than a short relative label | |
| 221 | - | assert.Greater(t, len(result), 5, "old entry should return a date string, got %q", result) | |
| 222 | - | } | |
| 215 | + | _, err := time.Parse("Jan 2, 2006", result) | |
| 216 | + | assert.NoError(t, err, "old entry should use the Jan 2, 2006 layout, got %q", result) | |
| 223 | 217 | } | |
| 224 | 218 | ||
| 225 | 219 | func TestDayLabelForKey_MultiYearAgo(t *testing.T) { | |
| @@ -353,6 +347,7 @@ func TestKeyboardNavModel_InvalidDirection(t *testing.T) { | |||
| 353 | 347 | m := newKeyboardNavModel(groups) | |
| 354 | 348 | assert.Equal(t, -1, m.nextSelectableIndex(0, 0)) | |
| 355 | 349 | assert.Equal(t, -1, m.nextSelectableIndex(0, 2)) | |
| 350 | + | assert.Equal(t, -1, m.nextSelectableIndex(0, -2)) | |
| 356 | 351 | } | |
| 357 | 352 | ||
| 358 | 353 | func TestKeyboardNavModel_DayBoundaries(t *testing.T) { | |
| @@ -587,15 +582,6 @@ func TestKeyboardNavModel_NextPreviousSelectable_EmptyGroups(t *testing.T) { | |||
| 587 | 582 | assert.Equal(t, -1, m.nextSelectableIndex(0, -1)) | |
| 588 | 583 | } | |
| 589 | 584 | ||
| 590 | - | func TestKeyboardNavModel_NegativeDirectionReturnsAllNil(t *testing.T) { | |
| 591 | - | groups := makeGroups(2) | |
| 592 | - | m := newKeyboardNavModel(groups) | |
| 593 | - | ||
| 594 | - | assert.Equal(t, -1, m.nextSelectableIndex(0, 0)) | |
| 595 | - | assert.Equal(t, -1, m.nextSelectableIndex(0, 2)) | |
| 596 | - | assert.Equal(t, -1, m.nextSelectableIndex(0, -2)) | |
| 597 | - | } | |
| 598 | - | ||
| 599 | 585 | func TestKeyboardNavModel_EntryCountWithEmptyGroups(t *testing.T) { | |
| 600 | 586 | groups := []historyGroup{ | |
| 601 | 587 | {Label: "A", Entries: []*entity.HistoryEntry{{ | |
| @@ -666,25 +652,6 @@ func TestTransitionSearchState_SearchThenClearThenReSearch(t *testing.T) { | |||
| 666 | 652 | assert.Equal(t, 7, s.ResultCount) | |
| 667 | 653 | } | |
| 668 | 654 | ||
| 669 | - | // TestTransitionSearchState_LateResultAfterClear verifies that if a stale | |
| 670 | - | // search result arrives after the user cleared the query, the next state | |
| 671 | - | // correctly transitions. This is the pure-model equivalent of what happens | |
| 672 | - | // when searchGen protects against stale idle callbacks. | |
| 673 | - | func TestTransitionSearchState_LateResultAfterClear(t *testing.T) { | |
| 674 | - | // Current state: query cleared, no results. | |
| 675 | - | current := searchStateSnapshot{Query: "", HasSearchDone: false, HasResults: false, ResultCount: 0} | |
| 676 | - | ||
| 677 | - | // A late result from a stale search arrives with different query. | |
| 678 | - | // Production code guards against this via searchGen; the pure model | |
| 679 | - | // accepts it because it has no concept of staleness. The test documents | |
| 680 | - | // that the generation guard lives in the HistorySidebar callback. | |
| 681 | - | staleResult := transitionSearchState(current, "stale-query", 3) | |
| 682 | - | assert.Equal(t, "stale-query", staleResult.Query, "the pure model accepts the result; stale protection is in HistorySidebar") | |
| 683 | - | assert.True(t, staleResult.HasSearchDone) | |
| 684 | - | assert.True(t, staleResult.HasResults) | |
| 685 | - | assert.Equal(t, 3, staleResult.ResultCount) | |
| 686 | - | } | |
| 687 | - | ||
| 688 | 655 | // TestApplyReloadState_EmptyAndNonEmpty verifies all reload preservation | |
| 689 | 656 | // states are correctly modeled by the pure function. | |
| 690 | 657 | func TestApplyReloadState_EmptyAndNonEmpty(t *testing.T) { | |
| @@ -727,10 +694,7 @@ func TestDayLabelForKey_DifferentYears(t *testing.T) { | |||
| 727 | 694 | lastYear := now.AddDate(-1, 0, 0) | |
| 728 | 695 | key = dayKey{lastYear.Year(), lastYear.Month(), lastYear.Day()} | |
| 729 | 696 | label = dayLabelForKey(key, todayStart, now) | |
| 730 | - | // Should include weekday but not year in format | |
| 731 | - | if lastYear.Year() == now.Year()-1 { | |
| 732 | - | assert.NotContains(t, label, lastYear.Format("2006"), "last-year label should use short format") | |
| 733 | - | } | |
| 697 | + | assert.Equal(t, lastYear.Format(dayLabelOtherYearFormat), label) | |
| 734 | 698 | ||
| 735 | 699 | // Multiple years ago | |
| 736 | 700 | twoYearsAgo := now.AddDate(-2, 0, 0) | |
| @@ -41,6 +41,7 @@ func TestGroupHistoryByDay_TodayYesterdayOlder(t *testing.T) { | |||||
| 41 | require.Len(t, groups, 3) | 41 | require.Len(t, groups, 3) | ||
| 42 | assert.Equal(t, "Today", groups[0].Label) | 42 | assert.Equal(t, "Today", groups[0].Label) | ||
| 43 | assert.Equal(t, "Yesterday", groups[1].Label) | 43 | assert.Equal(t, "Yesterday", groups[1].Label) | ||
| 44 | + | assert.NotEmpty(t, groups[2].Label, "older entry should have a formatted date label") | |||
| 44 | assert.Len(t, groups[2].Entries, 1) | 45 | assert.Len(t, groups[2].Entries, 1) | ||
| 45 | } | 46 | } | ||
| 46 | 47 | ||||
| @@ -98,15 +99,10 @@ func TestGroupHistoryByDay_CrossYearDifferentLabels(t *testing.T) { | |||||
| 98 | } | 99 | } | ||
| 99 | groups := groupHistoryByDay(entries) | 100 | groups := groupHistoryByDay(entries) | ||
| 100 | require.Len(t, groups, 3) | 101 | require.Len(t, groups, 3) | ||
| 101 | - | // Most recent entry label depends on whether it's today | 102 | + | assert.Equal(t, "Today", groups[0].Label) |
| 102 | - | if now.YearDay() == thisYear.YearDay() && now.Year() == thisYear.Year() { | 103 | + | assert.Equal(t, lastYear.Format(dayLabelOtherYearFormat), groups[1].Label) |
| 103 | - | assert.Equal(t, "Today", groups[0].Label) | 104 | + | assert.Equal(t, twoYearsAgo.Format(dayLabelOtherYearFormat), groups[2].Label) |
| 104 | - | } | |||
| 105 | assert.Len(t, groups[2].Entries, 1) | 105 | assert.Len(t, groups[2].Entries, 1) | ||
| 106 | - | // Two-year-old entry should include year if not current year | |||
| 107 | - | if twoYearsAgo.Year() != now.Year() && twoYearsAgo.Year() != now.Year()-1 { | |||
| 108 | - | assert.Contains(t, groups[2].Label, twoYearsAgo.Format("2006"), "two-year-old entry should include year") | |||
| 109 | - | } | |||
| 110 | } | 106 | } | ||
| 111 | 107 | ||||
| 112 | func TestGroupHistoryByDay_MaintainsInputOrderWithinDay(t *testing.T) { | 108 | func TestGroupHistoryByDay_MaintainsInputOrderWithinDay(t *testing.T) { | ||
| @@ -214,12 +210,10 @@ func TestRelativeTime_Future(t *testing.T) { | |||||
| 214 | func TestRelativeTime_DifferentYear(t *testing.T) { | 210 | func TestRelativeTime_DifferentYear(t *testing.T) { | ||
| 215 | // An entry from a previous year should show a date with month abbreviation | 211 | // An entry from a previous year should show a date with month abbreviation | ||
| 216 | // and year when not current year. | 212 | // and year when not current year. | ||
| 217 | - | lastYear := time.Now().AddDate(-1, -1, 0) // >1 year ago, definitely a different year | 213 | + | lastYear := time.Now().AddDate(-1, -1, 0) |
| 218 | result := relativeTime(lastYear) | 214 | result := relativeTime(lastYear) | ||
| 219 | - | if lastYear.Year() != time.Now().Year() { | 215 | + | _, err := time.Parse("Jan 2, 2006", result) |
| 220 | - | // Format: "Jan 2, 2006" — verify it's longer than a short relative label | 216 | + | assert.NoError(t, err, "old entry should use the Jan 2, 2006 layout, got %q", result) |
| 221 | - | assert.Greater(t, len(result), 5, "old entry should return a date string, got %q", result) | |||
| 222 | - | } | |||
| 223 | } | 217 | } | ||
| 224 | 218 | ||||
| 225 | func TestDayLabelForKey_MultiYearAgo(t *testing.T) { | 219 | func TestDayLabelForKey_MultiYearAgo(t *testing.T) { | ||
| @@ -353,6 +347,7 @@ func TestKeyboardNavModel_InvalidDirection(t *testing.T) { | |||||
| 353 | m := newKeyboardNavModel(groups) | 347 | m := newKeyboardNavModel(groups) | ||
| 354 | assert.Equal(t, -1, m.nextSelectableIndex(0, 0)) | 348 | assert.Equal(t, -1, m.nextSelectableIndex(0, 0)) | ||
| 355 | assert.Equal(t, -1, m.nextSelectableIndex(0, 2)) | 349 | assert.Equal(t, -1, m.nextSelectableIndex(0, 2)) | ||
| 350 | + | assert.Equal(t, -1, m.nextSelectableIndex(0, -2)) | |||
| 356 | } | 351 | } | ||
| 357 | 352 | ||||
| 358 | func TestKeyboardNavModel_DayBoundaries(t *testing.T) { | 353 | func TestKeyboardNavModel_DayBoundaries(t *testing.T) { | ||
| @@ -587,15 +582,6 @@ func TestKeyboardNavModel_NextPreviousSelectable_EmptyGroups(t *testing.T) { | |||||
| 587 | assert.Equal(t, -1, m.nextSelectableIndex(0, -1)) | 582 | assert.Equal(t, -1, m.nextSelectableIndex(0, -1)) | ||
| 588 | } | 583 | } | ||
| 589 | 584 | ||||
| 590 | - | func TestKeyboardNavModel_NegativeDirectionReturnsAllNil(t *testing.T) { | |||
| 591 | - | groups := makeGroups(2) | |||
| 592 | - | m := newKeyboardNavModel(groups) | |||
| 593 | - | ||||
| 594 | - | assert.Equal(t, -1, m.nextSelectableIndex(0, 0)) | |||
| 595 | - | assert.Equal(t, -1, m.nextSelectableIndex(0, 2)) | |||
| 596 | - | assert.Equal(t, -1, m.nextSelectableIndex(0, -2)) | |||
| 597 | - | } | |||
| 598 | - | ||||
| 599 | func TestKeyboardNavModel_EntryCountWithEmptyGroups(t *testing.T) { | 585 | func TestKeyboardNavModel_EntryCountWithEmptyGroups(t *testing.T) { | ||
| 600 | groups := []historyGroup{ | 586 | groups := []historyGroup{ | ||
| 601 | {Label: "A", Entries: []*entity.HistoryEntry{{ | 587 | {Label: "A", Entries: []*entity.HistoryEntry{{ | ||
| @@ -666,25 +652,6 @@ func TestTransitionSearchState_SearchThenClearThenReSearch(t *testing.T) { | |||||
| 666 | assert.Equal(t, 7, s.ResultCount) | 652 | assert.Equal(t, 7, s.ResultCount) | ||
| 667 | } | 653 | } | ||
| 668 | 654 | ||||
| 669 | - | // TestTransitionSearchState_LateResultAfterClear verifies that if a stale | |||
| 670 | - | // search result arrives after the user cleared the query, the next state | |||
| 671 | - | // correctly transitions. This is the pure-model equivalent of what happens | |||
| 672 | - | // when searchGen protects against stale idle callbacks. | |||
| 673 | - | func TestTransitionSearchState_LateResultAfterClear(t *testing.T) { | |||
| 674 | - | // Current state: query cleared, no results. | |||
| 675 | - | current := searchStateSnapshot{Query: "", HasSearchDone: false, HasResults: false, ResultCount: 0} | |||
| 676 | - | ||||
| 677 | - | // A late result from a stale search arrives with different query. | |||
| 678 | - | // Production code guards against this via searchGen; the pure model | |||
| 679 | - | // accepts it because it has no concept of staleness. The test documents | |||
| 680 | - | // that the generation guard lives in the HistorySidebar callback. | |||
| 681 | - | staleResult := transitionSearchState(current, "stale-query", 3) | |||
| 682 | - | assert.Equal(t, "stale-query", staleResult.Query, "the pure model accepts the result; stale protection is in HistorySidebar") | |||
| 683 | - | assert.True(t, staleResult.HasSearchDone) | |||
| 684 | - | assert.True(t, staleResult.HasResults) | |||
| 685 | - | assert.Equal(t, 3, staleResult.ResultCount) | |||
| 686 | - | } | |||
| 687 | - | ||||
| 688 | // TestApplyReloadState_EmptyAndNonEmpty verifies all reload preservation | 655 | // TestApplyReloadState_EmptyAndNonEmpty verifies all reload preservation | ||
| 689 | // states are correctly modeled by the pure function. | 656 | // states are correctly modeled by the pure function. | ||
| 690 | func TestApplyReloadState_EmptyAndNonEmpty(t *testing.T) { | 657 | func TestApplyReloadState_EmptyAndNonEmpty(t *testing.T) { | ||
| @@ -727,10 +694,7 @@ func TestDayLabelForKey_DifferentYears(t *testing.T) { | |||||
| 727 | lastYear := now.AddDate(-1, 0, 0) | 694 | lastYear := now.AddDate(-1, 0, 0) | ||
| 728 | key = dayKey{lastYear.Year(), lastYear.Month(), lastYear.Day()} | 695 | key = dayKey{lastYear.Year(), lastYear.Month(), lastYear.Day()} | ||
| 729 | label = dayLabelForKey(key, todayStart, now) | 696 | label = dayLabelForKey(key, todayStart, now) | ||
| 730 | - | // Should include weekday but not year in format | 697 | + | assert.Equal(t, lastYear.Format(dayLabelOtherYearFormat), label) |
| 731 | - | if lastYear.Year() == now.Year()-1 { | |||
| 732 | - | assert.NotContains(t, label, lastYear.Format("2006"), "last-year label should use short format") | |||
| 733 | - | } | |||
| 734 | 698 | ||||
| 735 | // Multiple years ago | 699 | // Multiple years ago | ||
| 736 | twoYearsAgo := now.AddDate(-2, 0, 0) | 700 | twoYearsAgo := now.AddDate(-2, 0, 0) | ||
M internal/ui/component/history_sidebar.go +101 -213
| @@ -196,16 +196,16 @@ func (hs *HistorySidebar) Destroy() { | |||
| 196 | 196 | return | |
| 197 | 197 | } | |
| 198 | 198 | hs.destroyed = true | |
| 199 | + | timerID := hs.debounceTimer | |
| 200 | + | hs.debounceTimer = 0 | |
| 199 | 201 | hs.mu.Unlock() | |
| 200 | 202 | ||
| 201 | 203 | if hs.cancel != nil { | |
| 202 | 204 | hs.cancel() | |
| 203 | 205 | } | |
| 204 | 206 | ||
| 205 | - | // Cancel pending debounce | |
| 206 | - | if hs.debounceTimer != 0 { | |
| 207 | - | glib.SourceRemove(hs.debounceTimer) | |
| 208 | - | hs.debounceTimer = 0 | |
| 207 | + | if timerID != 0 { | |
| 208 | + | glib.SourceRemove(timerID) | |
| 209 | 209 | } | |
| 210 | 210 | } | |
| 211 | 211 | ||
| @@ -578,7 +578,7 @@ func (hs *HistorySidebar) preserveScrollAndSelection() { | |||
| 578 | 578 | } | |
| 579 | 579 | if hs.listBox != nil { | |
| 580 | 580 | if selected := hs.listBox.GetSelectedRow(); selected != nil { | |
| 581 | - | if url := hs.getRowURL(selected); url != "" { | |
| 581 | + | if url := hs.entryURLAtIndex(selected.GetIndex()); url != "" { | |
| 582 | 582 | hs.prevSelectedURL = url | |
| 583 | 583 | } | |
| 584 | 584 | } | |
| @@ -633,19 +633,7 @@ func (hs *HistorySidebar) getRowURL(row *gtk.ListBoxRow) string { | |||
| 633 | 633 | // entryURLAtIndex returns the URL of the history entry at the given | |
| 634 | 634 | // linear list index (including group headers which return ""). | |
| 635 | 635 | func (hs *HistorySidebar) entryURLAtIndex(index int) string { | |
| 636 | - | linearEntryIdx := 0 | |
| 637 | - | for _, group := range hs.groups { | |
| 638 | - | if index == linearEntryIdx { | |
| 639 | - | return "" // header | |
| 640 | - | } | |
| 641 | - | linearEntryIdx++ // Skip header | |
| 642 | - | ||
| 643 | - | if index < linearEntryIdx+len(group.Entries) { | |
| 644 | - | return group.Entries[index-linearEntryIdx].URL | |
| 645 | - | } | |
| 646 | - | linearEntryIdx += len(group.Entries) | |
| 647 | - | } | |
| 648 | - | return "" | |
| 636 | + | return newKeyboardNavModel(hs.groups).entryURLAt(index) | |
| 649 | 637 | } | |
| 650 | 638 | ||
| 651 | 639 | // selectRowByURL finds and selects a row whose URL matches. | |
| @@ -686,19 +674,36 @@ func (hs *HistorySidebar) setupSearchHandler() { | |||
| 686 | 674 | ||
| 687 | 675 | func (hs *HistorySidebar) onSearchChanged() { | |
| 688 | 676 | hs.mu.Lock() | |
| 677 | + | if hs.destroyed { | |
| 678 | + | hs.mu.Unlock() | |
| 679 | + | return | |
| 680 | + | } | |
| 689 | 681 | hs.currentQuery = hs.searchEntry.GetText() | |
| 690 | 682 | hs.preserveScrollAndSelection() | |
| 683 | + | oldTimer := hs.debounceTimer | |
| 684 | + | hs.debounceTimer = 0 | |
| 691 | 685 | hs.mu.Unlock() | |
| 692 | 686 | ||
| 693 | - | // Debounce filtering/search | |
| 694 | - | if hs.debounceTimer != 0 { | |
| 695 | - | glib.SourceRemove(hs.debounceTimer) | |
| 687 | + | if oldTimer != 0 { | |
| 688 | + | glib.SourceRemove(oldTimer) | |
| 696 | 689 | } | |
| 690 | + | ||
| 697 | 691 | filterCb := glib.SourceFunc(func(uintptr) bool { | |
| 698 | 692 | hs.applyFilter() | |
| 699 | 693 | return false | |
| 700 | 694 | }) | |
| 701 | - | hs.debounceTimer = glib.TimeoutAdd(uint(sidebarSearchDebounceMs), &filterCb, 0) | |
| 695 | + | timerID := glib.TimeoutAdd(uint(sidebarSearchDebounceMs), &filterCb, 0) | |
| 696 | + | ||
| 697 | + | hs.mu.Lock() | |
| 698 | + | if hs.destroyed { | |
| 699 | + | hs.mu.Unlock() | |
| 700 | + | if timerID != 0 { | |
| 701 | + | glib.SourceRemove(timerID) | |
| 702 | + | } | |
| 703 | + | return | |
| 704 | + | } | |
| 705 | + | hs.debounceTimer = timerID | |
| 706 | + | hs.mu.Unlock() | |
| 702 | 707 | } | |
| 703 | 708 | ||
| 704 | 709 | func (hs *HistorySidebar) applyFilter() { | |
| @@ -1176,58 +1181,49 @@ func (hs *HistorySidebar) handleDeleteKey() bool { | |||
| 1176 | 1181 | if row == nil || !row.GetSelectable() { | |
| 1177 | 1182 | return false | |
| 1178 | 1183 | } | |
| 1184 | + | if hs.historyUC == nil { | |
| 1185 | + | return false | |
| 1186 | + | } | |
| 1179 | 1187 | ||
| 1180 | 1188 | idx := row.GetIndex() | |
| 1181 | - | // Find the entry URL and ID from the in-memory groups under one lock. | |
| 1189 | + | ||
| 1182 | 1190 | hs.mu.RLock() | |
| 1183 | 1191 | url := hs.entryURLAtIndex(idx) | |
| 1184 | 1192 | entryID := hs.findEntryIDByIndex(idx) | |
| 1193 | + | nextSelectedURL := "" | |
| 1194 | + | if nextRow := hs.findNextSelectableAfter(idx); nextRow != -1 { | |
| 1195 | + | nextSelectedURL = hs.entryURLAtIndex(nextRow) | |
| 1196 | + | } | |
| 1185 | 1197 | hs.mu.RUnlock() | |
| 1186 | 1198 | ||
| 1187 | 1199 | if url == "" || entryID <= 0 { | |
| 1188 | 1200 | return false | |
| 1189 | 1201 | } | |
| 1190 | 1202 | ||
| 1191 | - | // Find the next row to select before deletion | |
| 1192 | - | nextRow := hs.findNextSelectableAfter(idx) | |
| 1193 | - | ||
| 1194 | - | // Delete via the search history use case | |
| 1195 | - | cb := glib.SourceFunc(func(uintptr) bool { | |
| 1196 | - | if hs.historyUC == nil { | |
| 1197 | - | return false | |
| 1198 | - | } | |
| 1203 | + | go func() { | |
| 1199 | 1204 | if err := hs.historyUC.Delete(hs.ctx, entryID); err != nil { | |
| 1200 | 1205 | hs.logger.Error().Err(err).Int64("entry_id", entryID).Msg("failed to delete history entry") | |
| 1201 | - | return false | |
| 1206 | + | return | |
| 1202 | 1207 | } | |
| 1203 | - | return false | |
| 1204 | - | }) | |
| 1205 | - | glib.IdleAdd(&cb, 0) | |
| 1206 | - | ||
| 1207 | - | // Remove the entry from local data and rebuild the list. | |
| 1208 | - | // Must also remove from allEntries and searchResults so the | |
| 1209 | - | // deleted entry does not reappear after rebuildLocalGroups | |
| 1210 | - | // (which re-groups from allEntries). | |
| 1211 | - | hs.mu.Lock() | |
| 1212 | - | hs.removeEntryByIndex(idx) | |
| 1213 | - | hs.removeFromAllEntries(url, entryID) | |
| 1214 | - | hs.removeFromSearchResults(entryID) | |
| 1215 | - | hs.rebuildLocalGroups() | |
| 1216 | - | hs.mu.Unlock() | |
| 1217 | 1208 | ||
| 1218 | - | hs.scheduleRebuild() | |
| 1219 | - | ||
| 1220 | - | // After rebuild, select the next row | |
| 1221 | - | // (scheduled after rebuild to ensure rows exist) | |
| 1222 | - | if nextRow != -1 { | |
| 1223 | - | selectCb := glib.SourceFunc(func(uintptr) bool { | |
| 1224 | - | if target := hs.listBox.GetRowAtIndex(nextRow); target != nil { | |
| 1225 | - | hs.listBox.SelectRow(target) | |
| 1209 | + | cb := glib.SourceFunc(func(uintptr) bool { | |
| 1210 | + | hs.mu.Lock() | |
| 1211 | + | if hs.destroyed { | |
| 1212 | + | hs.mu.Unlock() | |
| 1213 | + | return false | |
| 1226 | 1214 | } | |
| 1215 | + | hs.preserveScrollAndSelection() | |
| 1216 | + | hs.prevSelectedURL = nextSelectedURL | |
| 1217 | + | hs.removeFromAllEntries(url, entryID) | |
| 1218 | + | hs.removeFromSearchResults(entryID) | |
| 1219 | + | hs.rebuildLocalGroups() | |
| 1220 | + | hs.mu.Unlock() | |
| 1221 | + | ||
| 1222 | + | hs.rebuildList() | |
| 1227 | 1223 | return false | |
| 1228 | 1224 | }) | |
| 1229 | - | glib.IdleAdd(&selectCb, 0) | |
| 1230 | - | } | |
| 1225 | + | glib.IdleAdd(&cb, 0) | |
| 1226 | + | }() | |
| 1231 | 1227 | ||
| 1232 | 1228 | return true | |
| 1233 | 1229 | } | |
| @@ -1235,37 +1231,11 @@ func (hs *HistorySidebar) handleDeleteKey() bool { | |||
| 1235 | 1231 | // findEntryIDByIndex returns the entry ID for the linear ListBox index. | |
| 1236 | 1232 | // Must be called with hs.mu read lock held. | |
| 1237 | 1233 | func (hs *HistorySidebar) findEntryIDByIndex(index int) int64 { | |
| 1238 | - | linearEntryIdx := 0 | |
| 1239 | - | for _, group := range hs.groups { | |
| 1240 | - | if index == linearEntryIdx { | |
| 1241 | - | return 0 // header row | |
| 1242 | - | } | |
| 1243 | - | linearEntryIdx++ | |
| 1244 | - | if index < linearEntryIdx+len(group.Entries) { | |
| 1245 | - | return group.Entries[index-linearEntryIdx].ID | |
| 1246 | - | } | |
| 1247 | - | linearEntryIdx += len(group.Entries) | |
| 1248 | - | } | |
| 1249 | - | return 0 | |
| 1250 | - | } | |
| 1251 | - | ||
| 1252 | - | // removeEntryByIndex removes an entry from the groups slice by linear index. | |
| 1253 | - | // Must be called with hs.mu write lock held. | |
| 1254 | - | func (hs *HistorySidebar) removeEntryByIndex(index int) { | |
| 1255 | - | linearEntryIdx := 0 | |
| 1256 | - | for gi, group := range hs.groups { | |
| 1257 | - | linearEntryIdx++ // skip header | |
| 1258 | - | if index >= linearEntryIdx && index < linearEntryIdx+len(group.Entries) { | |
| 1259 | - | entryIdx := index - linearEntryIdx | |
| 1260 | - | hs.groups[gi].Entries = append(group.Entries[:entryIdx], group.Entries[entryIdx+1:]...) | |
| 1261 | - | // Remove empty groups | |
| 1262 | - | if len(hs.groups[gi].Entries) == 0 { | |
| 1263 | - | hs.groups = append(hs.groups[:gi], hs.groups[gi+1:]...) | |
| 1264 | - | } | |
| 1265 | - | return | |
| 1266 | - | } | |
| 1267 | - | linearEntryIdx += len(group.Entries) | |
| 1234 | + | entry := newKeyboardNavModel(hs.groups).entryAt(index) | |
| 1235 | + | if entry == nil { | |
| 1236 | + | return 0 | |
| 1268 | 1237 | } | |
| 1238 | + | return entry.ID | |
| 1269 | 1239 | } | |
| 1270 | 1240 | ||
| 1271 | 1241 | // rebuildLocalGroups rebuilds hs.groups from the current allEntries and query. | |
| @@ -1313,28 +1283,17 @@ func (hs *HistorySidebar) removeFromSearchResults(id int64) { | |||
| 1313 | 1283 | } | |
| 1314 | 1284 | ||
| 1315 | 1285 | // findNextSelectableAfter returns the ListBox index of the next selectable | |
| 1316 | - | // row after the given index, preferring the same position then previous. | |
| 1286 | + | // row after the given index, falling back to the previous selectable row. | |
| 1287 | + | // Must be called with hs.mu read lock held. | |
| 1317 | 1288 | func (hs *HistorySidebar) findNextSelectableAfter(idx int) int { | |
| 1318 | - | hs.mu.RLock() | |
| 1319 | - | defer hs.mu.RUnlock() | |
| 1320 | - | ||
| 1321 | - | total := 0 | |
| 1322 | - | for _, group := range hs.groups { | |
| 1323 | - | total++ // header | |
| 1324 | - | total += len(group.Entries) | |
| 1325 | - | } | |
| 1326 | - | ||
| 1327 | - | // Try same position first | |
| 1328 | - | candidate := idx | |
| 1329 | - | // Adjust for next rebuild (lose 1 entry, possibly a group header) | |
| 1330 | - | // Worst case: we just pick (idx) if within range | |
| 1331 | - | if candidate >= total-1 { | |
| 1332 | - | candidate = total - 2 | |
| 1289 | + | model := newKeyboardNavModel(hs.groups) | |
| 1290 | + | if next := model.nextSelectableIndex(idx, +1); next != -1 { | |
| 1291 | + | return next | |
| 1333 | 1292 | } | |
| 1334 | - | if candidate < 0 { | |
| 1335 | - | candidate = 0 | |
| 1293 | + | if prev := model.nextSelectableIndex(idx, -1); prev != -1 { | |
| 1294 | + | return prev | |
| 1336 | 1295 | } | |
| 1337 | - | return candidate | |
| 1296 | + | return -1 | |
| 1338 | 1297 | } | |
| 1339 | 1298 | ||
| 1340 | 1299 | // scrollByPage scrolls the list by one page up or down, | |
| @@ -1370,36 +1329,19 @@ func (hs *HistorySidebar) jumpToPreviousDay() { | |||
| 1370 | 1329 | currentIdx = row.GetIndex() | |
| 1371 | 1330 | } | |
| 1372 | 1331 | ||
| 1373 | - | // Walk backwards through rows to find the previous group header, | |
| 1374 | - | // then select the first entry after it. | |
| 1375 | - | prevHeaderIdx := -1 | |
| 1376 | - | for i := currentIdx - 1; i >= 0; i-- { | |
| 1377 | - | row := hs.listBox.GetRowAtIndex(i) | |
| 1378 | - | if row == nil { | |
| 1379 | - | break | |
| 1380 | - | } | |
| 1381 | - | if !row.GetSelectable() { | |
| 1382 | - | prevHeaderIdx = i | |
| 1383 | - | break | |
| 1384 | - | } | |
| 1385 | - | } | |
| 1386 | - | ||
| 1387 | - | if prevHeaderIdx == -1 { | |
| 1388 | - | // No previous group; try the very first row (header or entry) | |
| 1332 | + | hs.mu.RLock() | |
| 1333 | + | targetIdx := newKeyboardNavModel(hs.groups).previousDayBoundary(currentIdx) | |
| 1334 | + | hs.mu.RUnlock() | |
| 1335 | + | if targetIdx == -1 { | |
| 1389 | 1336 | hs.jumpToFirstSelectable() | |
| 1390 | 1337 | return | |
| 1391 | 1338 | } | |
| 1392 | - | ||
| 1393 | - | // Select the first entry after the previous header | |
| 1394 | - | firstEntryIdx := prevHeaderIdx + 1 | |
| 1395 | - | if row := hs.listBox.GetRowAtIndex(firstEntryIdx); row != nil && row.GetSelectable() { | |
| 1339 | + | if row := hs.listBox.GetRowAtIndex(targetIdx); row != nil && row.GetSelectable() { | |
| 1396 | 1340 | hs.listBox.SelectRow(row) | |
| 1397 | - | // Ensure the selected row is scrolled into view | |
| 1398 | - | hs.scrollRowIntoView(firstEntryIdx) | |
| 1399 | - | } else { | |
| 1400 | - | // No entry in this group | |
| 1401 | - | hs.jumpToFirstSelectable() | |
| 1341 | + | hs.scrollRowIntoView(targetIdx) | |
| 1342 | + | return | |
| 1402 | 1343 | } | |
| 1344 | + | hs.jumpToFirstSelectable() | |
| 1403 | 1345 | } | |
| 1404 | 1346 | ||
| 1405 | 1347 | // jumpToNextDay selects the first entry in the next day group | |
| @@ -1410,43 +1352,19 @@ func (hs *HistorySidebar) jumpToNextDay() { | |||
| 1410 | 1352 | currentIdx = row.GetIndex() | |
| 1411 | 1353 | } | |
| 1412 | 1354 | ||
| 1413 | - | totalRows := 0 | |
| 1414 | - | for { | |
| 1415 | - | if hs.listBox.GetRowAtIndex(totalRows) == nil { | |
| 1416 | - | break | |
| 1417 | - | } | |
| 1418 | - | totalRows++ | |
| 1419 | - | } | |
| 1420 | - | ||
| 1421 | - | // Walk forwards through rows to find the next group header, | |
| 1422 | - | // then select the first entry after it. | |
| 1423 | - | nextHeaderIdx := -1 | |
| 1424 | - | for i := currentIdx + 1; i < totalRows; i++ { | |
| 1425 | - | row := hs.listBox.GetRowAtIndex(i) | |
| 1426 | - | if row == nil { | |
| 1427 | - | break | |
| 1428 | - | } | |
| 1429 | - | if !row.GetSelectable() { | |
| 1430 | - | nextHeaderIdx = i | |
| 1431 | - | break | |
| 1432 | - | } | |
| 1433 | - | } | |
| 1434 | - | ||
| 1435 | - | if nextHeaderIdx == -1 { | |
| 1436 | - | // No next group; jump to last selectable entry | |
| 1355 | + | hs.mu.RLock() | |
| 1356 | + | targetIdx := newKeyboardNavModel(hs.groups).nextDayBoundary(currentIdx) | |
| 1357 | + | hs.mu.RUnlock() | |
| 1358 | + | if targetIdx == -1 { | |
| 1437 | 1359 | hs.jumpToLastSelectable() | |
| 1438 | 1360 | return | |
| 1439 | 1361 | } | |
| 1440 | - | ||
| 1441 | - | // Select the first entry after the next header | |
| 1442 | - | firstEntryIdx := nextHeaderIdx + 1 | |
| 1443 | - | if row := hs.listBox.GetRowAtIndex(firstEntryIdx); row != nil && row.GetSelectable() { | |
| 1362 | + | if row := hs.listBox.GetRowAtIndex(targetIdx); row != nil && row.GetSelectable() { | |
| 1444 | 1363 | hs.listBox.SelectRow(row) | |
| 1445 | - | hs.scrollRowIntoView(firstEntryIdx) | |
| 1446 | - | } else { | |
| 1447 | - | // Empty group header? | |
| 1448 | - | hs.jumpToLastSelectable() | |
| 1364 | + | hs.scrollRowIntoView(targetIdx) | |
| 1365 | + | return | |
| 1449 | 1366 | } | |
| 1367 | + | hs.jumpToLastSelectable() | |
| 1450 | 1368 | } | |
| 1451 | 1369 | ||
| 1452 | 1370 | // scrollRowIntoView scrolls the scrolled window to ensure the row at | |
| @@ -1540,44 +1458,32 @@ func (hs *HistorySidebar) selectAdjacentRow(direction int) { | |||
| 1540 | 1458 | return | |
| 1541 | 1459 | } | |
| 1542 | 1460 | ||
| 1543 | - | totalRows := 0 | |
| 1544 | - | for { | |
| 1545 | - | if hs.listBox.GetRowAtIndex(totalRows) == nil { | |
| 1546 | - | break | |
| 1547 | - | } | |
| 1548 | - | totalRows++ | |
| 1549 | - | } | |
| 1550 | - | if totalRows == 0 { | |
| 1551 | - | return | |
| 1552 | - | } | |
| 1553 | - | ||
| 1554 | 1461 | current := -1 | |
| 1555 | 1462 | if row := hs.listBox.GetSelectedRow(); row != nil { | |
| 1556 | 1463 | current = row.GetIndex() | |
| 1557 | 1464 | } | |
| 1558 | 1465 | ||
| 1559 | - | // Nothing selected yet — pick first/last depending on direction. | |
| 1466 | + | hs.mu.RLock() | |
| 1467 | + | model := newKeyboardNavModel(hs.groups) | |
| 1468 | + | target := -1 | |
| 1560 | 1469 | if current < 0 { | |
| 1561 | 1470 | if direction > 0 { | |
| 1562 | - | hs.jumpToFirstSelectable() | |
| 1471 | + | target = model.firstSelectableIndex() | |
| 1563 | 1472 | } else { | |
| 1564 | - | hs.jumpToLastSelectable() | |
| 1473 | + | target = model.lastSelectableIndex() | |
| 1565 | 1474 | } | |
| 1566 | - | return | |
| 1475 | + | } else { | |
| 1476 | + | target = model.nextSelectableIndex(current, direction) | |
| 1567 | 1477 | } | |
| 1478 | + | hs.mu.RUnlock() | |
| 1568 | 1479 | ||
| 1569 | - | // Walk in the given direction to find the next selectable row. | |
| 1570 | - | candidate := current + direction | |
| 1571 | - | for candidate >= 0 && candidate < totalRows { | |
| 1572 | - | row := hs.listBox.GetRowAtIndex(candidate) | |
| 1573 | - | if row != nil && row.GetSelectable() { | |
| 1574 | - | hs.listBox.SelectRow(row) | |
| 1575 | - | hs.ensureRowVisible(candidate) | |
| 1576 | - | return | |
| 1577 | - | } | |
| 1578 | - | candidate += direction | |
| 1480 | + | if target == -1 { | |
| 1481 | + | return | |
| 1482 | + | } | |
| 1483 | + | if row := hs.listBox.GetRowAtIndex(target); row != nil && row.GetSelectable() { | |
| 1484 | + | hs.listBox.SelectRow(row) | |
| 1485 | + | hs.ensureRowVisible(target) | |
| 1579 | 1486 | } | |
| 1580 | - | // No more selectable rows in this direction; selection unchanged. | |
| 1581 | 1487 | } | |
| 1582 | 1488 | ||
| 1583 | 1489 | // ensureRowVisible adjusts the scrolled window so the row at index is | |
| @@ -1645,31 +1551,13 @@ func (hs *HistorySidebar) onRowActivated(row *gtk.ListBoxRow) { | |||
| 1645 | 1551 | hs.mu.RUnlock() | |
| 1646 | 1552 | return | |
| 1647 | 1553 | } | |
| 1648 | - | ||
| 1649 | - | index := row.GetIndex() | |
| 1650 | - | ||
| 1651 | - | // Compute entry index across all groups, excluding header rows. | |
| 1652 | - | // ListBox indices: for each group, 1 header row + N entry rows. | |
| 1653 | - | linearEntryIdx := 0 | |
| 1654 | - | for _, group := range hs.groups { | |
| 1655 | - | // Check if index falls on the header row for this group | |
| 1656 | - | if index == linearEntryIdx { | |
| 1657 | - | // Header row - not an activatable entry | |
| 1658 | - | hs.mu.RUnlock() | |
| 1659 | - | return | |
| 1660 | - | } | |
| 1661 | - | linearEntryIdx++ // Skip header | |
| 1662 | - | ||
| 1663 | - | if index < linearEntryIdx+len(group.Entries) { | |
| 1664 | - | entry := group.Entries[index-linearEntryIdx] | |
| 1665 | - | url := entry.URL | |
| 1666 | - | hs.mu.RUnlock() | |
| 1667 | - | hs.navigateToURL(url) | |
| 1668 | - | return | |
| 1669 | - | } | |
| 1670 | - | linearEntryIdx += len(group.Entries) | |
| 1671 | - | } | |
| 1554 | + | entry := newKeyboardNavModel(hs.groups).entryAt(row.GetIndex()) | |
| 1672 | 1555 | hs.mu.RUnlock() | |
| 1556 | + | if entry == nil || entry.URL == "" { | |
| 1557 | + | return | |
| 1558 | + | } | |
| 1559 | + | ||
| 1560 | + | hs.navigateToURL(entry.URL) | |
| 1673 | 1561 | } | |
| 1674 | 1562 | ||
| 1675 | 1563 | func (hs *HistorySidebar) navigateToURL(url string) { | |
| @@ -1687,7 +1575,7 @@ func (hs *HistorySidebar) navigateToURL(url string) { | |||
| 1687 | 1575 | // navigateWithoutClosing navigates to the URL but does NOT close the sidebar. | |
| 1688 | 1576 | // Used by Ctrl+Enter activation. | |
| 1689 | 1577 | func (hs *HistorySidebar) navigateWithoutClosing(url string) { | |
| 1690 | - | if hs.onURL == nil || url == "" { | |
| 1578 | + | if hs.onNavigateKeepOpen == nil || url == "" { | |
| 1691 | 1579 | return | |
| 1692 | 1580 | } | |
| 1693 | 1581 | ||
| @@ -196,16 +196,16 @@ func (hs *HistorySidebar) Destroy() { | |||||
| 196 | return | 196 | return | ||
| 197 | } | 197 | } | ||
| 198 | hs.destroyed = true | 198 | hs.destroyed = true | ||
| 199 | + | timerID := hs.debounceTimer | |||
| 200 | + | hs.debounceTimer = 0 | |||
| 199 | hs.mu.Unlock() | 201 | hs.mu.Unlock() | ||
| 200 | 202 | ||||
| 201 | if hs.cancel != nil { | 203 | if hs.cancel != nil { | ||
| 202 | hs.cancel() | 204 | hs.cancel() | ||
| 203 | } | 205 | } | ||
| 204 | 206 | ||||
| 205 | - | // Cancel pending debounce | 207 | + | if timerID != 0 { |
| 206 | - | if hs.debounceTimer != 0 { | 208 | + | glib.SourceRemove(timerID) |
| 207 | - | glib.SourceRemove(hs.debounceTimer) | |||
| 208 | - | hs.debounceTimer = 0 | |||
| 209 | } | 209 | } | ||
| 210 | } | 210 | } | ||
| 211 | 211 | ||||
| @@ -578,7 +578,7 @@ func (hs *HistorySidebar) preserveScrollAndSelection() { | |||||
| 578 | } | 578 | } | ||
| 579 | if hs.listBox != nil { | 579 | if hs.listBox != nil { | ||
| 580 | if selected := hs.listBox.GetSelectedRow(); selected != nil { | 580 | if selected := hs.listBox.GetSelectedRow(); selected != nil { | ||
| 581 | - | if url := hs.getRowURL(selected); url != "" { | 581 | + | if url := hs.entryURLAtIndex(selected.GetIndex()); url != "" { |
| 582 | hs.prevSelectedURL = url | 582 | hs.prevSelectedURL = url | ||
| 583 | } | 583 | } | ||
| 584 | } | 584 | } | ||
| @@ -633,19 +633,7 @@ func (hs *HistorySidebar) getRowURL(row *gtk.ListBoxRow) string { | |||||
| 633 | // entryURLAtIndex returns the URL of the history entry at the given | 633 | // entryURLAtIndex returns the URL of the history entry at the given | ||
| 634 | // linear list index (including group headers which return ""). | 634 | // linear list index (including group headers which return ""). | ||
| 635 | func (hs *HistorySidebar) entryURLAtIndex(index int) string { | 635 | func (hs *HistorySidebar) entryURLAtIndex(index int) string { | ||
| 636 | - | linearEntryIdx := 0 | 636 | + | return newKeyboardNavModel(hs.groups).entryURLAt(index) |
| 637 | - | for _, group := range hs.groups { | |||
| 638 | - | if index == linearEntryIdx { | |||
| 639 | - | return "" // header | |||
| 640 | - | } | |||
| 641 | - | linearEntryIdx++ // Skip header | |||
| 642 | - | ||||
| 643 | - | if index < linearEntryIdx+len(group.Entries) { | |||
| 644 | - | return group.Entries[index-linearEntryIdx].URL | |||
| 645 | - | } | |||
| 646 | - | linearEntryIdx += len(group.Entries) | |||
| 647 | - | } | |||
| 648 | - | return "" | |||
| 649 | } | 637 | } | ||
| 650 | 638 | ||||
| 651 | // selectRowByURL finds and selects a row whose URL matches. | 639 | // selectRowByURL finds and selects a row whose URL matches. | ||
| @@ -686,19 +674,36 @@ func (hs *HistorySidebar) setupSearchHandler() { | |||||
| 686 | 674 | ||||
| 687 | func (hs *HistorySidebar) onSearchChanged() { | 675 | func (hs *HistorySidebar) onSearchChanged() { | ||
| 688 | hs.mu.Lock() | 676 | hs.mu.Lock() | ||
| 677 | + | if hs.destroyed { | |||
| 678 | + | hs.mu.Unlock() | |||
| 679 | + | return | |||
| 680 | + | } | |||
| 689 | hs.currentQuery = hs.searchEntry.GetText() | 681 | hs.currentQuery = hs.searchEntry.GetText() | ||
| 690 | hs.preserveScrollAndSelection() | 682 | hs.preserveScrollAndSelection() | ||
| 683 | + | oldTimer := hs.debounceTimer | |||
| 684 | + | hs.debounceTimer = 0 | |||
| 691 | hs.mu.Unlock() | 685 | hs.mu.Unlock() | ||
| 692 | 686 | ||||
| 693 | - | // Debounce filtering/search | 687 | + | if oldTimer != 0 { |
| 694 | - | if hs.debounceTimer != 0 { | 688 | + | glib.SourceRemove(oldTimer) |
| 695 | - | glib.SourceRemove(hs.debounceTimer) | |||
| 696 | } | 689 | } | ||
| 690 | + | ||||
| 697 | filterCb := glib.SourceFunc(func(uintptr) bool { | 691 | filterCb := glib.SourceFunc(func(uintptr) bool { | ||
| 698 | hs.applyFilter() | 692 | hs.applyFilter() | ||
| 699 | return false | 693 | return false | ||
| 700 | }) | 694 | }) | ||
| 701 | - | hs.debounceTimer = glib.TimeoutAdd(uint(sidebarSearchDebounceMs), &filterCb, 0) | 695 | + | timerID := glib.TimeoutAdd(uint(sidebarSearchDebounceMs), &filterCb, 0) |
| 696 | + | ||||
| 697 | + | hs.mu.Lock() | |||
| 698 | + | if hs.destroyed { | |||
| 699 | + | hs.mu.Unlock() | |||
| 700 | + | if timerID != 0 { | |||
| 701 | + | glib.SourceRemove(timerID) | |||
| 702 | + | } | |||
| 703 | + | return | |||
| 704 | + | } | |||
| 705 | + | hs.debounceTimer = timerID | |||
| 706 | + | hs.mu.Unlock() | |||
| 702 | } | 707 | } | ||
| 703 | 708 | ||||
| 704 | func (hs *HistorySidebar) applyFilter() { | 709 | func (hs *HistorySidebar) applyFilter() { | ||
| @@ -1176,58 +1181,49 @@ func (hs *HistorySidebar) handleDeleteKey() bool { | |||||
| 1176 | if row == nil || !row.GetSelectable() { | 1181 | if row == nil || !row.GetSelectable() { | ||
| 1177 | return false | 1182 | return false | ||
| 1178 | } | 1183 | } | ||
| 1184 | + | if hs.historyUC == nil { | |||
| 1185 | + | return false | |||
| 1186 | + | } | |||
| 1179 | 1187 | ||||
| 1180 | idx := row.GetIndex() | 1188 | idx := row.GetIndex() | ||
| 1181 | - | // Find the entry URL and ID from the in-memory groups under one lock. | 1189 | + | |
| 1182 | hs.mu.RLock() | 1190 | hs.mu.RLock() | ||
| 1183 | url := hs.entryURLAtIndex(idx) | 1191 | url := hs.entryURLAtIndex(idx) | ||
| 1184 | entryID := hs.findEntryIDByIndex(idx) | 1192 | entryID := hs.findEntryIDByIndex(idx) | ||
| 1193 | + | nextSelectedURL := "" | |||
| 1194 | + | if nextRow := hs.findNextSelectableAfter(idx); nextRow != -1 { | |||
| 1195 | + | nextSelectedURL = hs.entryURLAtIndex(nextRow) | |||
| 1196 | + | } | |||
| 1185 | hs.mu.RUnlock() | 1197 | hs.mu.RUnlock() | ||
| 1186 | 1198 | ||||
| 1187 | if url == "" || entryID <= 0 { | 1199 | if url == "" || entryID <= 0 { | ||
| 1188 | return false | 1200 | return false | ||
| 1189 | } | 1201 | } | ||
| 1190 | 1202 | ||||
| 1191 | - | // Find the next row to select before deletion | 1203 | + | go func() { |
| 1192 | - | nextRow := hs.findNextSelectableAfter(idx) | |||
| 1193 | - | ||||
| 1194 | - | // Delete via the search history use case | |||
| 1195 | - | cb := glib.SourceFunc(func(uintptr) bool { | |||
| 1196 | - | if hs.historyUC == nil { | |||
| 1197 | - | return false | |||
| 1198 | - | } | |||
| 1199 | if err := hs.historyUC.Delete(hs.ctx, entryID); err != nil { | 1204 | if err := hs.historyUC.Delete(hs.ctx, entryID); err != nil { | ||
| 1200 | hs.logger.Error().Err(err).Int64("entry_id", entryID).Msg("failed to delete history entry") | 1205 | hs.logger.Error().Err(err).Int64("entry_id", entryID).Msg("failed to delete history entry") | ||
| 1201 | - | return false | 1206 | + | return |
| 1202 | } | 1207 | } | ||
| 1203 | - | return false | |||
| 1204 | - | }) | |||
| 1205 | - | glib.IdleAdd(&cb, 0) | |||
| 1206 | - | ||||
| 1207 | - | // Remove the entry from local data and rebuild the list. | |||
| 1208 | - | // Must also remove from allEntries and searchResults so the | |||
| 1209 | - | // deleted entry does not reappear after rebuildLocalGroups | |||
| 1210 | - | // (which re-groups from allEntries). | |||
| 1211 | - | hs.mu.Lock() | |||
| 1212 | - | hs.removeEntryByIndex(idx) | |||
| 1213 | - | hs.removeFromAllEntries(url, entryID) | |||
| 1214 | - | hs.removeFromSearchResults(entryID) | |||
| 1215 | - | hs.rebuildLocalGroups() | |||
| 1216 | - | hs.mu.Unlock() | |||
| 1217 | 1208 | ||||
| 1218 | - | hs.scheduleRebuild() | 1209 | + | cb := glib.SourceFunc(func(uintptr) bool { |
| 1219 | - | 1210 | + | hs.mu.Lock() | |
| 1220 | - | // After rebuild, select the next row | 1211 | + | if hs.destroyed { |
| 1221 | - | // (scheduled after rebuild to ensure rows exist) | 1212 | + | hs.mu.Unlock() |
| 1222 | - | if nextRow != -1 { | 1213 | + | return false |
| 1223 | - | selectCb := glib.SourceFunc(func(uintptr) bool { | |||
| 1224 | - | if target := hs.listBox.GetRowAtIndex(nextRow); target != nil { | |||
| 1225 | - | hs.listBox.SelectRow(target) | |||
| 1226 | } | 1214 | } | ||
| 1215 | + | hs.preserveScrollAndSelection() | |||
| 1216 | + | hs.prevSelectedURL = nextSelectedURL | |||
| 1217 | + | hs.removeFromAllEntries(url, entryID) | |||
| 1218 | + | hs.removeFromSearchResults(entryID) | |||
| 1219 | + | hs.rebuildLocalGroups() | |||
| 1220 | + | hs.mu.Unlock() | |||
| 1221 | + | ||||
| 1222 | + | hs.rebuildList() | |||
| 1227 | return false | 1223 | return false | ||
| 1228 | }) | 1224 | }) | ||
| 1229 | - | glib.IdleAdd(&selectCb, 0) | 1225 | + | glib.IdleAdd(&cb, 0) |
| 1230 | - | } | 1226 | + | }() |
| 1231 | 1227 | ||||
| 1232 | return true | 1228 | return true | ||
| 1233 | } | 1229 | } | ||
| @@ -1235,37 +1231,11 @@ func (hs *HistorySidebar) handleDeleteKey() bool { | |||||
| 1235 | // findEntryIDByIndex returns the entry ID for the linear ListBox index. | 1231 | // findEntryIDByIndex returns the entry ID for the linear ListBox index. | ||
| 1236 | // Must be called with hs.mu read lock held. | 1232 | // Must be called with hs.mu read lock held. | ||
| 1237 | func (hs *HistorySidebar) findEntryIDByIndex(index int) int64 { | 1233 | func (hs *HistorySidebar) findEntryIDByIndex(index int) int64 { | ||
| 1238 | - | linearEntryIdx := 0 | 1234 | + | entry := newKeyboardNavModel(hs.groups).entryAt(index) |
| 1239 | - | for _, group := range hs.groups { | 1235 | + | if entry == nil { |
| 1240 | - | if index == linearEntryIdx { | 1236 | + | return 0 |
| 1241 | - | return 0 // header row | |||
| 1242 | - | } | |||
| 1243 | - | linearEntryIdx++ | |||
| 1244 | - | if index < linearEntryIdx+len(group.Entries) { | |||
| 1245 | - | return group.Entries[index-linearEntryIdx].ID | |||
| 1246 | - | } | |||
| 1247 | - | linearEntryIdx += len(group.Entries) | |||
| 1248 | - | } | |||
| 1249 | - | return 0 | |||
| 1250 | - | } | |||
| 1251 | - | ||||
| 1252 | - | // removeEntryByIndex removes an entry from the groups slice by linear index. | |||
| 1253 | - | // Must be called with hs.mu write lock held. | |||
| 1254 | - | func (hs *HistorySidebar) removeEntryByIndex(index int) { | |||
| 1255 | - | linearEntryIdx := 0 | |||
| 1256 | - | for gi, group := range hs.groups { | |||
| 1257 | - | linearEntryIdx++ // skip header | |||
| 1258 | - | if index >= linearEntryIdx && index < linearEntryIdx+len(group.Entries) { | |||
| 1259 | - | entryIdx := index - linearEntryIdx | |||
| 1260 | - | hs.groups[gi].Entries = append(group.Entries[:entryIdx], group.Entries[entryIdx+1:]...) | |||
| 1261 | - | // Remove empty groups | |||
| 1262 | - | if len(hs.groups[gi].Entries) == 0 { | |||
| 1263 | - | hs.groups = append(hs.groups[:gi], hs.groups[gi+1:]...) | |||
| 1264 | - | } | |||
| 1265 | - | return | |||
| 1266 | - | } | |||
| 1267 | - | linearEntryIdx += len(group.Entries) | |||
| 1268 | } | 1237 | } | ||
| 1238 | + | return entry.ID | |||
| 1269 | } | 1239 | } | ||
| 1270 | 1240 | ||||
| 1271 | // rebuildLocalGroups rebuilds hs.groups from the current allEntries and query. | 1241 | // rebuildLocalGroups rebuilds hs.groups from the current allEntries and query. | ||
| @@ -1313,28 +1283,17 @@ func (hs *HistorySidebar) removeFromSearchResults(id int64) { | |||||
| 1313 | } | 1283 | } | ||
| 1314 | 1284 | ||||
| 1315 | // findNextSelectableAfter returns the ListBox index of the next selectable | 1285 | // findNextSelectableAfter returns the ListBox index of the next selectable | ||
| 1316 | - | // row after the given index, preferring the same position then previous. | 1286 | + | // row after the given index, falling back to the previous selectable row. |
| 1287 | + | // Must be called with hs.mu read lock held. | |||
| 1317 | func (hs *HistorySidebar) findNextSelectableAfter(idx int) int { | 1288 | func (hs *HistorySidebar) findNextSelectableAfter(idx int) int { | ||
| 1318 | - | hs.mu.RLock() | 1289 | + | model := newKeyboardNavModel(hs.groups) |
| 1319 | - | defer hs.mu.RUnlock() | 1290 | + | if next := model.nextSelectableIndex(idx, +1); next != -1 { |
| 1320 | - | 1291 | + | return next | |
| 1321 | - | total := 0 | |||
| 1322 | - | for _, group := range hs.groups { | |||
| 1323 | - | total++ // header | |||
| 1324 | - | total += len(group.Entries) | |||
| 1325 | - | } | |||
| 1326 | - | ||||
| 1327 | - | // Try same position first | |||
| 1328 | - | candidate := idx | |||
| 1329 | - | // Adjust for next rebuild (lose 1 entry, possibly a group header) | |||
| 1330 | - | // Worst case: we just pick (idx) if within range | |||
| 1331 | - | if candidate >= total-1 { | |||
| 1332 | - | candidate = total - 2 | |||
| 1333 | } | 1292 | } | ||
| 1334 | - | if candidate < 0 { | 1293 | + | if prev := model.nextSelectableIndex(idx, -1); prev != -1 { |
| 1335 | - | candidate = 0 | 1294 | + | return prev |
| 1336 | } | 1295 | } | ||
| 1337 | - | return candidate | 1296 | + | return -1 |
| 1338 | } | 1297 | } | ||
| 1339 | 1298 | ||||
| 1340 | // scrollByPage scrolls the list by one page up or down, | 1299 | // scrollByPage scrolls the list by one page up or down, | ||
| @@ -1370,36 +1329,19 @@ func (hs *HistorySidebar) jumpToPreviousDay() { | |||||
| 1370 | currentIdx = row.GetIndex() | 1329 | currentIdx = row.GetIndex() | ||
| 1371 | } | 1330 | } | ||
| 1372 | 1331 | ||||
| 1373 | - | // Walk backwards through rows to find the previous group header, | 1332 | + | hs.mu.RLock() |
| 1374 | - | // then select the first entry after it. | 1333 | + | targetIdx := newKeyboardNavModel(hs.groups).previousDayBoundary(currentIdx) |
| 1375 | - | prevHeaderIdx := -1 | 1334 | + | hs.mu.RUnlock() |
| 1376 | - | for i := currentIdx - 1; i >= 0; i-- { | 1335 | + | if targetIdx == -1 { |
| 1377 | - | row := hs.listBox.GetRowAtIndex(i) | |||
| 1378 | - | if row == nil { | |||
| 1379 | - | break | |||
| 1380 | - | } | |||
| 1381 | - | if !row.GetSelectable() { | |||
| 1382 | - | prevHeaderIdx = i | |||
| 1383 | - | break | |||
| 1384 | - | } | |||
| 1385 | - | } | |||
| 1386 | - | ||||
| 1387 | - | if prevHeaderIdx == -1 { | |||
| 1388 | - | // No previous group; try the very first row (header or entry) | |||
| 1389 | hs.jumpToFirstSelectable() | 1336 | hs.jumpToFirstSelectable() | ||
| 1390 | return | 1337 | return | ||
| 1391 | } | 1338 | } | ||
| 1392 | - | 1339 | + | if row := hs.listBox.GetRowAtIndex(targetIdx); row != nil && row.GetSelectable() { | |
| 1393 | - | // Select the first entry after the previous header | |||
| 1394 | - | firstEntryIdx := prevHeaderIdx + 1 | |||
| 1395 | - | if row := hs.listBox.GetRowAtIndex(firstEntryIdx); row != nil && row.GetSelectable() { | |||
| 1396 | hs.listBox.SelectRow(row) | 1340 | hs.listBox.SelectRow(row) | ||
| 1397 | - | // Ensure the selected row is scrolled into view | 1341 | + | hs.scrollRowIntoView(targetIdx) |
| 1398 | - | hs.scrollRowIntoView(firstEntryIdx) | 1342 | + | return |
| 1399 | - | } else { | |||
| 1400 | - | // No entry in this group | |||
| 1401 | - | hs.jumpToFirstSelectable() | |||
| 1402 | } | 1343 | } | ||
| 1344 | + | hs.jumpToFirstSelectable() | |||
| 1403 | } | 1345 | } | ||
| 1404 | 1346 | ||||
| 1405 | // jumpToNextDay selects the first entry in the next day group | 1347 | // jumpToNextDay selects the first entry in the next day group | ||
| @@ -1410,43 +1352,19 @@ func (hs *HistorySidebar) jumpToNextDay() { | |||||
| 1410 | currentIdx = row.GetIndex() | 1352 | currentIdx = row.GetIndex() | ||
| 1411 | } | 1353 | } | ||
| 1412 | 1354 | ||||
| 1413 | - | totalRows := 0 | 1355 | + | hs.mu.RLock() |
| 1414 | - | for { | 1356 | + | targetIdx := newKeyboardNavModel(hs.groups).nextDayBoundary(currentIdx) |
| 1415 | - | if hs.listBox.GetRowAtIndex(totalRows) == nil { | 1357 | + | hs.mu.RUnlock() |
| 1416 | - | break | 1358 | + | if targetIdx == -1 { |
| 1417 | - | } | |||
| 1418 | - | totalRows++ | |||
| 1419 | - | } | |||
| 1420 | - | ||||
| 1421 | - | // Walk forwards through rows to find the next group header, | |||
| 1422 | - | // then select the first entry after it. | |||
| 1423 | - | nextHeaderIdx := -1 | |||
| 1424 | - | for i := currentIdx + 1; i < totalRows; i++ { | |||
| 1425 | - | row := hs.listBox.GetRowAtIndex(i) | |||
| 1426 | - | if row == nil { | |||
| 1427 | - | break | |||
| 1428 | - | } | |||
| 1429 | - | if !row.GetSelectable() { | |||
| 1430 | - | nextHeaderIdx = i | |||
| 1431 | - | break | |||
| 1432 | - | } | |||
| 1433 | - | } | |||
| 1434 | - | ||||
| 1435 | - | if nextHeaderIdx == -1 { | |||
| 1436 | - | // No next group; jump to last selectable entry | |||
| 1437 | hs.jumpToLastSelectable() | 1359 | hs.jumpToLastSelectable() | ||
| 1438 | return | 1360 | return | ||
| 1439 | } | 1361 | } | ||
| 1440 | - | 1362 | + | if row := hs.listBox.GetRowAtIndex(targetIdx); row != nil && row.GetSelectable() { | |
| 1441 | - | // Select the first entry after the next header | |||
| 1442 | - | firstEntryIdx := nextHeaderIdx + 1 | |||
| 1443 | - | if row := hs.listBox.GetRowAtIndex(firstEntryIdx); row != nil && row.GetSelectable() { | |||
| 1444 | hs.listBox.SelectRow(row) | 1363 | hs.listBox.SelectRow(row) | ||
| 1445 | - | hs.scrollRowIntoView(firstEntryIdx) | 1364 | + | hs.scrollRowIntoView(targetIdx) |
| 1446 | - | } else { | 1365 | + | return |
| 1447 | - | // Empty group header? | |||
| 1448 | - | hs.jumpToLastSelectable() | |||
| 1449 | } | 1366 | } | ||
| 1367 | + | hs.jumpToLastSelectable() | |||
| 1450 | } | 1368 | } | ||
| 1451 | 1369 | ||||
| 1452 | // scrollRowIntoView scrolls the scrolled window to ensure the row at | 1370 | // scrollRowIntoView scrolls the scrolled window to ensure the row at | ||
| @@ -1540,44 +1458,32 @@ func (hs *HistorySidebar) selectAdjacentRow(direction int) { | |||||
| 1540 | return | 1458 | return | ||
| 1541 | } | 1459 | } | ||
| 1542 | 1460 | ||||
| 1543 | - | totalRows := 0 | |||
| 1544 | - | for { | |||
| 1545 | - | if hs.listBox.GetRowAtIndex(totalRows) == nil { | |||
| 1546 | - | break | |||
| 1547 | - | } | |||
| 1548 | - | totalRows++ | |||
| 1549 | - | } | |||
| 1550 | - | if totalRows == 0 { | |||
| 1551 | - | return | |||
| 1552 | - | } | |||
| 1553 | - | ||||
| 1554 | current := -1 | 1461 | current := -1 | ||
| 1555 | if row := hs.listBox.GetSelectedRow(); row != nil { | 1462 | if row := hs.listBox.GetSelectedRow(); row != nil { | ||
| 1556 | current = row.GetIndex() | 1463 | current = row.GetIndex() | ||
| 1557 | } | 1464 | } | ||
| 1558 | 1465 | ||||
| 1559 | - | // Nothing selected yet — pick first/last depending on direction. | 1466 | + | hs.mu.RLock() |
| 1467 | + | model := newKeyboardNavModel(hs.groups) | |||
| 1468 | + | target := -1 | |||
| 1560 | if current < 0 { | 1469 | if current < 0 { | ||
| 1561 | if direction > 0 { | 1470 | if direction > 0 { | ||
| 1562 | - | hs.jumpToFirstSelectable() | 1471 | + | target = model.firstSelectableIndex() |
| 1563 | } else { | 1472 | } else { | ||
| 1564 | - | hs.jumpToLastSelectable() | 1473 | + | target = model.lastSelectableIndex() |
| 1565 | } | 1474 | } | ||
| 1566 | - | return | 1475 | + | } else { |
| 1476 | + | target = model.nextSelectableIndex(current, direction) | |||
| 1567 | } | 1477 | } | ||
| 1478 | + | hs.mu.RUnlock() | |||
| 1568 | 1479 | ||||
| 1569 | - | // Walk in the given direction to find the next selectable row. | 1480 | + | if target == -1 { |
| 1570 | - | candidate := current + direction | 1481 | + | return |
| 1571 | - | for candidate >= 0 && candidate < totalRows { | 1482 | + | } |
| 1572 | - | row := hs.listBox.GetRowAtIndex(candidate) | 1483 | + | if row := hs.listBox.GetRowAtIndex(target); row != nil && row.GetSelectable() { |
| 1573 | - | if row != nil && row.GetSelectable() { | 1484 | + | hs.listBox.SelectRow(row) |
| 1574 | - | hs.listBox.SelectRow(row) | 1485 | + | hs.ensureRowVisible(target) |
| 1575 | - | hs.ensureRowVisible(candidate) | |||
| 1576 | - | return | |||
| 1577 | - | } | |||
| 1578 | - | candidate += direction | |||
| 1579 | } | 1486 | } | ||
| 1580 | - | // No more selectable rows in this direction; selection unchanged. | |||
| 1581 | } | 1487 | } | ||
| 1582 | 1488 | ||||
| 1583 | // ensureRowVisible adjusts the scrolled window so the row at index is | 1489 | // ensureRowVisible adjusts the scrolled window so the row at index is | ||
| @@ -1645,31 +1551,13 @@ func (hs *HistorySidebar) onRowActivated(row *gtk.ListBoxRow) { | |||||
| 1645 | hs.mu.RUnlock() | 1551 | hs.mu.RUnlock() | ||
| 1646 | return | 1552 | return | ||
| 1647 | } | 1553 | } | ||
| 1648 | - | 1554 | + | entry := newKeyboardNavModel(hs.groups).entryAt(row.GetIndex()) | |
| 1649 | - | index := row.GetIndex() | |||
| 1650 | - | ||||
| 1651 | - | // Compute entry index across all groups, excluding header rows. | |||
| 1652 | - | // ListBox indices: for each group, 1 header row + N entry rows. | |||
| 1653 | - | linearEntryIdx := 0 | |||
| 1654 | - | for _, group := range hs.groups { | |||
| 1655 | - | // Check if index falls on the header row for this group | |||
| 1656 | - | if index == linearEntryIdx { | |||
| 1657 | - | // Header row - not an activatable entry | |||
| 1658 | - | hs.mu.RUnlock() | |||
| 1659 | - | return | |||
| 1660 | - | } | |||
| 1661 | - | linearEntryIdx++ // Skip header | |||
| 1662 | - | ||||
| 1663 | - | if index < linearEntryIdx+len(group.Entries) { | |||
| 1664 | - | entry := group.Entries[index-linearEntryIdx] | |||
| 1665 | - | url := entry.URL | |||
| 1666 | - | hs.mu.RUnlock() | |||
| 1667 | - | hs.navigateToURL(url) | |||
| 1668 | - | return | |||
| 1669 | - | } | |||
| 1670 | - | linearEntryIdx += len(group.Entries) | |||
| 1671 | - | } | |||
| 1672 | hs.mu.RUnlock() | 1555 | hs.mu.RUnlock() | ||
| 1556 | + | if entry == nil || entry.URL == "" { | |||
| 1557 | + | return | |||
| 1558 | + | } | |||
| 1559 | + | ||||
| 1560 | + | hs.navigateToURL(entry.URL) | |||
| 1673 | } | 1561 | } | ||
| 1674 | 1562 | ||||
| 1675 | func (hs *HistorySidebar) navigateToURL(url string) { | 1563 | func (hs *HistorySidebar) navigateToURL(url string) { | ||
| @@ -1687,7 +1575,7 @@ func (hs *HistorySidebar) navigateToURL(url string) { | |||||
| 1687 | // navigateWithoutClosing navigates to the URL but does NOT close the sidebar. | 1575 | // navigateWithoutClosing navigates to the URL but does NOT close the sidebar. | ||
| 1688 | // Used by Ctrl+Enter activation. | 1576 | // Used by Ctrl+Enter activation. | ||
| 1689 | func (hs *HistorySidebar) navigateWithoutClosing(url string) { | 1577 | func (hs *HistorySidebar) navigateWithoutClosing(url string) { | ||
| 1690 | - | if hs.onURL == nil || url == "" { | 1578 | + | if hs.onNavigateKeepOpen == nil || url == "" { |
| 1691 | return | 1579 | return | ||
| 1692 | } | 1580 | } | ||
| 1693 | 1581 | ||||
M internal/ui/component/history_sidebar_search_test.go +25 -65
| @@ -190,10 +190,11 @@ func TestDoFTSearch_WithFakeUC_StaleGenerationDropsResults(t *testing.T) { | |||
| 190 | 190 | } | |
| 191 | 191 | ||
| 192 | 192 | func TestDoFTSearch_WithFakeUC_CurrentGenApplied(t *testing.T) { | |
| 193 | - | searchDone := make(chan struct{}, 1) | |
| 193 | + | searchCalled := make(chan struct{}, 1) | |
| 194 | 194 | ||
| 195 | 195 | repo := &fakeHistoryRepo{ | |
| 196 | 196 | searchFn: func(_ context.Context, query string, _ int) ([]entity.HistoryMatch, error) { | |
| 197 | + | searchCalled <- struct{}{} | |
| 197 | 198 | return []entity.HistoryMatch{ | |
| 198 | 199 | {Entry: &entity.HistoryEntry{ID: 1, URL: "https://live.com", Title: "Live", LastVisited: time.Now()}}, | |
| 199 | 200 | }, nil | |
| @@ -206,41 +207,26 @@ func TestDoFTSearch_WithFakeUC_CurrentGenApplied(t *testing.T) { | |||
| 206 | 207 | hs.historyUC = fakeUC | |
| 207 | 208 | hs.searchGen = 1 | |
| 208 | 209 | ||
| 209 | - | // Spin up a goroutine that polls for results to be applied. | |
| 210 | - | go func() { | |
| 211 | - | for { | |
| 212 | - | hs.mu.RLock() | |
| 213 | - | if hs.searchDone { | |
| 214 | - | hs.mu.RUnlock() | |
| 215 | - | searchDone <- struct{}{} | |
| 216 | - | return | |
| 217 | - | } | |
| 218 | - | hs.mu.RUnlock() | |
| 219 | - | time.Sleep(5 * time.Millisecond) | |
| 220 | - | } | |
| 221 | - | }() | |
| 222 | - | ||
| 223 | - | // Start the search; goroutine fetches and tries to idle-apply. | |
| 210 | + | // Start the search and wait for the use case to be invoked. | |
| 224 | 211 | hs.doFTSearch("live", 1) | |
| 212 | + | select { | |
| 213 | + | case <-searchCalled: | |
| 214 | + | case <-time.After(time.Second): | |
| 215 | + | t.Fatal("timed out waiting for search use case to be invoked") | |
| 216 | + | } | |
| 225 | 217 | ||
| 226 | - | // glib.IdleAdd is a no-op without GTK, so the idle callback never runs. | |
| 227 | - | // We simulate it by calling applySearchResults ourselves, as the | |
| 228 | - | // production idle callback would. | |
| 229 | - | hs.applySearchResults([]*entity.HistoryEntry{ | |
| 218 | + | // glib.IdleAdd is a no-op without GTK, so apply the callback effect directly. | |
| 219 | + | applied := hs.applySearchResults([]*entity.HistoryEntry{ | |
| 230 | 220 | {ID: 1, URL: "https://live.com", Title: "Live", LastVisited: time.Now()}, | |
| 231 | 221 | }, 1, nil) | |
| 222 | + | require.True(t, applied) | |
| 232 | 223 | ||
| 233 | - | select { | |
| 234 | - | case <-searchDone: | |
| 235 | - | hs.mu.RLock() | |
| 236 | - | assert.NotNil(t, hs.searchResults) | |
| 237 | - | assert.True(t, hs.searchDone) | |
| 238 | - | assert.Len(t, hs.searchResults, 1) | |
| 239 | - | assert.Equal(t, "https://live.com", hs.searchResults[0].URL) | |
| 240 | - | hs.mu.RUnlock() | |
| 241 | - | case <-time.After(time.Second): | |
| 242 | - | t.Fatal("timed out waiting for search results to be applied") | |
| 243 | - | } | |
| 224 | + | hs.mu.RLock() | |
| 225 | + | assert.NotNil(t, hs.searchResults) | |
| 226 | + | assert.True(t, hs.searchDone) | |
| 227 | + | assert.Len(t, hs.searchResults, 1) | |
| 228 | + | assert.Equal(t, "https://live.com", hs.searchResults[0].URL) | |
| 229 | + | hs.mu.RUnlock() | |
| 244 | 230 | } | |
| 245 | 231 | ||
| 246 | 232 | // ============================================================================= | |
| @@ -248,29 +234,13 @@ func TestDoFTSearch_WithFakeUC_CurrentGenApplied(t *testing.T) { | |||
| 248 | 234 | // ============================================================================= | |
| 249 | 235 | ||
| 250 | 236 | // TestHistorySidebar_ReloadPreservesQuery verifies that Reload preserves the | |
| 251 | - | // search query and resets internal state without losing the query string. | |
| 252 | - | // This is a seam test that uses applyReloadState and the Reload method's | |
| 253 | - | // state transitions without GTK widgets. | |
| 237 | + | // active query while resetting browse/search state before the refreshed load. | |
| 254 | 238 | func TestHistorySidebar_ReloadPreservesQuery(t *testing.T) { | |
| 255 | - | // Use the pure-model reload state function to confirm the expected | |
| 256 | - | // transition when a query is active. | |
| 257 | - | withQuery := applyReloadState("search-term") | |
| 258 | - | assert.Equal(t, "search-term", withQuery.PreservedQuery) | |
| 259 | - | assert.False(t, withQuery.ResetBrowse) | |
| 260 | - | assert.True(t, withQuery.ClearSearch) | |
| 261 | - | ||
| 262 | - | withoutQuery := applyReloadState("") | |
| 263 | - | assert.Equal(t, "", withoutQuery.PreservedQuery) | |
| 264 | - | assert.True(t, withoutQuery.ResetBrowse) | |
| 265 | - | assert.False(t, withoutQuery.ClearSearch) | |
| 266 | - | ||
| 267 | - | // Now test the actual Reload method seam on a minimal HistorySidebar. | |
| 268 | 239 | hs := newTestSidebarSearchHarness() | |
| 269 | 240 | hs.currentQuery = "preserved" | |
| 270 | 241 | hs.historyUC = usecase.NewSearchHistoryUseCase(&fakeHistoryRepo{}) | |
| 271 | 242 | hs.ctx = context.Background() | |
| 272 | 243 | ||
| 273 | - | // Simulate the parts of Reload that don't require GTK widgets. | |
| 274 | 244 | hs.mu.Lock() | |
| 275 | 245 | oldGen := hs.searchGen | |
| 276 | 246 | hs.loadDone = true | |
| @@ -278,32 +248,22 @@ func TestHistorySidebar_ReloadPreservesQuery(t *testing.T) { | |||
| 278 | 248 | {ID: 1, URL: "https://old.com", Title: "Old", LastVisited: time.Now()}, | |
| 279 | 249 | } | |
| 280 | 250 | hs.groups = groupHistoryByDay(hs.allEntries) | |
| 251 | + | hs.searchResults = []*entity.HistoryEntry{{ID: 2, URL: "https://stale.com", Title: "Stale", LastVisited: time.Now()}} | |
| 252 | + | hs.searchDone = true | |
| 281 | 253 | hs.mu.Unlock() | |
| 282 | 254 | ||
| 283 | - | // Call Reload (skipping the startLoadHistory which needs GTK). | |
| 284 | - | // Reload resets state and preserves query. | |
| 285 | - | savedQuery := hs.currentQuery // "preserved" | |
| 286 | - | hs.preserveScrollAndSelection() | |
| 287 | - | hs.loadDone = false | |
| 288 | - | hs.loadStarted = false | |
| 289 | - | hs.totalLoaded = 0 | |
| 290 | - | hs.hasMore = hs.historyUC != nil | |
| 291 | - | hs.isLoading = false | |
| 292 | - | hs.allEntries = nil | |
| 293 | - | hs.groups = nil | |
| 294 | - | hs.searchResults = nil | |
| 295 | - | hs.searchDone = false | |
| 296 | - | hs.searchErr = nil | |
| 297 | - | hs.currentQuery = savedQuery | |
| 298 | - | hs.searchGen++ | |
| 255 | + | hs.Reload() | |
| 299 | 256 | ||
| 257 | + | hs.mu.RLock() | |
| 300 | 258 | assert.Equal(t, "preserved", hs.currentQuery, "query must be preserved after Reload") | |
| 301 | 259 | assert.False(t, hs.loadDone, "loadDone must be reset") | |
| 260 | + | assert.False(t, hs.loadStarted, "loadStarted must be reset") | |
| 302 | 261 | assert.Nil(t, hs.allEntries, "entries must be cleared") | |
| 303 | 262 | assert.Nil(t, hs.groups, "groups must be cleared") | |
| 304 | - | assert.Nil(t, hs.searchResults, "searchResults must be cleared") | |
| 263 | + | assert.Nil(t, hs.searchResults, "searchResults must be cleared before refreshed search applies") | |
| 305 | 264 | assert.False(t, hs.searchDone, "searchDone must be reset") | |
| 306 | 265 | assert.Equal(t, oldGen+1, hs.searchGen, "searchGen must be incremented") | |
| 266 | + | hs.mu.RUnlock() | |
| 307 | 267 | } | |
| 308 | 268 | ||
| 309 | 269 | // ============================================================================= | |
| @@ -190,10 +190,11 @@ func TestDoFTSearch_WithFakeUC_StaleGenerationDropsResults(t *testing.T) { | |||||
| 190 | } | 190 | } | ||
| 191 | 191 | ||||
| 192 | func TestDoFTSearch_WithFakeUC_CurrentGenApplied(t *testing.T) { | 192 | func TestDoFTSearch_WithFakeUC_CurrentGenApplied(t *testing.T) { | ||
| 193 | - | searchDone := make(chan struct{}, 1) | 193 | + | searchCalled := make(chan struct{}, 1) |
| 194 | 194 | ||||
| 195 | repo := &fakeHistoryRepo{ | 195 | repo := &fakeHistoryRepo{ | ||
| 196 | searchFn: func(_ context.Context, query string, _ int) ([]entity.HistoryMatch, error) { | 196 | searchFn: func(_ context.Context, query string, _ int) ([]entity.HistoryMatch, error) { | ||
| 197 | + | searchCalled <- struct{}{} | |||
| 197 | return []entity.HistoryMatch{ | 198 | return []entity.HistoryMatch{ | ||
| 198 | {Entry: &entity.HistoryEntry{ID: 1, URL: "https://live.com", Title: "Live", LastVisited: time.Now()}}, | 199 | {Entry: &entity.HistoryEntry{ID: 1, URL: "https://live.com", Title: "Live", LastVisited: time.Now()}}, | ||
| 199 | }, nil | 200 | }, nil | ||
| @@ -206,41 +207,26 @@ func TestDoFTSearch_WithFakeUC_CurrentGenApplied(t *testing.T) { | |||||
| 206 | hs.historyUC = fakeUC | 207 | hs.historyUC = fakeUC | ||
| 207 | hs.searchGen = 1 | 208 | hs.searchGen = 1 | ||
| 208 | 209 | ||||
| 209 | - | // Spin up a goroutine that polls for results to be applied. | 210 | + | // Start the search and wait for the use case to be invoked. |
| 210 | - | go func() { | |||
| 211 | - | for { | |||
| 212 | - | hs.mu.RLock() | |||
| 213 | - | if hs.searchDone { | |||
| 214 | - | hs.mu.RUnlock() | |||
| 215 | - | searchDone <- struct{}{} | |||
| 216 | - | return | |||
| 217 | - | } | |||
| 218 | - | hs.mu.RUnlock() | |||
| 219 | - | time.Sleep(5 * time.Millisecond) | |||
| 220 | - | } | |||
| 221 | - | }() | |||
| 222 | - | ||||
| 223 | - | // Start the search; goroutine fetches and tries to idle-apply. | |||
| 224 | hs.doFTSearch("live", 1) | 211 | hs.doFTSearch("live", 1) | ||
| 212 | + | select { | |||
| 213 | + | case <-searchCalled: | |||
| 214 | + | case <-time.After(time.Second): | |||
| 215 | + | t.Fatal("timed out waiting for search use case to be invoked") | |||
| 216 | + | } | |||
| 225 | 217 | ||||
| 226 | - | // glib.IdleAdd is a no-op without GTK, so the idle callback never runs. | 218 | + | // glib.IdleAdd is a no-op without GTK, so apply the callback effect directly. |
| 227 | - | // We simulate it by calling applySearchResults ourselves, as the | 219 | + | applied := hs.applySearchResults([]*entity.HistoryEntry{ |
| 228 | - | // production idle callback would. | |||
| 229 | - | hs.applySearchResults([]*entity.HistoryEntry{ | |||
| 230 | {ID: 1, URL: "https://live.com", Title: "Live", LastVisited: time.Now()}, | 220 | {ID: 1, URL: "https://live.com", Title: "Live", LastVisited: time.Now()}, | ||
| 231 | }, 1, nil) | 221 | }, 1, nil) | ||
| 222 | + | require.True(t, applied) | |||
| 232 | 223 | ||||
| 233 | - | select { | 224 | + | hs.mu.RLock() |
| 234 | - | case <-searchDone: | 225 | + | assert.NotNil(t, hs.searchResults) |
| 235 | - | hs.mu.RLock() | 226 | + | assert.True(t, hs.searchDone) |
| 236 | - | assert.NotNil(t, hs.searchResults) | 227 | + | assert.Len(t, hs.searchResults, 1) |
| 237 | - | assert.True(t, hs.searchDone) | 228 | + | assert.Equal(t, "https://live.com", hs.searchResults[0].URL) |
| 238 | - | assert.Len(t, hs.searchResults, 1) | 229 | + | hs.mu.RUnlock() |
| 239 | - | assert.Equal(t, "https://live.com", hs.searchResults[0].URL) | |||
| 240 | - | hs.mu.RUnlock() | |||
| 241 | - | case <-time.After(time.Second): | |||
| 242 | - | t.Fatal("timed out waiting for search results to be applied") | |||
| 243 | - | } | |||
| 244 | } | 230 | } | ||
| 245 | 231 | ||||
| 246 | // ============================================================================= | 232 | // ============================================================================= | ||
| @@ -248,29 +234,13 @@ func TestDoFTSearch_WithFakeUC_CurrentGenApplied(t *testing.T) { | |||||
| 248 | // ============================================================================= | 234 | // ============================================================================= | ||
| 249 | 235 | ||||
| 250 | // TestHistorySidebar_ReloadPreservesQuery verifies that Reload preserves the | 236 | // TestHistorySidebar_ReloadPreservesQuery verifies that Reload preserves the | ||
| 251 | - | // search query and resets internal state without losing the query string. | 237 | + | // active query while resetting browse/search state before the refreshed load. |
| 252 | - | // This is a seam test that uses applyReloadState and the Reload method's | |||
| 253 | - | // state transitions without GTK widgets. | |||
| 254 | func TestHistorySidebar_ReloadPreservesQuery(t *testing.T) { | 238 | func TestHistorySidebar_ReloadPreservesQuery(t *testing.T) { | ||
| 255 | - | // Use the pure-model reload state function to confirm the expected | |||
| 256 | - | // transition when a query is active. | |||
| 257 | - | withQuery := applyReloadState("search-term") | |||
| 258 | - | assert.Equal(t, "search-term", withQuery.PreservedQuery) | |||
| 259 | - | assert.False(t, withQuery.ResetBrowse) | |||
| 260 | - | assert.True(t, withQuery.ClearSearch) | |||
| 261 | - | ||||
| 262 | - | withoutQuery := applyReloadState("") | |||
| 263 | - | assert.Equal(t, "", withoutQuery.PreservedQuery) | |||
| 264 | - | assert.True(t, withoutQuery.ResetBrowse) | |||
| 265 | - | assert.False(t, withoutQuery.ClearSearch) | |||
| 266 | - | ||||
| 267 | - | // Now test the actual Reload method seam on a minimal HistorySidebar. | |||
| 268 | hs := newTestSidebarSearchHarness() | 239 | hs := newTestSidebarSearchHarness() | ||
| 269 | hs.currentQuery = "preserved" | 240 | hs.currentQuery = "preserved" | ||
| 270 | hs.historyUC = usecase.NewSearchHistoryUseCase(&fakeHistoryRepo{}) | 241 | hs.historyUC = usecase.NewSearchHistoryUseCase(&fakeHistoryRepo{}) | ||
| 271 | hs.ctx = context.Background() | 242 | hs.ctx = context.Background() | ||
| 272 | 243 | ||||
| 273 | - | // Simulate the parts of Reload that don't require GTK widgets. | |||
| 274 | hs.mu.Lock() | 244 | hs.mu.Lock() | ||
| 275 | oldGen := hs.searchGen | 245 | oldGen := hs.searchGen | ||
| 276 | hs.loadDone = true | 246 | hs.loadDone = true | ||
| @@ -278,32 +248,22 @@ func TestHistorySidebar_ReloadPreservesQuery(t *testing.T) { | |||||
| 278 | {ID: 1, URL: "https://old.com", Title: "Old", LastVisited: time.Now()}, | 248 | {ID: 1, URL: "https://old.com", Title: "Old", LastVisited: time.Now()}, | ||
| 279 | } | 249 | } | ||
| 280 | hs.groups = groupHistoryByDay(hs.allEntries) | 250 | hs.groups = groupHistoryByDay(hs.allEntries) | ||
| 251 | + | hs.searchResults = []*entity.HistoryEntry{{ID: 2, URL: "https://stale.com", Title: "Stale", LastVisited: time.Now()}} | |||
| 252 | + | hs.searchDone = true | |||
| 281 | hs.mu.Unlock() | 253 | hs.mu.Unlock() | ||
| 282 | 254 | ||||
| 283 | - | // Call Reload (skipping the startLoadHistory which needs GTK). | 255 | + | hs.Reload() |
| 284 | - | // Reload resets state and preserves query. | |||
| 285 | - | savedQuery := hs.currentQuery // "preserved" | |||
| 286 | - | hs.preserveScrollAndSelection() | |||
| 287 | - | hs.loadDone = false | |||
| 288 | - | hs.loadStarted = false | |||
| 289 | - | hs.totalLoaded = 0 | |||
| 290 | - | hs.hasMore = hs.historyUC != nil | |||
| 291 | - | hs.isLoading = false | |||
| 292 | - | hs.allEntries = nil | |||
| 293 | - | hs.groups = nil | |||
| 294 | - | hs.searchResults = nil | |||
| 295 | - | hs.searchDone = false | |||
| 296 | - | hs.searchErr = nil | |||
| 297 | - | hs.currentQuery = savedQuery | |||
| 298 | - | hs.searchGen++ | |||
| 299 | 256 | ||||
| 257 | + | hs.mu.RLock() | |||
| 300 | assert.Equal(t, "preserved", hs.currentQuery, "query must be preserved after Reload") | 258 | assert.Equal(t, "preserved", hs.currentQuery, "query must be preserved after Reload") | ||
| 301 | assert.False(t, hs.loadDone, "loadDone must be reset") | 259 | assert.False(t, hs.loadDone, "loadDone must be reset") | ||
| 260 | + | assert.False(t, hs.loadStarted, "loadStarted must be reset") | |||
| 302 | assert.Nil(t, hs.allEntries, "entries must be cleared") | 261 | assert.Nil(t, hs.allEntries, "entries must be cleared") | ||
| 303 | assert.Nil(t, hs.groups, "groups must be cleared") | 262 | assert.Nil(t, hs.groups, "groups must be cleared") | ||
| 304 | - | assert.Nil(t, hs.searchResults, "searchResults must be cleared") | 263 | + | assert.Nil(t, hs.searchResults, "searchResults must be cleared before refreshed search applies") |
| 305 | assert.False(t, hs.searchDone, "searchDone must be reset") | 264 | assert.False(t, hs.searchDone, "searchDone must be reset") | ||
| 306 | assert.Equal(t, oldGen+1, hs.searchGen, "searchGen must be incremented") | 265 | assert.Equal(t, oldGen+1, hs.searchGen, "searchGen must be incremented") | ||
| 266 | + | hs.mu.RUnlock() | |||
| 307 | } | 267 | } | ||
| 308 | 268 | ||||
| 309 | // ============================================================================= | 269 | // ============================================================================= | ||
A internal/ui/component/history_test_helpers_test.go +39 -0
| @@ -0,0 +1,39 @@ | |||
| 1 | + | package component | |
| 2 | + | ||
| 3 | + | // searchStateSnapshot captures the observable state of a history search | |
| 4 | + | // at a point in time. This is test-only data for pure transition checks. | |
| 5 | + | type searchStateSnapshot struct { | |
| 6 | + | Query string | |
| 7 | + | HasSearchDone bool | |
| 8 | + | HasResults bool | |
| 9 | + | ResultCount int | |
| 10 | + | } | |
| 11 | + | ||
| 12 | + | // transitionSearchState models a search state transition for tests without GTK. | |
| 13 | + | func transitionSearchState(_ searchStateSnapshot, newQuery string, resultCount int) searchStateSnapshot { | |
| 14 | + | return searchStateSnapshot{ | |
| 15 | + | Query: newQuery, | |
| 16 | + | HasSearchDone: newQuery != "", | |
| 17 | + | HasResults: newQuery != "" && resultCount > 0, | |
| 18 | + | ResultCount: resultCount, | |
| 19 | + | } | |
| 20 | + | } | |
| 21 | + | ||
| 22 | + | // reloadPreservationSnapshot captures the preserved state during a reload. | |
| 23 | + | type reloadPreservationSnapshot struct { | |
| 24 | + | PreservedQuery string | |
| 25 | + | ResetBrowse bool | |
| 26 | + | ClearSearch bool | |
| 27 | + | } | |
| 28 | + | ||
| 29 | + | // applyReloadState computes the expected state after a reload for test-only | |
| 30 | + | // transition checks. | |
| 31 | + | func applyReloadState(currentQuery string) reloadPreservationSnapshot { | |
| 32 | + | s := reloadPreservationSnapshot{PreservedQuery: currentQuery} | |
| 33 | + | if currentQuery == "" { | |
| 34 | + | s.ResetBrowse = true | |
| 35 | + | } else { | |
| 36 | + | s.ClearSearch = true | |
| 37 | + | } | |
| 38 | + | return s | |
| 39 | + | } | |
| @@ -0,0 +1,39 @@ | |||||
| 1 | + | package component | |||
| 2 | + | ||||
| 3 | + | // searchStateSnapshot captures the observable state of a history search | |||
| 4 | + | // at a point in time. This is test-only data for pure transition checks. | |||
| 5 | + | type searchStateSnapshot struct { | |||
| 6 | + | Query string | |||
| 7 | + | HasSearchDone bool | |||
| 8 | + | HasResults bool | |||
| 9 | + | ResultCount int | |||
| 10 | + | } | |||
| 11 | + | ||||
| 12 | + | // transitionSearchState models a search state transition for tests without GTK. | |||
| 13 | + | func transitionSearchState(_ searchStateSnapshot, newQuery string, resultCount int) searchStateSnapshot { | |||
| 14 | + | return searchStateSnapshot{ | |||
| 15 | + | Query: newQuery, | |||
| 16 | + | HasSearchDone: newQuery != "", | |||
| 17 | + | HasResults: newQuery != "" && resultCount > 0, | |||
| 18 | + | ResultCount: resultCount, | |||
| 19 | + | } | |||
| 20 | + | } | |||
| 21 | + | ||||
| 22 | + | // reloadPreservationSnapshot captures the preserved state during a reload. | |||
| 23 | + | type reloadPreservationSnapshot struct { | |||
| 24 | + | PreservedQuery string | |||
| 25 | + | ResetBrowse bool | |||
| 26 | + | ClearSearch bool | |||
| 27 | + | } | |||
| 28 | + | ||||
| 29 | + | // applyReloadState computes the expected state after a reload for test-only | |||
| 30 | + | // transition checks. | |||
| 31 | + | func applyReloadState(currentQuery string) reloadPreservationSnapshot { | |||
| 32 | + | s := reloadPreservationSnapshot{PreservedQuery: currentQuery} | |||
| 33 | + | if currentQuery == "" { | |||
| 34 | + | s.ResetBrowse = true | |||
| 35 | + | } else { | |||
| 36 | + | s.ClearSearch = true | |||
| 37 | + | } | |||
| 38 | + | return s | |||
| 39 | + | } | |||
M internal/ui/theme/history_sidebar_css.go +18 -24
| @@ -1,28 +1,22 @@ | |||
| 1 | 1 | package theme | |
| 2 | 2 | ||
| 3 | - | import ( | |
| 4 | - | "fmt" | |
| 5 | - | ) | |
| 6 | - | ||
| 7 | 3 | // generateHistorySidebarCSS creates GTK4 CSS for the history sidebar component. | |
| 8 | - | func generateHistorySidebarCSS(p Palette) string { | |
| 9 | - | accentAlpha := fmt.Sprintf("alpha(%s, 0.18)", p.Accent) | |
| 10 | - | ||
| 11 | - | return fmt.Sprintf(`/* ===== History Sidebar Styling ===== */ | |
| 4 | + | func generateHistorySidebarCSS(_ Palette) string { | |
| 5 | + | return `/* ===== History Sidebar Styling ===== */ | |
| 12 | 6 | ||
| 13 | 7 | .history-sidebar-outer { | |
| 14 | 8 | background-color: var(--surface); | |
| 15 | - | border-left: 1px solid var(--border); | |
| 9 | + | border-left: 0.0625em solid var(--border); | |
| 16 | 10 | } | |
| 17 | 11 | ||
| 18 | 12 | .history-sidebar-search-box { | |
| 19 | - | padding: 6px 8px; | |
| 20 | - | border-bottom: 1px solid var(--border); | |
| 13 | + | padding: 0.375em 0.5em; | |
| 14 | + | border-bottom: 0.0625em solid var(--border); | |
| 21 | 15 | background-color: var(--surface); | |
| 22 | 16 | } | |
| 23 | 17 | ||
| 24 | 18 | .history-sidebar-search { | |
| 25 | - | padding: 2px 6px; | |
| 19 | + | padding: 0.125em 0.375em; | |
| 26 | 20 | font-size: 0.85em; | |
| 27 | 21 | } | |
| 28 | 22 | ||
| @@ -31,35 +25,35 @@ func generateHistorySidebarCSS(p Palette) string { | |||
| 31 | 25 | } | |
| 32 | 26 | ||
| 33 | 27 | .history-sidebar-group-header { | |
| 34 | - | padding: 4px 10px; | |
| 35 | - | padding-top: 6px; | |
| 28 | + | padding: 0.25em 0.625em; | |
| 29 | + | padding-top: 0.375em; | |
| 36 | 30 | font-size: 0.75em; | |
| 37 | 31 | font-weight: 600; | |
| 38 | 32 | color: var(--muted); | |
| 39 | 33 | text-transform: uppercase; | |
| 40 | 34 | letter-spacing: 0.04em; | |
| 41 | 35 | background-color: var(--surface-variant); | |
| 42 | - | border-bottom: 1px solid var(--border); | |
| 36 | + | border-bottom: 0.0625em solid var(--border); | |
| 43 | 37 | } | |
| 44 | 38 | ||
| 45 | 39 | .history-sidebar-row { | |
| 46 | - | padding: 3px 10px; | |
| 40 | + | padding: 0.1875em 0.625em; | |
| 47 | 41 | min-height: 0; | |
| 48 | - | border-bottom: 1px solid alpha(var(--border), 0.4); | |
| 42 | + | border-bottom: 0.0625em solid alpha(var(--border), 0.4); | |
| 49 | 43 | background-color: var(--surface); | |
| 50 | 44 | transition: background-color 100ms ease; | |
| 51 | 45 | } | |
| 52 | 46 | ||
| 53 | 47 | .history-sidebar-row:hover { | |
| 54 | - | background-color: %s; | |
| 48 | + | background-color: alpha(var(--accent), 0.18); | |
| 55 | 49 | } | |
| 56 | 50 | ||
| 57 | 51 | .history-sidebar-row:selected { | |
| 58 | - | background-color: %s; | |
| 52 | + | background-color: alpha(var(--accent), 0.18); | |
| 59 | 53 | } | |
| 60 | 54 | ||
| 61 | 55 | .history-sidebar-row:focus { | |
| 62 | - | background-color: %s; | |
| 56 | + | background-color: alpha(var(--accent), 0.18); | |
| 63 | 57 | } | |
| 64 | 58 | ||
| 65 | 59 | .history-sidebar-row-title { | |
| @@ -76,21 +70,21 @@ func generateHistorySidebarCSS(p Palette) string { | |||
| 76 | 70 | .history-sidebar-row-time { | |
| 77 | 71 | font-size: 0.68em; | |
| 78 | 72 | color: var(--muted); | |
| 79 | - | padding-left: 8px; | |
| 73 | + | padding-left: 0.5em; | |
| 80 | 74 | opacity: 0.75; | |
| 81 | 75 | } | |
| 82 | 76 | ||
| 83 | 77 | .history-sidebar-empty { | |
| 84 | - | padding: 24px 12px; | |
| 78 | + | padding: 1.5em 0.75em; | |
| 85 | 79 | font-size: 0.82em; | |
| 86 | 80 | color: var(--muted); | |
| 87 | 81 | font-style: italic; | |
| 88 | 82 | } | |
| 89 | 83 | ||
| 90 | 84 | .history-sidebar-loading { | |
| 91 | - | padding: 24px 12px; | |
| 85 | + | padding: 1.5em 0.75em; | |
| 92 | 86 | font-size: 0.82em; | |
| 93 | 87 | color: var(--muted); | |
| 94 | 88 | } | |
| 95 | - | `, accentAlpha, accentAlpha, accentAlpha) | |
| 89 | + | ` | |
| 96 | 90 | } | |
| @@ -1,28 +1,22 @@ | |||||
| 1 | package theme | 1 | package theme | ||
| 2 | 2 | ||||
| 3 | - | import ( | |||
| 4 | - | "fmt" | |||
| 5 | - | ) | |||
| 6 | - | ||||
| 7 | // generateHistorySidebarCSS creates GTK4 CSS for the history sidebar component. | 3 | // generateHistorySidebarCSS creates GTK4 CSS for the history sidebar component. | ||
| 8 | - | func generateHistorySidebarCSS(p Palette) string { | 4 | + | func generateHistorySidebarCSS(_ Palette) string { |
| 9 | - | accentAlpha := fmt.Sprintf("alpha(%s, 0.18)", p.Accent) | 5 | + | return `/* ===== History Sidebar Styling ===== */ |
| 10 | - | ||||
| 11 | - | return fmt.Sprintf(`/* ===== History Sidebar Styling ===== */ | |||
| 12 | 6 | ||||
| 13 | .history-sidebar-outer { | 7 | .history-sidebar-outer { | ||
| 14 | background-color: var(--surface); | 8 | background-color: var(--surface); | ||
| 15 | - | border-left: 1px solid var(--border); | 9 | + | border-left: 0.0625em solid var(--border); |
| 16 | } | 10 | } | ||
| 17 | 11 | ||||
| 18 | .history-sidebar-search-box { | 12 | .history-sidebar-search-box { | ||
| 19 | - | padding: 6px 8px; | 13 | + | padding: 0.375em 0.5em; |
| 20 | - | border-bottom: 1px solid var(--border); | 14 | + | border-bottom: 0.0625em solid var(--border); |
| 21 | background-color: var(--surface); | 15 | background-color: var(--surface); | ||
| 22 | } | 16 | } | ||
| 23 | 17 | ||||
| 24 | .history-sidebar-search { | 18 | .history-sidebar-search { | ||
| 25 | - | padding: 2px 6px; | 19 | + | padding: 0.125em 0.375em; |
| 26 | font-size: 0.85em; | 20 | font-size: 0.85em; | ||
| 27 | } | 21 | } | ||
| 28 | 22 | ||||
| @@ -31,35 +25,35 @@ func generateHistorySidebarCSS(p Palette) string { | |||||
| 31 | } | 25 | } | ||
| 32 | 26 | ||||
| 33 | .history-sidebar-group-header { | 27 | .history-sidebar-group-header { | ||
| 34 | - | padding: 4px 10px; | 28 | + | padding: 0.25em 0.625em; |
| 35 | - | padding-top: 6px; | 29 | + | padding-top: 0.375em; |
| 36 | font-size: 0.75em; | 30 | font-size: 0.75em; | ||
| 37 | font-weight: 600; | 31 | font-weight: 600; | ||
| 38 | color: var(--muted); | 32 | color: var(--muted); | ||
| 39 | text-transform: uppercase; | 33 | text-transform: uppercase; | ||
| 40 | letter-spacing: 0.04em; | 34 | letter-spacing: 0.04em; | ||
| 41 | background-color: var(--surface-variant); | 35 | background-color: var(--surface-variant); | ||
| 42 | - | border-bottom: 1px solid var(--border); | 36 | + | border-bottom: 0.0625em solid var(--border); |
| 43 | } | 37 | } | ||
| 44 | 38 | ||||
| 45 | .history-sidebar-row { | 39 | .history-sidebar-row { | ||
| 46 | - | padding: 3px 10px; | 40 | + | padding: 0.1875em 0.625em; |
| 47 | min-height: 0; | 41 | min-height: 0; | ||
| 48 | - | border-bottom: 1px solid alpha(var(--border), 0.4); | 42 | + | border-bottom: 0.0625em solid alpha(var(--border), 0.4); |
| 49 | background-color: var(--surface); | 43 | background-color: var(--surface); | ||
| 50 | transition: background-color 100ms ease; | 44 | transition: background-color 100ms ease; | ||
| 51 | } | 45 | } | ||
| 52 | 46 | ||||
| 53 | .history-sidebar-row:hover { | 47 | .history-sidebar-row:hover { | ||
| 54 | - | background-color: %s; | 48 | + | background-color: alpha(var(--accent), 0.18); |
| 55 | } | 49 | } | ||
| 56 | 50 | ||||
| 57 | .history-sidebar-row:selected { | 51 | .history-sidebar-row:selected { | ||
| 58 | - | background-color: %s; | 52 | + | background-color: alpha(var(--accent), 0.18); |
| 59 | } | 53 | } | ||
| 60 | 54 | ||||
| 61 | .history-sidebar-row:focus { | 55 | .history-sidebar-row:focus { | ||
| 62 | - | background-color: %s; | 56 | + | background-color: alpha(var(--accent), 0.18); |
| 63 | } | 57 | } | ||
| 64 | 58 | ||||
| 65 | .history-sidebar-row-title { | 59 | .history-sidebar-row-title { | ||
| @@ -76,21 +70,21 @@ func generateHistorySidebarCSS(p Palette) string { | |||||
| 76 | .history-sidebar-row-time { | 70 | .history-sidebar-row-time { | ||
| 77 | font-size: 0.68em; | 71 | font-size: 0.68em; | ||
| 78 | color: var(--muted); | 72 | color: var(--muted); | ||
| 79 | - | padding-left: 8px; | 73 | + | padding-left: 0.5em; |
| 80 | opacity: 0.75; | 74 | opacity: 0.75; | ||
| 81 | } | 75 | } | ||
| 82 | 76 | ||||
| 83 | .history-sidebar-empty { | 77 | .history-sidebar-empty { | ||
| 84 | - | padding: 24px 12px; | 78 | + | padding: 1.5em 0.75em; |
| 85 | font-size: 0.82em; | 79 | font-size: 0.82em; | ||
| 86 | color: var(--muted); | 80 | color: var(--muted); | ||
| 87 | font-style: italic; | 81 | font-style: italic; | ||
| 88 | } | 82 | } | ||
| 89 | 83 | ||||
| 90 | .history-sidebar-loading { | 84 | .history-sidebar-loading { | ||
| 91 | - | padding: 24px 12px; | 85 | + | padding: 1.5em 0.75em; |
| 92 | font-size: 0.82em; | 86 | font-size: 0.82em; | ||
| 93 | color: var(--muted); | 87 | color: var(--muted); | ||
| 94 | } | 88 | } | ||
| 95 | - | `, accentAlpha, accentAlpha, accentAlpha) | 89 | + | ` |
| 96 | } | 90 | } | ||
M internal/ui/theme/history_sidebar_css_test.go +32 -39
| @@ -2,7 +2,6 @@ package theme | |||
| 2 | 2 | ||
| 3 | 3 | import ( | |
| 4 | 4 | "crypto/sha256" | |
| 5 | - | "fmt" | |
| 6 | 5 | "strings" | |
| 7 | 6 | "testing" | |
| 8 | 7 | ||
| @@ -36,14 +35,15 @@ func TestGenerateHistorySidebarCSS_ContainsExpectedSelectors(t *testing.T) { | |||
| 36 | 35 | } | |
| 37 | 36 | ||
| 38 | 37 | func TestGenerateHistorySidebarCSS_AccentAlphaInterpolation(t *testing.T) { | |
| 39 | - | darkPalette := DefaultDarkPalette() | |
| 40 | - | css := generateHistorySidebarCSS(darkPalette) | |
| 38 | + | css := generateHistorySidebarCSS(DefaultDarkPalette()) | |
| 41 | 39 | ||
| 42 | - | expectedAlpha := fmt.Sprintf("alpha(%s, 0.18)", darkPalette.Accent) | |
| 40 | + | expectedAlpha := "alpha(var(--accent), 0.18)" | |
| 43 | 41 | assert.Contains(t, css, expectedAlpha, | |
| 44 | - | "expected accent alpha value %q in generated CSS", expectedAlpha) | |
| 42 | + | "expected accent alpha variable usage %q in generated CSS", expectedAlpha) | |
| 43 | + | assert.NotContains(t, css, "alpha(#", | |
| 44 | + | "history sidebar CSS should not inline hardcoded accent hex values") | |
| 45 | 45 | ||
| 46 | - | // The accent alpha should appear in hover, selected, and focus blocks | |
| 46 | + | // The accent alpha should appear in hover, selected, and focus blocks. | |
| 47 | 47 | assert.GreaterOrEqual(t, strings.Count(css, expectedAlpha), 3, | |
| 48 | 48 | "expected accent alpha to appear at least 3 times (hover/selected/focus)") | |
| 49 | 49 | } | |
| @@ -60,24 +60,14 @@ func TestGenerateHistorySidebarCSS_DeterministicOutput(t *testing.T) { | |||
| 60 | 60 | assert.Equal(t, hash1, hash2, "CSS output must be deterministic for the same palette") | |
| 61 | 61 | } | |
| 62 | 62 | ||
| 63 | - | func TestGenerateHistorySidebarCSS_DifferentPalettesProduceDifferentCSS(t *testing.T) { | |
| 63 | + | func TestGenerateHistorySidebarCSS_UsesPaletteVariablesInsteadOfInliningColors(t *testing.T) { | |
| 64 | 64 | darkCSS := generateHistorySidebarCSS(DefaultDarkPalette()) | |
| 65 | 65 | lightCSS := generateHistorySidebarCSS(DefaultLightPalette()) | |
| 66 | 66 | ||
| 67 | - | hashDark := sha256.Sum256([]byte(darkCSS)) | |
| 68 | - | hashLight := sha256.Sum256([]byte(lightCSS)) | |
| 69 | - | ||
| 70 | - | assert.NotEqual(t, hashDark, hashLight, | |
| 71 | - | "dark and light palettes should produce different CSS") | |
| 72 | - | } | |
| 73 | - | ||
| 74 | - | func TestGenerateHistorySidebarCSS_LightPaletteContainsAccentAlpha(t *testing.T) { | |
| 75 | - | lightPalette := DefaultLightPalette() | |
| 76 | - | css := generateHistorySidebarCSS(lightPalette) | |
| 77 | - | ||
| 78 | - | expectedAlpha := fmt.Sprintf("alpha(%s, 0.18)", lightPalette.Accent) | |
| 79 | - | assert.Contains(t, css, expectedAlpha, | |
| 80 | - | "expected light palette accent alpha value %q in generated CSS", expectedAlpha) | |
| 67 | + | assert.Equal(t, darkCSS, lightCSS, | |
| 68 | + | "history sidebar fragment should be palette-independent when it relies on CSS variables") | |
| 69 | + | assert.Contains(t, darkCSS, "alpha(var(--accent), 0.18)") | |
| 70 | + | assert.NotContains(t, darkCSS, "alpha(#") | |
| 81 | 71 | } | |
| 82 | 72 | ||
| 83 | 73 | func TestGenerateHistorySidebarCSS_ThroughGenerateCSS(t *testing.T) { | |
| @@ -126,8 +116,10 @@ func TestGenerateHistorySidebarCSS_CustomPaletteValuesInterpolated(t *testing.T) | |||
| 126 | 116 | ||
| 127 | 117 | css := generateHistorySidebarCSS(customPalette) | |
| 128 | 118 | ||
| 129 | - | assert.Contains(t, css, "alpha(#ff6600, 0.18)", | |
| 130 | - | "custom accent should be interpolated into CSS") | |
| 119 | + | assert.Contains(t, css, "alpha(var(--accent), 0.18)", | |
| 120 | + | "sidebar CSS should use the shared accent variable") | |
| 121 | + | assert.NotContains(t, css, "#ff6600", | |
| 122 | + | "sidebar fragment should not inline palette-specific accent values") | |
| 131 | 123 | ||
| 132 | 124 | assert.Contains(t, css, ".history-sidebar-outer {") | |
| 133 | 125 | assert.Contains(t, css, ".history-sidebar-row-title {") | |
| @@ -284,13 +276,14 @@ func TestGenerateHistorySidebarCSS_LiveReloadFullPath(t *testing.T) { | |||
| 284 | 276 | assert.NotEqual(t, darkCSS, customCSS, "dark and custom GenerateCSSFull output must differ") | |
| 285 | 277 | assert.NotEqual(t, lightCSS, customCSS, "light and custom GenerateCSSFull output must differ") | |
| 286 | 278 | ||
| 287 | - | // The accent alpha interpolation must use each palette's accent. | |
| 288 | - | assert.Contains(t, darkCSS, fmt.Sprintf("alpha(%s, 0.18)", darkPalette.Accent), | |
| 289 | - | "dark CSS must contain dark palette accent alpha") | |
| 290 | - | assert.Contains(t, lightCSS, fmt.Sprintf("alpha(%s, 0.18)", lightPalette.Accent), | |
| 291 | - | "light CSS must contain light palette accent alpha") | |
| 292 | - | assert.Contains(t, customCSS, fmt.Sprintf("alpha(%s, 0.18)", customPalette.Accent), | |
| 293 | - | "custom CSS must contain custom palette accent alpha") | |
| 279 | + | // Sidebar CSS should always use the accent variable, while the :root block | |
| 280 | + | // carries the palette-specific value that changes on live reload. | |
| 281 | + | assert.Contains(t, darkCSS, "alpha(var(--accent), 0.18)") | |
| 282 | + | assert.Contains(t, lightCSS, "alpha(var(--accent), 0.18)") | |
| 283 | + | assert.Contains(t, customCSS, "alpha(var(--accent), 0.18)") | |
| 284 | + | assert.Contains(t, darkCSS, "\n --accent: "+darkPalette.Accent+";\n") | |
| 285 | + | assert.Contains(t, lightCSS, "\n --accent: "+lightPalette.Accent+";\n") | |
| 286 | + | assert.Contains(t, customCSS, "\n --accent: "+customPalette.Accent+";\n") | |
| 294 | 287 | ||
| 295 | 288 | // All three outputs must be syntactically valid: every '{' has a matching '}'. | |
| 296 | 289 | for name, css := range map[string]string{ | |
| @@ -312,23 +305,23 @@ func TestGenerateHistorySidebarCSS_LiveReloadPaletteSwitch(t *testing.T) { | |||
| 312 | 305 | current := DefaultDarkPalette() | |
| 313 | 306 | cssBefore := GenerateCSSFull(current, 1.0, DefaultFontConfig(), DefaultModeColors()) | |
| 314 | 307 | ||
| 315 | - | // Verify the initial dark accent is in the sidebar sections. | |
| 316 | - | assert.Contains(t, cssBefore, fmt.Sprintf("alpha(%s, 0.18)", current.Accent), | |
| 317 | - | "initial dark palette accent must appear in generated CSS") | |
| 308 | + | // Verify the sidebar section uses the shared accent variable. | |
| 309 | + | assert.Contains(t, cssBefore, "alpha(var(--accent), 0.18)", | |
| 310 | + | "initial palette should render sidebar CSS through the accent variable") | |
| 311 | + | assert.Contains(t, cssBefore, "\n --accent: "+current.Accent+";\n") | |
| 318 | 312 | ||
| 319 | 313 | // Switch to light palette (simulating runtime theme change). | |
| 320 | 314 | current = DefaultLightPalette() | |
| 321 | 315 | cssAfter := GenerateCSSFull(current, 1.0, DefaultFontConfig(), DefaultModeColors()) | |
| 322 | 316 | ||
| 323 | - | // Verify the light accent replaces the dark one. | |
| 324 | - | assert.Contains(t, cssAfter, fmt.Sprintf("alpha(%s, 0.18)", current.Accent), | |
| 325 | - | "light palette accent must appear in CSS after switch") | |
| 317 | + | assert.Contains(t, cssAfter, "alpha(var(--accent), 0.18)", | |
| 318 | + | "updated palette should still use the shared accent variable") | |
| 319 | + | assert.Contains(t, cssAfter, "\n --accent: "+current.Accent+";\n") | |
| 326 | 320 | ||
| 327 | - | // The dark accent value must NOT appear in the light CSS. | |
| 321 | + | // The old palette variable definition must not remain after switching. | |
| 328 | 322 | darkAccent := DefaultDarkPalette().Accent | |
| 329 | 323 | if current.Accent != darkAccent { | |
| 330 | - | assert.NotContains(t, cssAfter, fmt.Sprintf("alpha(%s, 0.18)", darkAccent), | |
| 331 | - | "dark accent must not remain in CSS after switching to light palette") | |
| 324 | + | assert.NotContains(t, cssAfter, "\n --accent: "+darkAccent+";\n") | |
| 332 | 325 | } | |
| 333 | 326 | ||
| 334 | 327 | // History sidebar selectors must be present in both. | |
| @@ -2,7 +2,6 @@ package theme | |||||
| 2 | 2 | ||||
| 3 | import ( | 3 | import ( | ||
| 4 | "crypto/sha256" | 4 | "crypto/sha256" | ||
| 5 | - | "fmt" | |||
| 6 | "strings" | 5 | "strings" | ||
| 7 | "testing" | 6 | "testing" | ||
| 8 | 7 | ||||
| @@ -36,14 +35,15 @@ func TestGenerateHistorySidebarCSS_ContainsExpectedSelectors(t *testing.T) { | |||||
| 36 | } | 35 | } | ||
| 37 | 36 | ||||
| 38 | func TestGenerateHistorySidebarCSS_AccentAlphaInterpolation(t *testing.T) { | 37 | func TestGenerateHistorySidebarCSS_AccentAlphaInterpolation(t *testing.T) { | ||
| 39 | - | darkPalette := DefaultDarkPalette() | 38 | + | css := generateHistorySidebarCSS(DefaultDarkPalette()) |
| 40 | - | css := generateHistorySidebarCSS(darkPalette) | |||
| 41 | 39 | ||||
| 42 | - | expectedAlpha := fmt.Sprintf("alpha(%s, 0.18)", darkPalette.Accent) | 40 | + | expectedAlpha := "alpha(var(--accent), 0.18)" |
| 43 | assert.Contains(t, css, expectedAlpha, | 41 | assert.Contains(t, css, expectedAlpha, | ||
| 44 | - | "expected accent alpha value %q in generated CSS", expectedAlpha) | 42 | + | "expected accent alpha variable usage %q in generated CSS", expectedAlpha) |
| 43 | + | assert.NotContains(t, css, "alpha(#", | |||
| 44 | + | "history sidebar CSS should not inline hardcoded accent hex values") | |||
| 45 | 45 | ||||
| 46 | - | // The accent alpha should appear in hover, selected, and focus blocks | 46 | + | // The accent alpha should appear in hover, selected, and focus blocks. |
| 47 | assert.GreaterOrEqual(t, strings.Count(css, expectedAlpha), 3, | 47 | assert.GreaterOrEqual(t, strings.Count(css, expectedAlpha), 3, | ||
| 48 | "expected accent alpha to appear at least 3 times (hover/selected/focus)") | 48 | "expected accent alpha to appear at least 3 times (hover/selected/focus)") | ||
| 49 | } | 49 | } | ||
| @@ -60,24 +60,14 @@ func TestGenerateHistorySidebarCSS_DeterministicOutput(t *testing.T) { | |||||
| 60 | assert.Equal(t, hash1, hash2, "CSS output must be deterministic for the same palette") | 60 | assert.Equal(t, hash1, hash2, "CSS output must be deterministic for the same palette") | ||
| 61 | } | 61 | } | ||
| 62 | 62 | ||||
| 63 | - | func TestGenerateHistorySidebarCSS_DifferentPalettesProduceDifferentCSS(t *testing.T) { | 63 | + | func TestGenerateHistorySidebarCSS_UsesPaletteVariablesInsteadOfInliningColors(t *testing.T) { |
| 64 | darkCSS := generateHistorySidebarCSS(DefaultDarkPalette()) | 64 | darkCSS := generateHistorySidebarCSS(DefaultDarkPalette()) | ||
| 65 | lightCSS := generateHistorySidebarCSS(DefaultLightPalette()) | 65 | lightCSS := generateHistorySidebarCSS(DefaultLightPalette()) | ||
| 66 | 66 | ||||
| 67 | - | hashDark := sha256.Sum256([]byte(darkCSS)) | 67 | + | assert.Equal(t, darkCSS, lightCSS, |
| 68 | - | hashLight := sha256.Sum256([]byte(lightCSS)) | 68 | + | "history sidebar fragment should be palette-independent when it relies on CSS variables") |
| 69 | - | 69 | + | assert.Contains(t, darkCSS, "alpha(var(--accent), 0.18)") | |
| 70 | - | assert.NotEqual(t, hashDark, hashLight, | 70 | + | assert.NotContains(t, darkCSS, "alpha(#") |
| 71 | - | "dark and light palettes should produce different CSS") | |||
| 72 | - | } | |||
| 73 | - | ||||
| 74 | - | func TestGenerateHistorySidebarCSS_LightPaletteContainsAccentAlpha(t *testing.T) { | |||
| 75 | - | lightPalette := DefaultLightPalette() | |||
| 76 | - | css := generateHistorySidebarCSS(lightPalette) | |||
| 77 | - | ||||
| 78 | - | expectedAlpha := fmt.Sprintf("alpha(%s, 0.18)", lightPalette.Accent) | |||
| 79 | - | assert.Contains(t, css, expectedAlpha, | |||
| 80 | - | "expected light palette accent alpha value %q in generated CSS", expectedAlpha) | |||
| 81 | } | 71 | } | ||
| 82 | 72 | ||||
| 83 | func TestGenerateHistorySidebarCSS_ThroughGenerateCSS(t *testing.T) { | 73 | func TestGenerateHistorySidebarCSS_ThroughGenerateCSS(t *testing.T) { | ||
| @@ -126,8 +116,10 @@ func TestGenerateHistorySidebarCSS_CustomPaletteValuesInterpolated(t *testing.T) | |||||
| 126 | 116 | ||||
| 127 | css := generateHistorySidebarCSS(customPalette) | 117 | css := generateHistorySidebarCSS(customPalette) | ||
| 128 | 118 | ||||
| 129 | - | assert.Contains(t, css, "alpha(#ff6600, 0.18)", | 119 | + | assert.Contains(t, css, "alpha(var(--accent), 0.18)", |
| 130 | - | "custom accent should be interpolated into CSS") | 120 | + | "sidebar CSS should use the shared accent variable") |
| 121 | + | assert.NotContains(t, css, "#ff6600", | |||
| 122 | + | "sidebar fragment should not inline palette-specific accent values") | |||
| 131 | 123 | ||||
| 132 | assert.Contains(t, css, ".history-sidebar-outer {") | 124 | assert.Contains(t, css, ".history-sidebar-outer {") | ||
| 133 | assert.Contains(t, css, ".history-sidebar-row-title {") | 125 | assert.Contains(t, css, ".history-sidebar-row-title {") | ||
| @@ -284,13 +276,14 @@ func TestGenerateHistorySidebarCSS_LiveReloadFullPath(t *testing.T) { | |||||
| 284 | assert.NotEqual(t, darkCSS, customCSS, "dark and custom GenerateCSSFull output must differ") | 276 | assert.NotEqual(t, darkCSS, customCSS, "dark and custom GenerateCSSFull output must differ") | ||
| 285 | assert.NotEqual(t, lightCSS, customCSS, "light and custom GenerateCSSFull output must differ") | 277 | assert.NotEqual(t, lightCSS, customCSS, "light and custom GenerateCSSFull output must differ") | ||
| 286 | 278 | ||||
| 287 | - | // The accent alpha interpolation must use each palette's accent. | 279 | + | // Sidebar CSS should always use the accent variable, while the :root block |
| 288 | - | assert.Contains(t, darkCSS, fmt.Sprintf("alpha(%s, 0.18)", darkPalette.Accent), | 280 | + | // carries the palette-specific value that changes on live reload. |
| 289 | - | "dark CSS must contain dark palette accent alpha") | 281 | + | assert.Contains(t, darkCSS, "alpha(var(--accent), 0.18)") |
| 290 | - | assert.Contains(t, lightCSS, fmt.Sprintf("alpha(%s, 0.18)", lightPalette.Accent), | 282 | + | assert.Contains(t, lightCSS, "alpha(var(--accent), 0.18)") |
| 291 | - | "light CSS must contain light palette accent alpha") | 283 | + | assert.Contains(t, customCSS, "alpha(var(--accent), 0.18)") |
| 292 | - | assert.Contains(t, customCSS, fmt.Sprintf("alpha(%s, 0.18)", customPalette.Accent), | 284 | + | assert.Contains(t, darkCSS, "\n --accent: "+darkPalette.Accent+";\n") |
| 293 | - | "custom CSS must contain custom palette accent alpha") | 285 | + | assert.Contains(t, lightCSS, "\n --accent: "+lightPalette.Accent+";\n") |
| 286 | + | assert.Contains(t, customCSS, "\n --accent: "+customPalette.Accent+";\n") | |||
| 294 | 287 | ||||
| 295 | // All three outputs must be syntactically valid: every '{' has a matching '}'. | 288 | // All three outputs must be syntactically valid: every '{' has a matching '}'. | ||
| 296 | for name, css := range map[string]string{ | 289 | for name, css := range map[string]string{ | ||
| @@ -312,23 +305,23 @@ func TestGenerateHistorySidebarCSS_LiveReloadPaletteSwitch(t *testing.T) { | |||||
| 312 | current := DefaultDarkPalette() | 305 | current := DefaultDarkPalette() | ||
| 313 | cssBefore := GenerateCSSFull(current, 1.0, DefaultFontConfig(), DefaultModeColors()) | 306 | cssBefore := GenerateCSSFull(current, 1.0, DefaultFontConfig(), DefaultModeColors()) | ||
| 314 | 307 | ||||
| 315 | - | // Verify the initial dark accent is in the sidebar sections. | 308 | + | // Verify the sidebar section uses the shared accent variable. |
| 316 | - | assert.Contains(t, cssBefore, fmt.Sprintf("alpha(%s, 0.18)", current.Accent), | 309 | + | assert.Contains(t, cssBefore, "alpha(var(--accent), 0.18)", |
| 317 | - | "initial dark palette accent must appear in generated CSS") | 310 | + | "initial palette should render sidebar CSS through the accent variable") |
| 311 | + | assert.Contains(t, cssBefore, "\n --accent: "+current.Accent+";\n") | |||
| 318 | 312 | ||||
| 319 | // Switch to light palette (simulating runtime theme change). | 313 | // Switch to light palette (simulating runtime theme change). | ||
| 320 | current = DefaultLightPalette() | 314 | current = DefaultLightPalette() | ||
| 321 | cssAfter := GenerateCSSFull(current, 1.0, DefaultFontConfig(), DefaultModeColors()) | 315 | cssAfter := GenerateCSSFull(current, 1.0, DefaultFontConfig(), DefaultModeColors()) | ||
| 322 | 316 | ||||
| 323 | - | // Verify the light accent replaces the dark one. | 317 | + | assert.Contains(t, cssAfter, "alpha(var(--accent), 0.18)", |
| 324 | - | assert.Contains(t, cssAfter, fmt.Sprintf("alpha(%s, 0.18)", current.Accent), | 318 | + | "updated palette should still use the shared accent variable") |
| 325 | - | "light palette accent must appear in CSS after switch") | 319 | + | assert.Contains(t, cssAfter, "\n --accent: "+current.Accent+";\n") |
| 326 | 320 | ||||
| 327 | - | // The dark accent value must NOT appear in the light CSS. | 321 | + | // The old palette variable definition must not remain after switching. |
| 328 | darkAccent := DefaultDarkPalette().Accent | 322 | darkAccent := DefaultDarkPalette().Accent | ||
| 329 | if current.Accent != darkAccent { | 323 | if current.Accent != darkAccent { | ||
| 330 | - | assert.NotContains(t, cssAfter, fmt.Sprintf("alpha(%s, 0.18)", darkAccent), | 324 | + | assert.NotContains(t, cssAfter, "\n --accent: "+darkAccent+";\n") |
| 331 | - | "dark accent must not remain in CSS after switching to light palette") | |||
| 332 | } | 325 | } | ||
| 333 | 326 | ||||
| 334 | // History sidebar selectors must be present in both. | 327 | // History sidebar selectors must be present in both. | ||