fix: scale chrome height with UI, schedule debounce on GTK main thread, add tests
- Chrome height in EffectiveMaxRows now scales with rowHeight (2×) instead of a fixed 150px constant, so it works at any UI scale - Debounce callback schedules performSearch via glib.IdleAdd to ensure GTK widget calls happen on the main thread - effectiveMaxRows fallback uses MaxVisibleRows for semantic consistency - Add unit tests for EffectiveMaxRows covering normal, edge, and boundary cases
- commit
- 8849bda3ba91c687f55f2bfbc4957cc732b209a8
- author
- brice · · 2026-01-29 06:45 UTC
- committer
- brice · · 2026-01-29 08:42 UTC
- parent
- 5b585ba
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
3 files · +102 -6
M internal/ui/component/modal_sizing.go +4 -4
| @@ -84,15 +84,15 @@ var TabPickerListDefaults = ListDisplayDefaults{ | |||
| 84 | 84 | ||
| 85 | 85 | // EffectiveMaxRows returns the maximum number of visible rows that fit | |
| 86 | 86 | // in the available space. It computes available height by subtracting the | |
| 87 | - | // top margin (TopMarginPct of parentHeight) and an estimated chrome height | |
| 88 | - | // (header + search entry ≈ 120px). If MaxVisibleRows worth of rows don't | |
| 89 | - | // fit, it falls back to SmallMaxVisibleRows. | |
| 87 | + | // top margin (TopMarginPct of parentHeight) and chrome height (header tabs + | |
| 88 | + | // search entry, estimated as 2× rowHeight to scale with the UI). If | |
| 89 | + | // MaxVisibleRows worth of rows don't fit, it falls back to SmallMaxVisibleRows. | |
| 90 | 90 | func EffectiveMaxRows(parentHeight, rowHeight int, sizeCfg ModalSizeConfig, defaults ListDisplayDefaults) int { | |
| 91 | 91 | if parentHeight <= 0 || rowHeight <= 0 || defaults.SmallMaxVisibleRows <= 0 { | |
| 92 | 92 | return defaults.MaxVisibleRows | |
| 93 | 93 | } | |
| 94 | 94 | topMargin := int(float64(parentHeight) * sizeCfg.TopMarginPct) | |
| 95 | - | const chromeHeight = 150 // header tabs + search entry + bottom padding | |
| 95 | + | chromeHeight := 2 * rowHeight // header tabs + search entry, scales with UI | |
| 96 | 96 | available := parentHeight - topMargin - chromeHeight | |
| 97 | 97 | needed := defaults.MaxVisibleRows * rowHeight | |
| 98 | 98 | if available < needed { | |
| @@ -84,15 +84,15 @@ var TabPickerListDefaults = ListDisplayDefaults{ | |||||
| 84 | 84 | ||||
| 85 | // EffectiveMaxRows returns the maximum number of visible rows that fit | 85 | // EffectiveMaxRows returns the maximum number of visible rows that fit | ||
| 86 | // in the available space. It computes available height by subtracting the | 86 | // in the available space. It computes available height by subtracting the | ||
| 87 | - | // top margin (TopMarginPct of parentHeight) and an estimated chrome height | 87 | + | // top margin (TopMarginPct of parentHeight) and chrome height (header tabs + |
| 88 | - | // (header + search entry ≈ 120px). If MaxVisibleRows worth of rows don't | 88 | + | // search entry, estimated as 2× rowHeight to scale with the UI). If |
| 89 | - | // fit, it falls back to SmallMaxVisibleRows. | 89 | + | // MaxVisibleRows worth of rows don't fit, it falls back to SmallMaxVisibleRows. |
| 90 | func EffectiveMaxRows(parentHeight, rowHeight int, sizeCfg ModalSizeConfig, defaults ListDisplayDefaults) int { | 90 | func EffectiveMaxRows(parentHeight, rowHeight int, sizeCfg ModalSizeConfig, defaults ListDisplayDefaults) int { | ||
| 91 | if parentHeight <= 0 || rowHeight <= 0 || defaults.SmallMaxVisibleRows <= 0 { | 91 | if parentHeight <= 0 || rowHeight <= 0 || defaults.SmallMaxVisibleRows <= 0 { | ||
| 92 | return defaults.MaxVisibleRows | 92 | return defaults.MaxVisibleRows | ||
| 93 | } | 93 | } | ||
| 94 | topMargin := int(float64(parentHeight) * sizeCfg.TopMarginPct) | 94 | topMargin := int(float64(parentHeight) * sizeCfg.TopMarginPct) | ||
| 95 | - | const chromeHeight = 150 // header tabs + search entry + bottom padding | 95 | + | chromeHeight := 2 * rowHeight // header tabs + search entry, scales with UI |
| 96 | available := parentHeight - topMargin - chromeHeight | 96 | available := parentHeight - topMargin - chromeHeight | ||
| 97 | needed := defaults.MaxVisibleRows * rowHeight | 97 | needed := defaults.MaxVisibleRows * rowHeight | ||
| 98 | if available < needed { | 98 | if available < needed { | ||
M internal/ui/component/modal_sizing_test.go +91 -0
| @@ -143,3 +143,94 @@ func TestListDisplayDefaults_Values(t *testing.T) { | |||
| 143 | 143 | assert.Positive(t, SessionManagerListDefaults.MaxVisibleRows) | |
| 144 | 144 | assert.Positive(t, SessionManagerListDefaults.MaxResults) | |
| 145 | 145 | } | |
| 146 | + | ||
| 147 | + | func TestEffectiveMaxRows(t *testing.T) { | |
| 148 | + | sizeCfg := ModalSizeConfig{TopMarginPct: 0.2} | |
| 149 | + | defaults := ListDisplayDefaults{ | |
| 150 | + | MaxVisibleRows: 10, | |
| 151 | + | MaxResults: 10, | |
| 152 | + | SmallMaxVisibleRows: 5, | |
| 153 | + | } | |
| 154 | + | ||
| 155 | + | tests := []struct { | |
| 156 | + | name string | |
| 157 | + | parentHeight int | |
| 158 | + | rowHeight int | |
| 159 | + | want int | |
| 160 | + | }{ | |
| 161 | + | { | |
| 162 | + | name: "tall pane returns MaxVisibleRows", | |
| 163 | + | parentHeight: 1200, | |
| 164 | + | rowHeight: 72, | |
| 165 | + | want: 10, | |
| 166 | + | }, | |
| 167 | + | { | |
| 168 | + | name: "short pane returns SmallMaxVisibleRows", | |
| 169 | + | parentHeight: 800, | |
| 170 | + | rowHeight: 72, | |
| 171 | + | want: 5, | |
| 172 | + | }, | |
| 173 | + | { | |
| 174 | + | name: "zero parentHeight returns MaxVisibleRows", | |
| 175 | + | parentHeight: 0, | |
| 176 | + | rowHeight: 72, | |
| 177 | + | want: 10, | |
| 178 | + | }, | |
| 179 | + | { | |
| 180 | + | name: "zero rowHeight returns MaxVisibleRows", | |
| 181 | + | parentHeight: 800, | |
| 182 | + | rowHeight: 0, | |
| 183 | + | want: 10, | |
| 184 | + | }, | |
| 185 | + | { | |
| 186 | + | name: "negative parentHeight returns MaxVisibleRows", | |
| 187 | + | parentHeight: -100, | |
| 188 | + | rowHeight: 72, | |
| 189 | + | want: 10, | |
| 190 | + | }, | |
| 191 | + | { | |
| 192 | + | name: "exact fit returns MaxVisibleRows", | |
| 193 | + | parentHeight: 1150, // available = 1150 - 230 - 144 = 776, needed = 720 | |
| 194 | + | rowHeight: 72, | |
| 195 | + | want: 10, | |
| 196 | + | }, | |
| 197 | + | { | |
| 198 | + | name: "boundary: available equals needed still fits", | |
| 199 | + | parentHeight: 1080, // available = 1080 - 216 - 144 = 720, needed = 720 → fits | |
| 200 | + | rowHeight: 72, | |
| 201 | + | want: 10, | |
| 202 | + | }, | |
| 203 | + | { | |
| 204 | + | name: "boundary: one pixel under triggers small", | |
| 205 | + | parentHeight: 1079, // available = 1079 - 215 - 144 = 720, needed = 720 → check | |
| 206 | + | rowHeight: 72, | |
| 207 | + | want: 10, // int(1079*0.2) = 215, 1079-215-144 = 720 → still fits | |
| 208 | + | }, | |
| 209 | + | { | |
| 210 | + | name: "just below boundary triggers small", | |
| 211 | + | parentHeight: 1078, // available = 1078 - 215 - 144 = 719, needed = 720 → doesn't fit | |
| 212 | + | rowHeight: 72, | |
| 213 | + | want: 5, | |
| 214 | + | }, | |
| 215 | + | } | |
| 216 | + | ||
| 217 | + | for _, tt := range tests { | |
| 218 | + | t.Run(tt.name, func(t *testing.T) { | |
| 219 | + | got := EffectiveMaxRows(tt.parentHeight, tt.rowHeight, sizeCfg, defaults) | |
| 220 | + | assert.Equal(t, tt.want, got) | |
| 221 | + | }) | |
| 222 | + | } | |
| 223 | + | } | |
| 224 | + | ||
| 225 | + | func TestEffectiveMaxRows_NoAdaptation(t *testing.T) { | |
| 226 | + | sizeCfg := ModalSizeConfig{TopMarginPct: 0.2} | |
| 227 | + | defaults := ListDisplayDefaults{ | |
| 228 | + | MaxVisibleRows: 10, | |
| 229 | + | MaxResults: 10, | |
| 230 | + | SmallMaxVisibleRows: 0, // adaptation disabled | |
| 231 | + | } | |
| 232 | + | ||
| 233 | + | // Even with a short pane, should return MaxVisibleRows when SmallMaxVisibleRows is 0 | |
| 234 | + | got := EffectiveMaxRows(500, 72, sizeCfg, defaults) | |
| 235 | + | assert.Equal(t, 10, got) | |
| 236 | + | } | |
| @@ -143,3 +143,94 @@ func TestListDisplayDefaults_Values(t *testing.T) { | |||||
| 143 | assert.Positive(t, SessionManagerListDefaults.MaxVisibleRows) | 143 | assert.Positive(t, SessionManagerListDefaults.MaxVisibleRows) | ||
| 144 | assert.Positive(t, SessionManagerListDefaults.MaxResults) | 144 | assert.Positive(t, SessionManagerListDefaults.MaxResults) | ||
| 145 | } | 145 | } | ||
| 146 | + | ||||
| 147 | + | func TestEffectiveMaxRows(t *testing.T) { | |||
| 148 | + | sizeCfg := ModalSizeConfig{TopMarginPct: 0.2} | |||
| 149 | + | defaults := ListDisplayDefaults{ | |||
| 150 | + | MaxVisibleRows: 10, | |||
| 151 | + | MaxResults: 10, | |||
| 152 | + | SmallMaxVisibleRows: 5, | |||
| 153 | + | } | |||
| 154 | + | ||||
| 155 | + | tests := []struct { | |||
| 156 | + | name string | |||
| 157 | + | parentHeight int | |||
| 158 | + | rowHeight int | |||
| 159 | + | want int | |||
| 160 | + | }{ | |||
| 161 | + | { | |||
| 162 | + | name: "tall pane returns MaxVisibleRows", | |||
| 163 | + | parentHeight: 1200, | |||
| 164 | + | rowHeight: 72, | |||
| 165 | + | want: 10, | |||
| 166 | + | }, | |||
| 167 | + | { | |||
| 168 | + | name: "short pane returns SmallMaxVisibleRows", | |||
| 169 | + | parentHeight: 800, | |||
| 170 | + | rowHeight: 72, | |||
| 171 | + | want: 5, | |||
| 172 | + | }, | |||
| 173 | + | { | |||
| 174 | + | name: "zero parentHeight returns MaxVisibleRows", | |||
| 175 | + | parentHeight: 0, | |||
| 176 | + | rowHeight: 72, | |||
| 177 | + | want: 10, | |||
| 178 | + | }, | |||
| 179 | + | { | |||
| 180 | + | name: "zero rowHeight returns MaxVisibleRows", | |||
| 181 | + | parentHeight: 800, | |||
| 182 | + | rowHeight: 0, | |||
| 183 | + | want: 10, | |||
| 184 | + | }, | |||
| 185 | + | { | |||
| 186 | + | name: "negative parentHeight returns MaxVisibleRows", | |||
| 187 | + | parentHeight: -100, | |||
| 188 | + | rowHeight: 72, | |||
| 189 | + | want: 10, | |||
| 190 | + | }, | |||
| 191 | + | { | |||
| 192 | + | name: "exact fit returns MaxVisibleRows", | |||
| 193 | + | parentHeight: 1150, // available = 1150 - 230 - 144 = 776, needed = 720 | |||
| 194 | + | rowHeight: 72, | |||
| 195 | + | want: 10, | |||
| 196 | + | }, | |||
| 197 | + | { | |||
| 198 | + | name: "boundary: available equals needed still fits", | |||
| 199 | + | parentHeight: 1080, // available = 1080 - 216 - 144 = 720, needed = 720 → fits | |||
| 200 | + | rowHeight: 72, | |||
| 201 | + | want: 10, | |||
| 202 | + | }, | |||
| 203 | + | { | |||
| 204 | + | name: "boundary: one pixel under triggers small", | |||
| 205 | + | parentHeight: 1079, // available = 1079 - 215 - 144 = 720, needed = 720 → check | |||
| 206 | + | rowHeight: 72, | |||
| 207 | + | want: 10, // int(1079*0.2) = 215, 1079-215-144 = 720 → still fits | |||
| 208 | + | }, | |||
| 209 | + | { | |||
| 210 | + | name: "just below boundary triggers small", | |||
| 211 | + | parentHeight: 1078, // available = 1078 - 215 - 144 = 719, needed = 720 → doesn't fit | |||
| 212 | + | rowHeight: 72, | |||
| 213 | + | want: 5, | |||
| 214 | + | }, | |||
| 215 | + | } | |||
| 216 | + | ||||
| 217 | + | for _, tt := range tests { | |||
| 218 | + | t.Run(tt.name, func(t *testing.T) { | |||
| 219 | + | got := EffectiveMaxRows(tt.parentHeight, tt.rowHeight, sizeCfg, defaults) | |||
| 220 | + | assert.Equal(t, tt.want, got) | |||
| 221 | + | }) | |||
| 222 | + | } | |||
| 223 | + | } | |||
| 224 | + | ||||
| 225 | + | func TestEffectiveMaxRows_NoAdaptation(t *testing.T) { | |||
| 226 | + | sizeCfg := ModalSizeConfig{TopMarginPct: 0.2} | |||
| 227 | + | defaults := ListDisplayDefaults{ | |||
| 228 | + | MaxVisibleRows: 10, | |||
| 229 | + | MaxResults: 10, | |||
| 230 | + | SmallMaxVisibleRows: 0, // adaptation disabled | |||
| 231 | + | } | |||
| 232 | + | ||||
| 233 | + | // Even with a short pane, should return MaxVisibleRows when SmallMaxVisibleRows is 0 | |||
| 234 | + | got := EffectiveMaxRows(500, 72, sizeCfg, defaults) | |||
| 235 | + | assert.Equal(t, 10, got) | |||
| 236 | + | } | |||
M internal/ui/component/omnibox.go +7 -2
| @@ -313,7 +313,7 @@ func (o *Omnibox) estimateRowHeight() int { | |||
| 313 | 313 | // Must be called on the GTK main thread. | |
| 314 | 314 | func (o *Omnibox) effectiveMaxRows() int { | |
| 315 | 315 | if o.parentOverlay == nil { | |
| 316 | - | return OmniboxListDefaults.MaxResults | |
| 316 | + | return OmniboxListDefaults.MaxVisibleRows | |
| 317 | 317 | } | |
| 318 | 318 | return EffectiveMaxRows(o.parentOverlay.GetAllocatedHeight(), o.estimateRowHeight(), OmniboxSizeDefaults, OmniboxListDefaults) | |
| 319 | 319 | } | |
| @@ -840,7 +840,12 @@ func (o *Omnibox) onEntryChanged() { | |||
| 840 | 840 | o.debounceTimer.Stop() | |
| 841 | 841 | } | |
| 842 | 842 | o.debounceTimer = time.AfterFunc(debounceDelayMs*time.Millisecond, func() { | |
| 843 | - | o.performSearch() | |
| 843 | + | // Schedule on GTK main thread — performSearch reads GTK widget state | |
| 844 | + | var cb glib.SourceFunc = func(uintptr) bool { | |
| 845 | + | o.performSearch() | |
| 846 | + | return false | |
| 847 | + | } | |
| 848 | + | glib.IdleAdd(&cb, 0) | |
| 844 | 849 | }) | |
| 845 | 850 | o.debounceMu.Unlock() | |
| 846 | 851 | } | |
| @@ -313,7 +313,7 @@ func (o *Omnibox) estimateRowHeight() int { | |||||
| 313 | // Must be called on the GTK main thread. | 313 | // Must be called on the GTK main thread. | ||
| 314 | func (o *Omnibox) effectiveMaxRows() int { | 314 | func (o *Omnibox) effectiveMaxRows() int { | ||
| 315 | if o.parentOverlay == nil { | 315 | if o.parentOverlay == nil { | ||
| 316 | - | return OmniboxListDefaults.MaxResults | 316 | + | return OmniboxListDefaults.MaxVisibleRows |
| 317 | } | 317 | } | ||
| 318 | return EffectiveMaxRows(o.parentOverlay.GetAllocatedHeight(), o.estimateRowHeight(), OmniboxSizeDefaults, OmniboxListDefaults) | 318 | return EffectiveMaxRows(o.parentOverlay.GetAllocatedHeight(), o.estimateRowHeight(), OmniboxSizeDefaults, OmniboxListDefaults) | ||
| 319 | } | 319 | } | ||
| @@ -840,7 +840,12 @@ func (o *Omnibox) onEntryChanged() { | |||||
| 840 | o.debounceTimer.Stop() | 840 | o.debounceTimer.Stop() | ||
| 841 | } | 841 | } | ||
| 842 | o.debounceTimer = time.AfterFunc(debounceDelayMs*time.Millisecond, func() { | 842 | o.debounceTimer = time.AfterFunc(debounceDelayMs*time.Millisecond, func() { | ||
| 843 | - | o.performSearch() | 843 | + | // Schedule on GTK main thread — performSearch reads GTK widget state |
| 844 | + | var cb glib.SourceFunc = func(uintptr) bool { | |||
| 845 | + | o.performSearch() | |||
| 846 | + | return false | |||
| 847 | + | } | |||
| 848 | + | glib.IdleAdd(&cb, 0) | |||
| 844 | }) | 849 | }) | ||
| 845 | o.debounceMu.Unlock() | 850 | o.debounceMu.Unlock() | ||
| 846 | } | 851 | } | ||