refactor(omnibox): drop redundant most-visited flag
- commit
- 717ad15489937c3dba7c15a538e9c04ad0150be9
- author
- brice · · 2026-04-12 06:42 UTC
- parent
- b65c4d9
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
4 files · +16 -32
M internal/ui/app.go +0 -1
| @@ -803,7 +803,6 @@ func buildOmniboxConfig( | |||
| 803 | 803 | DefaultSearch: deps.Config.DefaultSearchEngine, | |
| 804 | 804 | InitialBehavior: deps.Config.Omnibox.InitialBehavior, | |
| 805 | 805 | MostVisitedDays: deps.Config.Omnibox.MostVisitedDays, | |
| 806 | - | MostVisitedDaysSet: true, | |
| 807 | 806 | SaveInitialBehavior: deps.HandlerDeps.SaveOmniboxInitialBehavior, | |
| 808 | 807 | UIScale: deps.Config.DefaultUIScale, | |
| 809 | 808 | OnNavigate: callbacks.OnNavigate, | |
| @@ -803,7 +803,6 @@ func buildOmniboxConfig( | |||||
| 803 | DefaultSearch: deps.Config.DefaultSearchEngine, | 803 | DefaultSearch: deps.Config.DefaultSearchEngine, | ||
| 804 | InitialBehavior: deps.Config.Omnibox.InitialBehavior, | 804 | InitialBehavior: deps.Config.Omnibox.InitialBehavior, | ||
| 805 | MostVisitedDays: deps.Config.Omnibox.MostVisitedDays, | 805 | MostVisitedDays: deps.Config.Omnibox.MostVisitedDays, | ||
| 806 | - | MostVisitedDaysSet: true, | |||
| 807 | SaveInitialBehavior: deps.HandlerDeps.SaveOmniboxInitialBehavior, | 806 | SaveInitialBehavior: deps.HandlerDeps.SaveOmniboxInitialBehavior, | ||
| 808 | UIScale: deps.Config.DefaultUIScale, | 807 | UIScale: deps.Config.DefaultUIScale, | ||
| 809 | OnNavigate: callbacks.OnNavigate, | 808 | OnNavigate: callbacks.OnNavigate, | ||
M internal/ui/app_omnibox_config_test.go +0 -3
| @@ -19,7 +19,4 @@ func TestBuildOmniboxConfig_UsesConfiguredMostVisitedDays(t *testing.T) { | |||
| 19 | 19 | if got.MostVisitedDays != 7 { | |
| 20 | 20 | t.Fatalf("MostVisitedDays = %d, want 7", got.MostVisitedDays) | |
| 21 | 21 | } | |
| 22 | - | if !got.MostVisitedDaysSet { | |
| 23 | - | t.Fatal("MostVisitedDaysSet = false, want true") | |
| 24 | - | } | |
| 25 | 22 | } | |
| @@ -19,7 +19,4 @@ func TestBuildOmniboxConfig_UsesConfiguredMostVisitedDays(t *testing.T) { | |||||
| 19 | if got.MostVisitedDays != 7 { | 19 | if got.MostVisitedDays != 7 { | ||
| 20 | t.Fatalf("MostVisitedDays = %d, want 7", got.MostVisitedDays) | 20 | t.Fatalf("MostVisitedDays = %d, want 7", got.MostVisitedDays) | ||
| 21 | } | 21 | } | ||
| 22 | - | if !got.MostVisitedDaysSet { | |||
| 23 | - | t.Fatal("MostVisitedDaysSet = false, want true") | |||
| 24 | - | } | |||
| 25 | } | 22 | } | ||
M internal/ui/component/omnibox.go +7 -18
| @@ -27,12 +27,11 @@ import ( | |||
| 27 | 27 | ) | |
| 28 | 28 | ||
| 29 | 29 | const ( | |
| 30 | - | debounceDelayMs = 50 | |
| 31 | - | defaultOmniboxMostVisitedDays = 30 | |
| 32 | - | endBoxSpacing = 6 | |
| 33 | - | defaultOmniboxPlaceholder = "Search history or enter URL… (! lists bangs)" | |
| 34 | - | minGhostInputLength = 1 | |
| 35 | - | initialBehaviorBadgeTooltip = "Toggle default history order (Ctrl+R)" | |
| 30 | + | debounceDelayMs = 50 | |
| 31 | + | endBoxSpacing = 6 | |
| 32 | + | defaultOmniboxPlaceholder = "Search history or enter URL… (! lists bangs)" | |
| 33 | + | minGhostInputLength = 1 | |
| 34 | + | initialBehaviorBadgeTooltip = "Toggle default history order (Ctrl+R)" | |
| 36 | 35 | ) | |
| 37 | 36 | ||
| 38 | 37 | type favoriteRowIndicatorUpdate struct { | |
| @@ -117,7 +116,6 @@ type Omnibox struct { | |||
| 117 | 116 | defaultSearch string | |
| 118 | 117 | initialBehavior entity.OmniboxInitialBehavior | |
| 119 | 118 | mostVisitedDays int | |
| 120 | - | mostVisitedDaysSet bool | |
| 121 | 119 | saveInitialBehaviorFn func(context.Context, entity.OmniboxInitialBehavior) error | |
| 122 | 120 | ctx context.Context | |
| 123 | 121 | ||
| @@ -165,7 +163,6 @@ type OmniboxConfig struct { | |||
| 165 | 163 | DefaultSearch string | |
| 166 | 164 | InitialBehavior entity.OmniboxInitialBehavior | |
| 167 | 165 | MostVisitedDays int | |
| 168 | - | MostVisitedDaysSet bool | |
| 169 | 166 | SaveInitialBehavior func(ctx context.Context, behavior entity.OmniboxInitialBehavior) error | |
| 170 | 167 | UIScale float64 // UI scale for favicon sizing | |
| 171 | 168 | OnNavigate func(url string) // Callback when user navigates via omnibox | |
| @@ -186,10 +183,6 @@ func NewOmnibox(ctx context.Context, cfg OmniboxConfig) *Omnibox { | |||
| 186 | 183 | if uiScale <= 0 { | |
| 187 | 184 | uiScale = 1.0 | |
| 188 | 185 | } | |
| 189 | - | mostVisitedDays := cfg.MostVisitedDays | |
| 190 | - | if !cfg.MostVisitedDaysSet { | |
| 191 | - | mostVisitedDays = defaultOmniboxMostVisitedDays | |
| 192 | - | } | |
| 193 | 186 | ||
| 194 | 187 | sizeCfg := ResolveModalSizeConfig(cfg.SizeConfig, OmniboxSizeDefaults) | |
| 195 | 188 | ||
| @@ -203,8 +196,7 @@ func NewOmnibox(ctx context.Context, cfg OmniboxConfig) *Omnibox { | |||
| 203 | 196 | shortcutsUC: cfg.ShortcutsUC, | |
| 204 | 197 | defaultSearch: cfg.DefaultSearch, | |
| 205 | 198 | initialBehavior: cfg.InitialBehavior, | |
| 206 | - | mostVisitedDays: mostVisitedDays, | |
| 207 | - | mostVisitedDaysSet: cfg.MostVisitedDaysSet, | |
| 199 | + | mostVisitedDays: cfg.MostVisitedDays, | |
| 208 | 200 | saveInitialBehaviorFn: cfg.SaveInitialBehavior, | |
| 209 | 201 | onToast: cfg.OnToast, | |
| 210 | 202 | onAccentKeyPress: cfg.OnAccentKeyPress, | |
| @@ -1602,7 +1594,6 @@ func (o *Omnibox) loadInitialHistory(token uint64) { | |||
| 1602 | 1594 | initialLimit := o.effectiveMaxRows() | |
| 1603 | 1595 | initialBehavior := o.initialBehavior | |
| 1604 | 1596 | mostVisitedDays := o.mostVisitedDays | |
| 1605 | - | mostVisitedDaysSet := o.mostVisitedDaysSet | |
| 1606 | 1597 | ||
| 1607 | 1598 | go func() { | |
| 1608 | 1599 | ctx := o.ctx | |
| @@ -1629,9 +1620,7 @@ func (o *Omnibox) loadInitialHistory(token uint64) { | |||
| 1629 | 1620 | err error | |
| 1630 | 1621 | ) | |
| 1631 | 1622 | if initialBehavior == entity.OmniboxInitialBehaviorMostVisited { | |
| 1632 | - | if !mostVisitedDaysSet { | |
| 1633 | - | results, err = o.historyUC.GetMostVisited(ctx, defaultOmniboxMostVisitedDays) | |
| 1634 | - | } else if mostVisitedDays == 0 { | |
| 1623 | + | if mostVisitedDays == 0 { | |
| 1635 | 1624 | results, err = o.historyUC.GetMostVisited(ctx, 0) | |
| 1636 | 1625 | } else { | |
| 1637 | 1626 | results, err = o.historyUC.GetMostVisited(ctx, mostVisitedDays) | |
| @@ -27,12 +27,11 @@ import ( | |||||
| 27 | ) | 27 | ) | ||
| 28 | 28 | ||||
| 29 | const ( | 29 | const ( | ||
| 30 | - | debounceDelayMs = 50 | 30 | + | debounceDelayMs = 50 |
| 31 | - | defaultOmniboxMostVisitedDays = 30 | 31 | + | endBoxSpacing = 6 |
| 32 | - | endBoxSpacing = 6 | 32 | + | defaultOmniboxPlaceholder = "Search history or enter URL… (! lists bangs)" |
| 33 | - | defaultOmniboxPlaceholder = "Search history or enter URL… (! lists bangs)" | 33 | + | minGhostInputLength = 1 |
| 34 | - | minGhostInputLength = 1 | 34 | + | initialBehaviorBadgeTooltip = "Toggle default history order (Ctrl+R)" |
| 35 | - | initialBehaviorBadgeTooltip = "Toggle default history order (Ctrl+R)" | |||
| 36 | ) | 35 | ) | ||
| 37 | 36 | ||||
| 38 | type favoriteRowIndicatorUpdate struct { | 37 | type favoriteRowIndicatorUpdate struct { | ||
| @@ -117,7 +116,6 @@ type Omnibox struct { | |||||
| 117 | defaultSearch string | 116 | defaultSearch string | ||
| 118 | initialBehavior entity.OmniboxInitialBehavior | 117 | initialBehavior entity.OmniboxInitialBehavior | ||
| 119 | mostVisitedDays int | 118 | mostVisitedDays int | ||
| 120 | - | mostVisitedDaysSet bool | |||
| 121 | saveInitialBehaviorFn func(context.Context, entity.OmniboxInitialBehavior) error | 119 | saveInitialBehaviorFn func(context.Context, entity.OmniboxInitialBehavior) error | ||
| 122 | ctx context.Context | 120 | ctx context.Context | ||
| 123 | 121 | ||||
| @@ -165,7 +163,6 @@ type OmniboxConfig struct { | |||||
| 165 | DefaultSearch string | 163 | DefaultSearch string | ||
| 166 | InitialBehavior entity.OmniboxInitialBehavior | 164 | InitialBehavior entity.OmniboxInitialBehavior | ||
| 167 | MostVisitedDays int | 165 | MostVisitedDays int | ||
| 168 | - | MostVisitedDaysSet bool | |||
| 169 | SaveInitialBehavior func(ctx context.Context, behavior entity.OmniboxInitialBehavior) error | 166 | SaveInitialBehavior func(ctx context.Context, behavior entity.OmniboxInitialBehavior) error | ||
| 170 | UIScale float64 // UI scale for favicon sizing | 167 | UIScale float64 // UI scale for favicon sizing | ||
| 171 | OnNavigate func(url string) // Callback when user navigates via omnibox | 168 | OnNavigate func(url string) // Callback when user navigates via omnibox | ||
| @@ -186,10 +183,6 @@ func NewOmnibox(ctx context.Context, cfg OmniboxConfig) *Omnibox { | |||||
| 186 | if uiScale <= 0 { | 183 | if uiScale <= 0 { | ||
| 187 | uiScale = 1.0 | 184 | uiScale = 1.0 | ||
| 188 | } | 185 | } | ||
| 189 | - | mostVisitedDays := cfg.MostVisitedDays | |||
| 190 | - | if !cfg.MostVisitedDaysSet { | |||
| 191 | - | mostVisitedDays = defaultOmniboxMostVisitedDays | |||
| 192 | - | } | |||
| 193 | 186 | ||||
| 194 | sizeCfg := ResolveModalSizeConfig(cfg.SizeConfig, OmniboxSizeDefaults) | 187 | sizeCfg := ResolveModalSizeConfig(cfg.SizeConfig, OmniboxSizeDefaults) | ||
| 195 | 188 | ||||
| @@ -203,8 +196,7 @@ func NewOmnibox(ctx context.Context, cfg OmniboxConfig) *Omnibox { | |||||
| 203 | shortcutsUC: cfg.ShortcutsUC, | 196 | shortcutsUC: cfg.ShortcutsUC, | ||
| 204 | defaultSearch: cfg.DefaultSearch, | 197 | defaultSearch: cfg.DefaultSearch, | ||
| 205 | initialBehavior: cfg.InitialBehavior, | 198 | initialBehavior: cfg.InitialBehavior, | ||
| 206 | - | mostVisitedDays: mostVisitedDays, | 199 | + | mostVisitedDays: cfg.MostVisitedDays, |
| 207 | - | mostVisitedDaysSet: cfg.MostVisitedDaysSet, | |||
| 208 | saveInitialBehaviorFn: cfg.SaveInitialBehavior, | 200 | saveInitialBehaviorFn: cfg.SaveInitialBehavior, | ||
| 209 | onToast: cfg.OnToast, | 201 | onToast: cfg.OnToast, | ||
| 210 | onAccentKeyPress: cfg.OnAccentKeyPress, | 202 | onAccentKeyPress: cfg.OnAccentKeyPress, | ||
| @@ -1602,7 +1594,6 @@ func (o *Omnibox) loadInitialHistory(token uint64) { | |||||
| 1602 | initialLimit := o.effectiveMaxRows() | 1594 | initialLimit := o.effectiveMaxRows() | ||
| 1603 | initialBehavior := o.initialBehavior | 1595 | initialBehavior := o.initialBehavior | ||
| 1604 | mostVisitedDays := o.mostVisitedDays | 1596 | mostVisitedDays := o.mostVisitedDays | ||
| 1605 | - | mostVisitedDaysSet := o.mostVisitedDaysSet | |||
| 1606 | 1597 | ||||
| 1607 | go func() { | 1598 | go func() { | ||
| 1608 | ctx := o.ctx | 1599 | ctx := o.ctx | ||
| @@ -1629,9 +1620,7 @@ func (o *Omnibox) loadInitialHistory(token uint64) { | |||||
| 1629 | err error | 1620 | err error | ||
| 1630 | ) | 1621 | ) | ||
| 1631 | if initialBehavior == entity.OmniboxInitialBehaviorMostVisited { | 1622 | if initialBehavior == entity.OmniboxInitialBehaviorMostVisited { | ||
| 1632 | - | if !mostVisitedDaysSet { | 1623 | + | if mostVisitedDays == 0 { |
| 1633 | - | results, err = o.historyUC.GetMostVisited(ctx, defaultOmniboxMostVisitedDays) | |||
| 1634 | - | } else if mostVisitedDays == 0 { | |||
| 1635 | results, err = o.historyUC.GetMostVisited(ctx, 0) | 1624 | results, err = o.historyUC.GetMostVisited(ctx, 0) | ||
| 1636 | } else { | 1625 | } else { | ||
| 1637 | results, err = o.historyUC.GetMostVisited(ctx, mostVisitedDays) | 1626 | results, err = o.historyUC.GetMostVisited(ctx, mostVisitedDays) | ||
M internal/ui/component/omnibox_initial_behavior_test.go +9 -10
| @@ -192,6 +192,7 @@ func TestOmniboxLoadInitialHistory_MostVisitedUsesThirtyDayWindow(t *testing.T) | |||
| 192 | 192 | o := &Omnibox{ | |
| 193 | 193 | historyUC: appusecase.NewSearchHistoryUseCase(repo), | |
| 194 | 194 | initialBehavior: entity.OmniboxInitialBehaviorMostVisited, | |
| 195 | + | mostVisitedDays: 30, | |
| 195 | 196 | ctx: context.Background(), | |
| 196 | 197 | } | |
| 197 | 198 | ||
| @@ -215,11 +216,10 @@ func TestOmniboxLoadInitialHistory_MostVisitedUsesConfiguredWindow(t *testing.T) | |||
| 215 | 216 | ) | |
| 216 | 217 | ||
| 217 | 218 | o := &Omnibox{ | |
| 218 | - | historyUC: appusecase.NewSearchHistoryUseCase(repo), | |
| 219 | - | initialBehavior: entity.OmniboxInitialBehaviorMostVisited, | |
| 220 | - | mostVisitedDays: 7, | |
| 221 | - | mostVisitedDaysSet: true, | |
| 222 | - | ctx: context.Background(), | |
| 219 | + | historyUC: appusecase.NewSearchHistoryUseCase(repo), | |
| 220 | + | initialBehavior: entity.OmniboxInitialBehaviorMostVisited, | |
| 221 | + | mostVisitedDays: 7, | |
| 222 | + | ctx: context.Background(), | |
| 223 | 223 | } | |
| 224 | 224 | ||
| 225 | 225 | o.loadInitialHistory(1) | |
| @@ -242,11 +242,10 @@ func TestOmniboxLoadInitialHistory_MostVisitedZeroWindowUsesAllHistory(t *testin | |||
| 242 | 242 | ) | |
| 243 | 243 | ||
| 244 | 244 | o := &Omnibox{ | |
| 245 | - | historyUC: appusecase.NewSearchHistoryUseCase(repo), | |
| 246 | - | initialBehavior: entity.OmniboxInitialBehaviorMostVisited, | |
| 247 | - | mostVisitedDays: 0, | |
| 248 | - | mostVisitedDaysSet: true, | |
| 249 | - | ctx: context.Background(), | |
| 245 | + | historyUC: appusecase.NewSearchHistoryUseCase(repo), | |
| 246 | + | initialBehavior: entity.OmniboxInitialBehaviorMostVisited, | |
| 247 | + | mostVisitedDays: 0, | |
| 248 | + | ctx: context.Background(), | |
| 250 | 249 | } | |
| 251 | 250 | ||
| 252 | 251 | o.loadInitialHistory(1) | |
| @@ -192,6 +192,7 @@ func TestOmniboxLoadInitialHistory_MostVisitedUsesThirtyDayWindow(t *testing.T) | |||||
| 192 | o := &Omnibox{ | 192 | o := &Omnibox{ | ||
| 193 | historyUC: appusecase.NewSearchHistoryUseCase(repo), | 193 | historyUC: appusecase.NewSearchHistoryUseCase(repo), | ||
| 194 | initialBehavior: entity.OmniboxInitialBehaviorMostVisited, | 194 | initialBehavior: entity.OmniboxInitialBehaviorMostVisited, | ||
| 195 | + | mostVisitedDays: 30, | |||
| 195 | ctx: context.Background(), | 196 | ctx: context.Background(), | ||
| 196 | } | 197 | } | ||
| 197 | 198 | ||||
| @@ -215,11 +216,10 @@ func TestOmniboxLoadInitialHistory_MostVisitedUsesConfiguredWindow(t *testing.T) | |||||
| 215 | ) | 216 | ) | ||
| 216 | 217 | ||||
| 217 | o := &Omnibox{ | 218 | o := &Omnibox{ | ||
| 218 | - | historyUC: appusecase.NewSearchHistoryUseCase(repo), | 219 | + | historyUC: appusecase.NewSearchHistoryUseCase(repo), |
| 219 | - | initialBehavior: entity.OmniboxInitialBehaviorMostVisited, | 220 | + | initialBehavior: entity.OmniboxInitialBehaviorMostVisited, |
| 220 | - | mostVisitedDays: 7, | 221 | + | mostVisitedDays: 7, |
| 221 | - | mostVisitedDaysSet: true, | 222 | + | ctx: context.Background(), |
| 222 | - | ctx: context.Background(), | |||
| 223 | } | 223 | } | ||
| 224 | 224 | ||||
| 225 | o.loadInitialHistory(1) | 225 | o.loadInitialHistory(1) | ||
| @@ -242,11 +242,10 @@ func TestOmniboxLoadInitialHistory_MostVisitedZeroWindowUsesAllHistory(t *testin | |||||
| 242 | ) | 242 | ) | ||
| 243 | 243 | ||||
| 244 | o := &Omnibox{ | 244 | o := &Omnibox{ | ||
| 245 | - | historyUC: appusecase.NewSearchHistoryUseCase(repo), | 245 | + | historyUC: appusecase.NewSearchHistoryUseCase(repo), |
| 246 | - | initialBehavior: entity.OmniboxInitialBehaviorMostVisited, | 246 | + | initialBehavior: entity.OmniboxInitialBehaviorMostVisited, |
| 247 | - | mostVisitedDays: 0, | 247 | + | mostVisitedDays: 0, |
| 248 | - | mostVisitedDaysSet: true, | 248 | + | ctx: context.Background(), |
| 249 | - | ctx: context.Background(), | |||
| 250 | } | 249 | } | ||
| 251 | 250 | ||||
| 252 | o.loadInitialHistory(1) | 251 | o.loadInitialHistory(1) | ||