From f770a4e09302364eb5e1b8c889c629d708f3b782 Mon Sep 17 00:00:00 2001 From: Ed_ Date: Sat, 21 Mar 2026 10:55:29 -0400 Subject: [PATCH] fix(gui): Implement correct UX for discussion takes tabs and combo box --- src/app_controller.py | 7 +++--- src/gui_2.py | 36 +++++++++++++++++++++++++------ tests/test_gui_discussion_tabs.py | 24 ++++++++++++--------- 3 files changed, 46 insertions(+), 21 deletions(-) diff --git a/src/app_controller.py b/src/app_controller.py index 0e8a4cd..63d8407 100644 --- a/src/app_controller.py +++ b/src/app_controller.py @@ -2194,18 +2194,17 @@ class AppController: def _branch_discussion(self, index: int) -> None: self._flush_disc_entries_to_project() # Generate a unique branch name - base_name = f"{self.active_discussion}_take" + base_name = self.active_discussion.split("_take_")[0] counter = 1 - new_name = f"{base_name}_{counter}" + new_name = f"{base_name}_take_{counter}" disc_sec = self.project.get("discussion", {}) discussions = disc_sec.get("discussions", {}) while new_name in discussions: counter += 1 - new_name = f"{base_name}_{counter}" + new_name = f"{base_name}_take_{counter}" project_manager.branch_discussion(self.project, self.active_discussion, new_name, index) self._switch_discussion(new_name) - def _rename_discussion(self, old_name: str, new_name: str) -> None: disc_sec = self.project.get("discussion", {}) discussions = disc_sec.get("discussions", {}) diff --git a/src/gui_2.py b/src/gui_2.py index 4affac0..7753cc8 100644 --- a/src/gui_2.py +++ b/src/gui_2.py @@ -2309,17 +2309,39 @@ def hello(): return if not self.is_viewing_prior_session and imgui.collapsing_header("Discussions", imgui.TreeNodeFlags_.default_open): names = self._get_discussion_names() + grouped_discussions = {} + for name in names: + base = name.split("_take_")[0] + grouped_discussions.setdefault(base, []).append(name) + + active_base = self.active_discussion.split("_take_")[0] + if active_base not in grouped_discussions: + active_base = names[0] if names else "" + + base_names = sorted(grouped_discussions.keys()) + if imgui.begin_combo("##disc_sel", active_base): + for bname in base_names: + is_selected = (bname == active_base) + if imgui.selectable(bname, is_selected)[0]: + target = bname if bname in names else grouped_discussions[bname][0] + if target != self.active_discussion: + self._switch_discussion(target) + if is_selected: + imgui.set_item_default_focus() + imgui.end_combo() + + current_takes = grouped_discussions.get(active_base, []) if imgui.begin_tab_bar("discussion_takes_tabs"): - for name in names: - # Only force selection if active_discussion changed externally - flags = imgui.TabItemFlags_.set_selected if name == self.active_discussion else 0 - opened, _ = imgui.begin_tab_item(name, None, flags) + for take_name in current_takes: + label = "Original" if take_name == active_base else take_name.replace(f"{active_base}_", "").replace("_", " ").title() + flags = imgui.TabItemFlags_.set_selected if take_name == self.active_discussion else 0 + opened, _ = imgui.begin_tab_item(f"{label}###{take_name}", None, flags) if opened: - if name != self.active_discussion: - self._switch_discussion(name) + if take_name != self.active_discussion: + self._switch_discussion(take_name) imgui.end_tab_item() - if imgui.begin_tab_item("Synthesis")[0]: + if imgui.begin_tab_item("Synthesis###Synthesis"): self._render_synthesis_panel() imgui.end_tab_item() diff --git a/tests/test_gui_discussion_tabs.py b/tests/test_gui_discussion_tabs.py index 6c8f41f..bbf9c97 100644 --- a/tests/test_gui_discussion_tabs.py +++ b/tests/test_gui_discussion_tabs.py @@ -11,22 +11,26 @@ def mock_gui(): 'active': 'main', 'discussions': { 'main': {'history': []}, - 'take_1': {'history': []}, - 'take_2': {'history': []} + 'main_take_1': {'history': []}, + 'other_topic': {'history': []} } } } gui.active_discussion = 'main' - # Mock some required state gui.perf_profiling_enabled = False gui.is_viewing_prior_session = False - gui._get_discussion_names = lambda: ['main', 'take_1', 'take_2'] + gui._get_discussion_names = lambda: ['main', 'main_take_1', 'other_topic'] return gui def test_discussion_tabs_rendered(mock_gui): with patch('src.gui_2.imgui') as mock_imgui, \ patch('src.app_controller.AppController.active_project_root', new_callable=PropertyMock, return_value='.'): - # We expect a tab bar to be used instead of a combo box + + # We expect a combo box for base discussion + mock_imgui.begin_combo.return_value = True + mock_imgui.selectable.return_value = (False, False) + + # We expect a tab bar for takes mock_imgui.begin_tab_bar.return_value = True mock_imgui.begin_tab_item.return_value = (True, True) mock_imgui.input_text.return_value = (False, "") @@ -34,16 +38,16 @@ def test_discussion_tabs_rendered(mock_gui): mock_imgui.checkbox.return_value = (False, False) mock_imgui.input_int.return_value = (False, 0) - # Prevent infinite loop in ListClipper mock_clipper = MagicMock() mock_clipper.step.return_value = False mock_imgui.ListClipper.return_value = mock_clipper mock_gui._render_discussion_panel() + mock_imgui.begin_combo.assert_called_once_with("##disc_sel", 'main') mock_imgui.begin_tab_bar.assert_called_once_with('discussion_takes_tabs') - # Check that begin_tab_item was called for each take + calls = [c[0][0] for c in mock_imgui.begin_tab_item.call_args_list] - assert 'main' in calls - assert 'take_1' in calls - assert 'take_2' in calls + assert 'Original###main' in calls + assert 'Take 1###main_take_1' in calls + assert 'Synthesis###Synthesis' in calls