From 881da59786ca064ce1fe6d4d257cdcccadd5b2b6 Mon Sep 17 00:00:00 2001 From: wyzula-jan Date: Thu, 18 Jun 2026 16:24:15 +0200 Subject: [PATCH] fix(actions): review adjustments --- bec_widgets/utils/toolbars/actions.py | 1 + .../toolbar_components/mouse_interactions.py | 55 ++++++++++--------- tests/unit_tests/test_modular_toolbar.py | 26 +++++++++ tests/unit_tests/test_plot_base_next_gen.py | 55 +++++++++++++++++++ 4 files changed, 110 insertions(+), 27 deletions(-) diff --git a/bec_widgets/utils/toolbars/actions.py b/bec_widgets/utils/toolbars/actions.py index 7d98d70a..2d692732 100644 --- a/bec_widgets/utils/toolbars/actions.py +++ b/bec_widgets/utils/toolbars/actions.py @@ -363,6 +363,7 @@ class SwitchableToolBarAction(IconAction): self.main_button.setToolTip(default_action.tooltip or "") self.main_button.clicked.connect(self._trigger_current_action) menu = QMenu(self.main_button) + self.menu_actions = {} for key, action_obj in self.actions.items(): menu_action = QAction( icon=action_obj.get_icon(), text=action_obj.tooltip, parent=self.main_button diff --git a/bec_widgets/widgets/plots/toolbar_components/mouse_interactions.py b/bec_widgets/widgets/plots/toolbar_components/mouse_interactions.py index 6fdab36d..11f7a0db 100644 --- a/bec_widgets/widgets/plots/toolbar_components/mouse_interactions.py +++ b/bec_widgets/widgets/plots/toolbar_components/mouse_interactions.py @@ -79,7 +79,6 @@ class MouseInteractionConnection(BundleConnection): self.bundle_name = "mouse_interaction" self.components = components self.target_widget = target_widget - self.mouse_mode = None if ( not hasattr(self.target_widget, "plot_item") or not hasattr(self.target_widget, "auto_range_x") @@ -114,50 +113,52 @@ class MouseInteractionConnection(BundleConnection): def get_viewbox_mode(self): """ - Returns the current interaction mode of a PyQtGraph ViewBox and sets the corresponding action. + Synchronise the toolbar selection with the plot's current mouse interaction mode. """ - if self.target_widget: - viewbox = self.target_widget.plot_item.getViewBox() - switch_mouse_action = self.components.get_action_reference("switch_mouse_mode")() - if viewbox.getState()["mouseMode"] == 3: - switch_mouse_action.set_default_action("drag_mode") - switch_mouse_action.main_button.setChecked(True) - self.mouse_mode = "PanMode" - elif viewbox.getState()["mouseMode"] == 1: - switch_mouse_action.set_default_action("rectangle_mode") - switch_mouse_action.main_button.setChecked(True) - self.mouse_mode = "RectMode" + if not self.target_widget: + return + mouse_mode = self.target_widget.plot_item.getViewBox().getState()["mouseMode"] + switch_mouse_action = self.components.get_action_reference("switch_mouse_mode")() + if mouse_mode == pg.ViewBox.PanMode: + switch_mouse_action.set_default_action("drag_mode") + elif mouse_mode == pg.ViewBox.RectMode: + switch_mouse_action.set_default_action("rectangle_mode") @SafeSlot(bool) def enable_mouse_rectangle_mode(self, checked: bool): """ Enable the rectangle zoom mode on the plot widget. + + A mouse mode is always active. When the rectangle action is unchecked because the + user switched to pan, the pan action becomes checked and takes over. When it is + unchecked directly (e.g. clicking the active main button), no other mode is active, + so the rectangle action is re-checked to avoid leaving every sub-action unchecked. """ - switch_mouse_action = self.components.get_action_reference("switch_mouse_mode")() - if self.mouse_mode == "RectMode": - switch_mouse_action.main_button.setChecked(True) + + if not checked: + self.components.get_action_reference("switch_mouse_mode")().main_button.setChecked(True) + if not self.components.get_action_reference("mouse_drag")().action.isChecked(): + self.components.get_action_reference("mouse_rect")().action.setChecked(True) return - drag_mode = self.components.get_action_reference("mouse_drag")() - drag_mode.action.setChecked(not checked) - if self.target_widget and checked: + if self.target_widget: self.target_widget.plot_item.getViewBox().setMouseMode(pg.ViewBox.RectMode) - self.mouse_mode = "RectMode" @SafeSlot(bool) def enable_mouse_pan_mode(self, checked: bool): """ Enable the pan mode on the plot widget. + + See :meth:`enable_mouse_rectangle_mode`: the pan action is re-checked when it is + unchecked directly while no other mode is active, so a mouse mode is always selected. """ - if self.mouse_mode == "PanMode": - switch_mouse_action = self.components.get_action_reference("switch_mouse_mode")() - switch_mouse_action.main_button.setChecked(True) + if not checked: + self.components.get_action_reference("switch_mouse_mode")().main_button.setChecked(True) + if not self.components.get_action_reference("mouse_rect")().action.isChecked(): + self.components.get_action_reference("mouse_drag")().action.setChecked(True) return - rect_mode = self.components.get_action_reference("mouse_rect")() - rect_mode.action.setChecked(not checked) - if self.target_widget and checked: + if self.target_widget: self.target_widget.plot_item.getViewBox().setMouseMode(pg.ViewBox.PanMode) - self.mouse_mode = "PanMode" @SafeSlot() def autorange_plot(self): diff --git a/tests/unit_tests/test_modular_toolbar.py b/tests/unit_tests/test_modular_toolbar.py index c1086161..9f4908a2 100644 --- a/tests/unit_tests/test_modular_toolbar.py +++ b/tests/unit_tests/test_modular_toolbar.py @@ -536,6 +536,32 @@ def test_switchable_toolbar_action_is_exclusive(toolbar_fixture, switchable_tool assert not action2.isChecked() +def test_switchable_toolbar_action_menu_cache_rebuilt_on_reshow(toolbar_fixture, qtbot): + """Re-showing bundles rebuilds the menu cache: no stale/duplicate menu actions.""" + action1 = MaterialIconAction(icon_name="counter_1", tooltip="Action 1", checkable=True) + action2 = MaterialIconAction(icon_name="counter_2", tooltip="Action 2", checkable=True) + switch = SwitchableToolBarAction( + actions={"action1": action1, "action2": action2}, initial_action="action1", checkable=True + ) + toolbar = toolbar_fixture + toolbar.add_action("switch_action", switch) + toolbar.show_bundles(["switch_action"]) + assert set(switch.menu_actions) == {"action1", "action2"} + + # add_to_toolbar runs again on the same instance -> the cache must not accumulate + # stale entries, and the cached actions must stay live and usable. + toolbar.show_bundles(["switch_action"]) + assert set(switch.menu_actions) == {"action1", "action2"} + for menu_action in switch.menu_actions.values(): + # Touching the cached action must not raise (i.e. not a deleted C++ object). + menu_action.setChecked(menu_action.isChecked()) + + switch.set_default_action("action2") + assert switch.current_key == "action2" + assert switch.menu_actions["action2"].isChecked() + assert not switch.menu_actions["action1"].isChecked() + + def test_switchable_toolbar_action_can_disable_exclusivity(toolbar_fixture): """When exclusive=False the actions are independent (no action group).""" action1 = MaterialIconAction(icon_name="counter_1", tooltip="Action 1", checkable=True) diff --git a/tests/unit_tests/test_plot_base_next_gen.py b/tests/unit_tests/test_plot_base_next_gen.py index c1918d65..ad67ba68 100644 --- a/tests/unit_tests/test_plot_base_next_gen.py +++ b/tests/unit_tests/test_plot_base_next_gen.py @@ -260,6 +260,61 @@ def test_crosshair_hook_unhook(qtbot, mocked_client): assert pb.crosshair is None +def test_mouse_mode_switch_is_exclusive(qtbot, mocked_client): + """The pan/rectangle mouse modes must be mutually exclusive and always one active.""" + import pyqtgraph as pg + + pb = create_widget(qtbot, PlotBase, client=mocked_client) + switch = pb.toolbar.components.get_action("switch_mouse_mode") + drag = switch.actions["drag_mode"].action + rect = switch.actions["rectangle_mode"].action + viewbox = pb.plot_item.getViewBox() + + # Switching to rectangle mode updates the viewbox and unchecks the pan action. + switch.set_default_action("rectangle_mode") + assert rect.isChecked() and not drag.isChecked() + assert viewbox.getState()["mouseMode"] == pg.ViewBox.RectMode + assert switch.main_button.isChecked() + + # Switching back to pan mode flips both the viewbox and the checked states. + switch.set_default_action("drag_mode") + assert drag.isChecked() and not rect.isChecked() + assert viewbox.getState()["mouseMode"] == pg.ViewBox.PanMode + + # The exclusive action group forbids both modes being active at once. + drag.setChecked(True) + rect.setChecked(True) + assert rect.isChecked() and not drag.isChecked() + + +def test_mouse_mode_always_one_active_on_toggle_off(qtbot, mocked_client): + """Toggling the active mode off directly must keep exactly one mode selected.""" + import pyqtgraph as pg + + pb = create_widget(qtbot, PlotBase, client=mocked_client) + switch = pb.toolbar.components.get_action("switch_mouse_mode") + drag = switch.actions["drag_mode"].action + rect = switch.actions["rectangle_mode"].action + viewbox = pb.plot_item.getViewBox() + + switch.set_default_action("drag_mode") + assert drag.isChecked() and not rect.isChecked() + + # Clicking the main button toggles the current action off; a mouse mode must + # remain active rather than leaving both sub-actions unchecked. + switch.main_button.click() + assert switch.main_button.isChecked() + assert drag.isChecked() and not rect.isChecked() + assert viewbox.getState()["mouseMode"] == pg.ViewBox.PanMode + + # Switching still works after a direct toggle-off. + switch.set_default_action("rectangle_mode") + assert rect.isChecked() and not drag.isChecked() + switch.main_button.click() + assert rect.isChecked() and not drag.isChecked() + assert viewbox.getState()["mouseMode"] == pg.ViewBox.RectMode + + def test_set_method(qtbot, mocked_client): """ Test using the set(...) convenience method to update multiple properties at once.