From 946e19ee41f3bb1a1224c36a3b75412d902b43a7 Mon Sep 17 00:00:00 2001 From: Kacper Paczos Date: Mon, 7 Sep 2026 23:12:04 +0200 Subject: [PATCH] feat(settings): make the language picker searchable while its list is open (#798) * feat(settings): make the language picker searchable while its list is open Entry completion only ever covered typing into the closed entry. Open the dropdown and the keyboard goes to GTK's own first-letter jump, so nothing typed reaches a filter, and with thirty-odd languages scrolling is the only way through. The first attempt at fixing this (a filtered ComboBox model) had the same limitation for the same reason. Replace the combo with a picker that opens a popover holding a search entry, focused on open, above the list. Every keystroke narrows the rows, Enter takes the first match, a click takes that row. It keeps the GtkComboBoxText methods the dialog calls, mirrors the store's column order in base_model for the shared row helpers, and emits "changed" only when the selection actually changes, so re-selecting the saved language on open is not taken for a user edit. Visibility uses the matcher the typed-text resolver already uses, so the list and Enter cannot disagree. The completion helper and the combo styling skip the picker: it filters its own list, and the ComboBox-only calls do not exist on a Box. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01TKVa2qTLQYMCHffjp97yqT * fix(settings): empty Enter must not auto-apply first language Opening the searchable language picker clears the filter, so every row is visible. Enter then took the first store row via _first_visible_row, emitted changed, and auto-applied a language the user never chose. Skip first-match activation when the needle is empty (or whitespace); Enter only selects after a non-empty filter. Add regression coverage and type the SearchablePicker surface (Greptile P2). * fix(settings): finish SearchablePicker annotations Greptile P2 stayed open after the empty-Enter fix: handler params, get_child and _first_visible_row returns, and the ComboBoxText-compat methods still lacked docs. Annotate the remaining signatures, add one-line public docs, and type the test helpers. Behavior unchanged. --------- Co-authored-by: Kacper Paczos <23406555+kacperpaczos@users.noreply.github.com> Co-authored-by: Claude Fable 5.1 Co-authored-by: Jatin K Malik --- src/vocalinux/ui/settings_dialog.py | 186 ++++++++++++++++++++- tests/test_language_list_filtering.py | 228 ++++++++++++++++++++++++++ tests/test_settings_dialog.py | 6 +- 3 files changed, 414 insertions(+), 6 deletions(-) create mode 100644 tests/test_language_list_filtering.py diff --git a/src/vocalinux/ui/settings_dialog.py b/src/vocalinux/ui/settings_dialog.py index d3a485f..e639c0e 100644 --- a/src/vocalinux/ui/settings_dialog.py +++ b/src/vocalinux/ui/settings_dialog.py @@ -30,7 +30,7 @@ import gi gi.require_version("Gtk", "3.0") gi.require_version("Gdk", "3.0") # Need GLib for idle_add -from gi.repository import Gdk, GLib, Gtk, Pango # noqa: E402 +from gi.repository import Gdk, GLib, GObject, Gtk, Pango # noqa: E402 from ..common_types import RecognitionState # noqa: E402 from ..speech_recognition.silero_vad import is_silero_available # noqa: E402 @@ -333,6 +333,8 @@ def _style_combo(combo: Gtk.ComboBox, width: int = _CONTROL_WIDTH) -> Gtk.ComboB combo.set_size_request(width, -1) combo.set_halign(Gtk.Align.END) combo.set_hexpand(False) + if not isinstance(combo, Gtk.ComboBox): + return combo # a SearchablePicker sizes its own button combo.set_popup_fixed_width(False) for cell in combo.get_cells(): cell.set_property("ellipsize", Pango.EllipsizeMode.END) @@ -1048,7 +1050,9 @@ def _combo_text_rows(combo: Gtk.ComboBoxText) -> list: Gtk.ComboBoxText stores display text in column 0 and the id in column 1. """ - model = combo.get_model() + # A SearchablePicker keeps its rows in base_model (ComboBoxText column order); + # resolve typed text against every row, not only the ones currently shown. + model = getattr(combo, "base_model", None) or combo.get_model() if not model: return [] return [(row[1], row[0]) for row in model] @@ -1063,14 +1067,188 @@ def _combo_completion_match(completion, key, tree_iter, *_args) -> bool: return _combo_text_matches_query(key, row[1], row[0]) +class SearchablePicker(Gtk.Box): + """A picker you can type into while its list is open. + + A GtkComboBox dropdown takes the keyboard for its own first-letter jump the + moment it opens, so nothing typed reaches a filter. This one opens a popover + holding a search entry, focused on open, above the list; every keystroke + narrows the rows, Enter takes the first match after a non-empty filter, and + a click takes that row. Empty Enter is a no-op so it cannot auto-apply the + first store row. + + Keeps the handful of GtkComboBoxText methods the dialog relies on, so it + drops in where one stood. ``base_model`` mirrors the ComboBoxText store — + display text in column 0, id in column 1 — for the shared row helpers. + """ + + __gsignals__ = {"changed": (GObject.SignalFlags.RUN_FIRST, None, ())} + + _LIST_HEIGHT = 300 + + def __init__(self) -> None: + super().__init__(orientation=Gtk.Orientation.HORIZONTAL, spacing=0) + self.base_model: list[list] = [] + self._active_id: Optional[str] = None + self._rows_by_id: dict = {} + + self._label = Gtk.Label(xalign=0) + self._label.set_ellipsize(Pango.EllipsizeMode.END) + arrow = Gtk.Image.new_from_icon_name("pan-down-symbolic", Gtk.IconSize.BUTTON) + button_box = Gtk.Box(orientation=Gtk.Orientation.HORIZONTAL, spacing=6) + button_box.pack_start(self._label, True, True, 0) + button_box.pack_end(arrow, False, False, 0) + self._button = Gtk.Button() + self._button.add(button_box) + self._button.connect("clicked", self._on_button_clicked) + self.pack_start(self._button, True, True, 0) + + self._search = Gtk.SearchEntry() + self._search.set_placeholder_text("Type to search…") + self._search.connect("search-changed", self._on_search_changed) + self._search.connect("activate", self._on_search_activate) + + self._list = Gtk.ListBox() + self._list.set_selection_mode(Gtk.SelectionMode.NONE) + self._list.set_filter_func(self._row_is_visible) + self._list.connect("row-activated", self._on_row_activated) + + scroller = Gtk.ScrolledWindow() + scroller.set_policy(Gtk.PolicyType.NEVER, Gtk.PolicyType.AUTOMATIC) + scroller.set_size_request(-1, self._LIST_HEIGHT) + scroller.add(self._list) + + content = Gtk.Box(orientation=Gtk.Orientation.VERTICAL, spacing=6) + content.set_border_width(6) + content.pack_start(self._search, False, False, 0) + content.pack_start(scroller, True, True, 0) + content.show_all() + + self._popover = Gtk.Popover.new(self._button) + self._popover.set_position(Gtk.PositionType.BOTTOM) + self._popover.add(content) + self._popover.connect("closed", self._on_popover_closed) + + # -- GtkComboBoxText-compatible surface -------------------------------- + + def append(self, item_id: Optional[str], text: str) -> None: + """Add a row, taking the arguments in GtkComboBoxText order.""" + self.base_model.append([text, item_id]) + row = Gtk.ListBoxRow() + label = Gtk.Label(label=text, xalign=0) + label.set_margin_top(6) + label.set_margin_bottom(6) + label.set_margin_start(8) + label.set_margin_end(8) + row.add(label) + row.item_id = item_id + row.item_text = text + row.show_all() + self._list.add(row) + self._rows_by_id[item_id] = row + + def remove_all(self) -> None: + """Drop every row and clear the active selection.""" + self.base_model.clear() + self._rows_by_id.clear() + for row in list(self._list.get_children()): + self._list.remove(row) + self._active_id = None + self._label.set_text("") + + def get_model(self) -> list: + """The ComboBoxText-shaped store: [display text, id] per row.""" + return self.base_model + + def get_active_id(self) -> Optional[str]: + """The id of the selected row, or None.""" + return self._active_id + + def get_active_text(self) -> Optional[str]: + """The display text of the selected row, or None.""" + row = self._rows_by_id.get(self._active_id) + return row.item_text if row is not None else None + + def set_active_id(self, item_id: Optional[str]) -> bool: + """Select by id; False if that id is not in the list.""" + row = self._rows_by_id.get(item_id) + if row is None: + return False + self._select(item_id, row.item_text) + return True + + def set_active(self, index: int) -> None: + """Select by store index; no-op if the index is out of range.""" + if 0 <= index < len(self.base_model): + text, item_id = self.base_model[index] + self._select(item_id, text) + + def get_child(self) -> Gtk.SearchEntry: + """The search entry, for callers that hook a ComboBoxText's entry.""" + return self._search + + # -- behaviour --------------------------------------------------------- + + def _select(self, item_id: Optional[str], text: Optional[str]) -> None: + changed = item_id != self._active_id + self._active_id = item_id + self._label.set_text(text or "") + if changed: + self.emit("changed") + + def _on_button_clicked(self, _button: Gtk.Button) -> None: + self._search.set_text("") + self._list.invalidate_filter() + self._popover.show_all() + self._popover.popup() + self._search.grab_focus() + + def _on_popover_closed(self, _popover: Gtk.Popover) -> None: + self._search.set_text("") + self._list.invalidate_filter() + + def _row_is_visible(self, row: Gtk.ListBoxRow) -> bool: + needle = (self._search.get_text() or "").strip() + if not needle: + return True + # Same rule the typed-text resolver uses, so the list and Enter agree. + return _combo_text_matches_query(needle, row.item_id, row.item_text) + + def _on_search_changed(self, _entry: Gtk.SearchEntry) -> None: + self._list.invalidate_filter() + + def _first_visible_row(self) -> Optional[Gtk.ListBoxRow]: + for row in self._list.get_children(): + if row.get_visible() and row.get_child_visible() and self._row_is_visible(row): + return row + return None + + def _on_search_activate(self, _entry: Gtk.SearchEntry) -> None: + # Empty (or whitespace-only) Enter must not grab the first store row and + # auto-apply a language the user never chose. First-match only after a + # non-empty filter. + needle = (self._search.get_text() or "").strip() + if not needle: + return + row = self._first_visible_row() + if row is not None: + self._on_row_activated(self._list, row) + + def _on_row_activated(self, _listbox: Gtk.ListBox, row: Gtk.ListBoxRow) -> None: + self._popover.popdown() + self._select(row.item_id, row.item_text) + + def _attach_language_combo_search(combo: Gtk.ComboBoxText) -> None: """Filter language choices as the user types in a ComboBoxText entry.""" + if isinstance(combo, SearchablePicker): + return # the picker filters its own list; a completion popup would fight it entry = combo.get_child() if entry is None: return entry.set_placeholder_text("Search languages…") completion = Gtk.EntryCompletion() - completion.set_model(combo.get_model()) + completion.set_model(getattr(combo, "base_model", None) or combo.get_model()) # ComboBoxText store: column 0 is display text, column 1 is id. completion.set_text_column(0) completion.set_inline_completion(False) @@ -2753,7 +2931,7 @@ class SettingsDialog(Gtk.Dialog): group.add_row(self.model_variant_row) # Language selection (searchable: type to filter the 30+ language list) - self.language_combo = Gtk.ComboBoxText.new_with_entry() + self.language_combo = SearchablePicker() _style_combo(self.language_combo) self.language_combo.set_tooltip_text(LANGUAGE_TOOLTIP) _prevent_scroll_on_hover(self.language_combo) diff --git a/tests/test_language_list_filtering.py b/tests/test_language_list_filtering.py new file mode 100644 index 0000000..b0293c0 --- /dev/null +++ b/tests/test_language_list_filtering.py @@ -0,0 +1,228 @@ +"""The language list must be searchable while it is open. + +A GtkComboBox dropdown takes the keyboard for its own first-letter jump the +moment it opens, so nothing typed reaches a filter. The picker opens a popover +with a search entry above the list instead; every keystroke narrows the rows. +""" + +import importlib +import sys +from typing import Any, Optional +from unittest.mock import MagicMock, Mock, patch + +import pytest + +import vocalinux.ui + + +@pytest.fixture(scope="module") +def settings_dialog(): + """Import settings_dialog with real base classes for its GTK subclasses. + + Box is already among the bases the module needs; SearchablePicker subclasses + it. Both sys.modules and the package attribute are restored so later tests + patching the module by name do not reach a second module object (#686). + """ + repository = sys.modules["gi.repository"] + bases = {name: type(name, (), {}) for name in ("Box", "ListBoxRow", "Dialog", "ComboBox")} + saved_module = sys.modules.pop("vocalinux.ui.settings_dialog", None) + saved_attribute = getattr(vocalinux.ui, "settings_dialog", None) + try: + with patch.object(repository, "Gtk", MagicMock(**bases)): + module = importlib.import_module("vocalinux.ui.settings_dialog") + yield module + finally: + sys.modules.pop("vocalinux.ui.settings_dialog", None) + if saved_module is not None: + sys.modules["vocalinux.ui.settings_dialog"] = saved_module + if saved_attribute is not None: + vocalinux.ui.settings_dialog = saved_attribute + elif hasattr(vocalinux.ui, "settings_dialog"): + del vocalinux.ui.settings_dialog + + +def _row(item_id: str, text: str) -> Mock: + row = Mock() + row.item_id = item_id + row.item_text = text + return row + + +def _picker_stub( + settings_dialog: Any, typed: str = "", rows: tuple = (), active: Optional[str] = None +) -> Mock: + """A stand-in ``self``: GTK is mocked, the picker's own logic is not.""" + picker = Mock() + picker.base_model = [[text, item_id] for item_id, text in rows] + picker._rows_by_id = {item_id: _row(item_id, text) for item_id, text in rows} + picker._active_id = active + picker._search.get_text.return_value = typed + picker._list.get_children.return_value = list(picker._rows_by_id.values()) + # These are the picker's own logic, not collaborators, so they stay real: + # _select decides whether "changed" fires, _row_is_visible is the filter. + picker._select.side_effect = lambda item_id, text: settings_dialog.SearchablePicker._select( + picker, item_id, text + ) + picker._row_is_visible.side_effect = ( + lambda row: settings_dialog.SearchablePicker._row_is_visible(picker, row) + ) + return picker + + +LANGS = (("en-us", "English (US)"), ("pl", "Polish"), ("ja", "Japanese")) + + +@pytest.mark.parametrize( + "typed,expected", + [ + ("pol", ["Polish"]), + ("POL", ["Polish"]), + ("ja", ["Japanese"]), + ("en", ["English (US)"]), + ("", ["English (US)", "Polish", "Japanese"]), + ("zzz", []), + ], +) +def test_typing_narrows_the_list(settings_dialog, typed, expected): + picker = _picker_stub(settings_dialog, typed=typed, rows=LANGS) + shown = [ + row.item_text + for row in picker._rows_by_id.values() + if settings_dialog.SearchablePicker._row_is_visible(picker, row) + ] + assert shown == expected + + +def test_enter_takes_the_first_row_still_showing(settings_dialog): + """Typing then Enter must not require reaching for the mouse.""" + picker = _picker_stub(settings_dialog, typed="pol", rows=LANGS) + for row in picker._list.get_children.return_value: + row.get_visible.return_value = True + row.get_child_visible.return_value = True + picker._first_visible_row.side_effect = ( + lambda: settings_dialog.SearchablePicker._first_visible_row(picker) + ) + + settings_dialog.SearchablePicker._on_search_activate(picker, picker._search) + + picker._on_row_activated.assert_called_once() + assert picker._on_row_activated.call_args[0][1].item_id == "pl" + + +def test_enter_with_nothing_matching_selects_nothing(settings_dialog): + picker = _picker_stub(settings_dialog, typed="zzz", rows=LANGS) + for row in picker._list.get_children.return_value: + row.get_visible.return_value = True + row.get_child_visible.return_value = True + picker._first_visible_row.side_effect = ( + lambda: settings_dialog.SearchablePicker._first_visible_row(picker) + ) + + settings_dialog.SearchablePicker._on_search_activate(picker, picker._search) + + picker._on_row_activated.assert_not_called() + + +def test_enter_with_empty_search_selects_nothing(settings_dialog): + """Open picker (search cleared); bare Enter must keep the current language. + + Every row is visible when the needle is empty, so first-visible would be the + first store row and would auto-apply a language the user never chose. + """ + picker = _picker_stub(settings_dialog, typed="", rows=LANGS, active="pl") + for row in picker._list.get_children.return_value: + row.get_visible.return_value = True + row.get_child_visible.return_value = True + picker._first_visible_row.side_effect = ( + lambda: settings_dialog.SearchablePicker._first_visible_row(picker) + ) + + settings_dialog.SearchablePicker._on_search_activate(picker, picker._search) + + picker._on_row_activated.assert_not_called() + picker._first_visible_row.assert_not_called() + picker.emit.assert_not_called() + assert picker._active_id == "pl" + + +def test_enter_with_whitespace_search_selects_nothing(settings_dialog): + """Whitespace-only is still an empty filter after strip.""" + picker = _picker_stub(settings_dialog, typed=" ", rows=LANGS, active="ja") + for row in picker._list.get_children.return_value: + row.get_visible.return_value = True + row.get_child_visible.return_value = True + picker._first_visible_row.side_effect = ( + lambda: settings_dialog.SearchablePicker._first_visible_row(picker) + ) + + settings_dialog.SearchablePicker._on_search_activate(picker, picker._search) + + picker._on_row_activated.assert_not_called() + assert picker._active_id == "ja" + + +def test_selecting_by_id_reports_whether_the_row_exists(settings_dialog): + """_set_combo_active_id_or_first relies on the boolean to fall back.""" + picker = _picker_stub(settings_dialog, rows=LANGS) + + assert settings_dialog.SearchablePicker.set_active_id(picker, "pl") is True + assert settings_dialog.SearchablePicker.set_active_id(picker, "xx") is False + + +def test_changed_fires_only_when_the_selection_actually_changes(settings_dialog): + """Re-selecting the saved language on open must not look like a user edit.""" + picker = _picker_stub(settings_dialog, rows=LANGS, active="pl") + + settings_dialog.SearchablePicker._select(picker, "pl", "Polish") + picker.emit.assert_not_called() + + settings_dialog.SearchablePicker._select(picker, "ja", "Japanese") + picker.emit.assert_called_once_with("changed") + assert picker._active_id == "ja" + + +def test_opening_starts_from_an_empty_search(settings_dialog): + """Leftover text from last time would hide most of the list on open.""" + picker = _picker_stub(settings_dialog, rows=LANGS) + + settings_dialog.SearchablePicker._on_button_clicked(picker, None) + + picker._search.set_text.assert_called_once_with("") + picker._search.grab_focus.assert_called_once() + + +def test_the_row_helpers_read_the_picker_store(settings_dialog): + """Resolving typed text must see every language, not only the shown ones.""" + picker = _picker_stub(settings_dialog, rows=LANGS) + picker.get_model.return_value = [] # would be wrong to consult + + assert settings_dialog._combo_text_rows(picker) == [ + ("en-us", "English (US)"), + ("pl", "Polish"), + ("ja", "Japanese"), + ] + + +def test_completion_is_not_bolted_onto_the_picker(settings_dialog): + """Its own filter and a completion popup would fight over the same entry.""" + # A plain Mock wearing the picker's class: isinstance() says picker, and the + # Box methods GTK would provide are auto-created rather than spec-blocked. + picker = Mock() + picker.__class__ = settings_dialog.SearchablePicker + + with patch.object(settings_dialog.Gtk, "EntryCompletion") as completion: + settings_dialog._attach_language_combo_search(picker) + + completion.assert_not_called() + + +def test_combo_styling_skips_combo_only_calls_on_the_picker(settings_dialog): + """get_cells and set_popup_fixed_width do not exist on a Box.""" + picker = Mock() + picker.__class__ = settings_dialog.SearchablePicker + + settings_dialog._style_combo(picker) + + picker.set_size_request.assert_called_once() + picker.set_popup_fixed_width.assert_not_called() + picker.get_cells.assert_not_called() diff --git a/tests/test_settings_dialog.py b/tests/test_settings_dialog.py index c1b3cae..d67d3c6 100644 --- a/tests/test_settings_dialog.py +++ b/tests/test_settings_dialog.py @@ -683,7 +683,9 @@ class TestSettingsDialogHelperFunctions(unittest.TestCase): source_code, ) self.assertIn("self.language_row.set_tooltip_text(LANGUAGE_TOOLTIP)", source_code) - self.assertIn("Gtk.ComboBoxText.new_with_entry()", source_code) + self.assertIn("self.language_combo = SearchablePicker()", source_code) + # SearchablePicker opens a popover with a search entry above the list, + # so typing works while the list is open, which no combo dropdown allows. self.assertIn("_attach_language_combo_search", source_code) def test_whispercpp_recommendation_uses_language_for_specialization(self): @@ -820,7 +822,7 @@ class TestLanguageComboSearch(unittest.TestCase): with open(source_path, "r") as f: source_code = f.read() - self.assertIn("Gtk.ComboBoxText.new_with_entry()", source_code) + self.assertIn("self.language_combo = SearchablePicker()", source_code) self.assertIn("Gtk.EntryCompletion()", source_code) self.assertIn("completion.set_text_column(0)", source_code) self.assertIn("Search languages…", source_code)