diff --git a/src/vocalinux/ui/settings_dialog.py b/src/vocalinux/ui/settings_dialog.py index a0ab08a..49d224d 100644 --- a/src/vocalinux/ui/settings_dialog.py +++ b/src/vocalinux/ui/settings_dialog.py @@ -147,6 +147,15 @@ ENGINE_MODELS = { "remote_api": [], # Remote API does not need local models } +# Tallest the "Unused downloads" list may grow before it starts scrolling +_UNUSED_DOWNLOADS_MAX_HEIGHT = 220 + + +def _clamp_unused_downloads_height(natural_height: int) -> int: + """Clamp a measured list height to the height the page gives the list.""" + return min(_UNUSED_DOWNLOADS_MAX_HEIGHT, natural_height) + + # Engine display name mapping ENGINE_DISPLAY_NAMES = { "vosk": "Vosk", @@ -2498,7 +2507,10 @@ class SettingsDialog(Gtk.Dialog): self.unused_models_scroll.set_policy(Gtk.PolicyType.NEVER, Gtk.PolicyType.AUTOMATIC) self.unused_models_scroll.set_shadow_type(Gtk.ShadowType.NONE) self.unused_models_scroll.set_propagate_natural_height(True) - self.unused_models_scroll.set_max_content_height(220) + self.unused_models_scroll.set_max_content_height(_UNUSED_DOWNLOADS_MAX_HEIGHT) + # Keep the scrollbar in the layout: an overlay one stays invisible until + # hovered, so a list clipped by the cap looks like it has no more rows. + self.unused_models_scroll.set_overlay_scrolling(False) # ListBox is GtkScrollable, so wrapping it in a Box forces a Viewport # and lets the expander child report a real height. @@ -2507,6 +2519,12 @@ class SettingsDialog(Gtk.Dialog): list_holder.pack_start(self.unused_models_group.listbox, False, False, 0) self.unused_models_scroll.add(list_holder) self.unused_expander.add(self.unused_models_scroll) + # Backstop: the refresh measures while the expander is collapsed and + # the dialog may not be mapped yet. Remeasure when the user expands, + # so a short first measurement can never leave the list clipped. + self.unused_expander.connect( + "notify::expanded", lambda *_args: self._fit_unused_downloads_height() + ) self.unused_models_group.pack_start(self.unused_expander, False, False, 0) self.content_box.pack_start(self.unused_models_group, False, False, 0) @@ -4610,7 +4628,6 @@ class SettingsDialog(Gtk.Dialog): count = len(unused) leftover = "leftover model" if count == 1 else "leftover models" self.unused_expander.set_label(f"Unused downloads ({count} {leftover})") - self.unused_models_scroll.set_min_content_height(min(220, 56 * count)) was_expanded = self.unused_expander.get_expanded() for model_id, title, size_label in unused: @@ -4633,6 +4650,20 @@ class SettingsDialog(Gtk.Dialog): self.unused_models_group.show_all() self.unused_expander.set_expanded(was_expanded) + self._fit_unused_downloads_height() + + def _fit_unused_downloads_height(self): + """Size the list from the rows it actually holds. + + A per-row estimate cannot know how tall a row renders: with margins and + a Delete button the rows are taller than the estimate, so the last one + was cut off below the edge of the viewport while the header still + counted it (#683). + """ + _, natural_height = self.unused_models_group.listbox.get_preferred_height() + self.unused_models_scroll.set_min_content_height( + _clamp_unused_downloads_height(natural_height) + ) def _confirm_model_delete(self, text: str, secondary: str) -> bool: """Ask before deleting model files from disk.""" diff --git a/tests/test_unused_downloads_height.py b/tests/test_unused_downloads_height.py new file mode 100644 index 0000000..8fc4496 --- /dev/null +++ b/tests/test_unused_downloads_height.py @@ -0,0 +1,72 @@ +"""Tests for sizing the "Unused downloads" list to the rows it holds. + +The settings dialog cannot be instantiated under the mocked-GTK harness (see +test_settings_mode_change.py), so the arithmetic lives in a module-level +helper that is unit-tested directly, and the wiring around it is asserted +against the source of the two methods. +""" + +import inspect +import re + +from vocalinux.ui import settings_dialog +from vocalinux.ui.settings_dialog import ( + _UNUSED_DOWNLOADS_MAX_HEIGHT, + _clamp_unused_downloads_height, +) + + +def _method_source(name: str) -> str: + src = inspect.getsource(settings_dialog) + match = re.search(rf"\n def {name}\(.*?(?=\n def )", src, re.DOTALL) + assert match, f"could not locate method {name}" + return match.group(0) + + +def test_list_is_as_tall_as_its_rows_render(): + """Two rows that measure 116px must not be squeezed into an estimate. + + 56px per row was the old guess; real rows render taller than that once + margins and the Delete button are counted, which cut off the last row + while the header still counted it (#683). + """ + assert _clamp_unused_downloads_height(116) == 116 + + +def test_long_lists_stop_growing_at_the_cap(): + """Beyond the cap the list scrolls instead of pushing the page down.""" + assert _clamp_unused_downloads_height(_UNUSED_DOWNLOADS_MAX_HEIGHT * 3) == ( + _UNUSED_DOWNLOADS_MAX_HEIGHT + ) + + +def test_refresh_sizes_the_list_after_building_the_rows(): + """A measured height is worthless if the refresh never asks for one.""" + body = _method_source("_refresh_unused_downloads") + assert "self._fit_unused_downloads_height()" in body + # The per-row estimate is what clipped the list; it must not come back. + assert "56 *" not in body + + +def test_fit_measures_the_listbox_and_feeds_the_scroll(): + body = _method_source("_fit_unused_downloads_height") + assert "self.unused_models_group.listbox.get_preferred_height()" in body + assert "self.unused_models_scroll.set_min_content_height(" in body + assert "_clamp_unused_downloads_height(natural_height)" in body + + +def test_scrollbar_stays_in_the_layout(): + """An overlay scrollbar hides until hovered, so a capped list looks whole.""" + src = inspect.getsource(settings_dialog) + assert "self.unused_models_scroll.set_overlay_scrolling(False)" in src + + +def test_expanding_the_list_remeasures_it(): + """The refresh measures while collapsed and possibly unmapped; expanding + must remeasure so a short first measurement can never leave rows clipped.""" + src = inspect.getsource(settings_dialog) + assert re.search( + r"self\.unused_expander\.connect\(\s*\"notify::expanded\"," + r"\s*lambda \*_args: self\._fit_unused_downloads_height\(\)", + src, + )