diff --git a/bec_widgets/utils/toolbars/actions.py b/bec_widgets/utils/toolbars/actions.py index 95d5b272..7d98d70a 100644 --- a/bec_widgets/utils/toolbars/actions.py +++ b/bec_widgets/utils/toolbars/actions.py @@ -11,7 +11,7 @@ from bec_lib.device import ReadoutPriority from bec_lib.logger import bec_logger from bec_qthemes._icon.material_icons import material_icon from qtpy.QtCore import QSize, Qt, QTimer -from qtpy.QtGui import QAction, QColor, QIcon # type: ignore +from qtpy.QtGui import QAction, QActionGroup, QColor, QIcon # type: ignore from qtpy.QtWidgets import ( QApplication, QComboBox, @@ -314,6 +314,10 @@ class SwitchableToolBarAction(IconAction): initial_action (str, optional): The key of the initial default action. If not provided, the first action is used. tooltip (str, optional): An optional tooltip for the split action; if provided, it overrides the default action's tooltip. checkable (bool, optional): Whether the action is checkable. Defaults to True. + exclusive (bool, optional): Whether the switchable actions are mutually exclusive, i.e. only + one of them can be active at a time. Defaults to True. When enabled the actions are + managed by a ``QActionGroup`` (``ExclusiveOptional`` policy), so checking one action + structurally unchecks the others while still allowing all of them to be off. parent (QWidget, optional): Parent widget for the underlying QAction. """ @@ -324,6 +328,7 @@ class SwitchableToolBarAction(IconAction): tooltip: str | None = None, checkable: bool = True, default_state_checked: bool = False, + exclusive: bool = True, parent=None, ): super().__init__(icon_path=None, tooltip=tooltip, checkable=checkable) @@ -332,8 +337,15 @@ class SwitchableToolBarAction(IconAction): self.parent = parent self.checkable = checkable self.default_state_checked = default_state_checked + self.exclusive = exclusive self.main_button = None self.menu_actions: Dict[str, QAction] = {} + self.action_group: QActionGroup | None = None + if self.exclusive and self.checkable: + self.action_group = QActionGroup(parent) + self.action_group.setExclusionPolicy(QActionGroup.ExclusionPolicy.ExclusiveOptional) + for action_obj in self.actions.values(): + self.action_group.addAction(action_obj.action) def add_to_toolbar(self, toolbar: QToolBar, target: QWidget): """ @@ -360,6 +372,7 @@ class SwitchableToolBarAction(IconAction): menu_action.setChecked(key == self.current_key) menu_action.triggered.connect(lambda checked, k=key: self.set_default_action(k)) menu.addAction(menu_action) + self.menu_actions[key] = menu_action self.main_button.setMenu(menu) if self.default_state_checked: self.main_button.setChecked(True) @@ -385,14 +398,16 @@ class SwitchableToolBarAction(IconAction): new_action = self.actions[self.current_key] self.main_button.setIcon(new_action.get_icon()) self.main_button.setToolTip(new_action.tooltip) - # Update check state of menu items - for k, menu_act in self.actions.items(): - menu_act.action.setChecked(False) + for sub_action in self.actions.values(): + sub_action.action.setChecked(False) new_action.action.trigger() # Active action chosen from menu is always checked, uncheck through main button if self.checkable: new_action.action.setChecked(True) self.main_button.setChecked(True) + # Keep the drop-down menu's check indicators in sync with the current selection. + for menu_key, menu_act in self.menu_actions.items(): + menu_act.setChecked(menu_key == self.current_key) def block_all_signals(self, block: bool = True): """ diff --git a/bec_widgets/widgets/plots/image/toolbar_components/image_base_actions.py b/bec_widgets/widgets/plots/image/toolbar_components/image_base_actions.py index 6c8d63b6..31235204 100644 --- a/bec_widgets/widgets/plots/image/toolbar_components/image_base_actions.py +++ b/bec_widgets/widgets/plots/image/toolbar_components/image_base_actions.py @@ -76,7 +76,7 @@ class ImageRoiConnection(BundleConnection): self.components.get_action("image_crosshair").action.toggled.connect( self.target_widget.toggle_crosshair ) - self.components.get_action("image_crosshair_roi").action.triggered.connect( + self.components.get_action("image_crosshair_roi").action.toggled.connect( self.target_widget.toggle_roi_panels ) @@ -87,7 +87,7 @@ class ImageRoiConnection(BundleConnection): self.components.get_action("image_crosshair").action.toggled.disconnect( self.target_widget.toggle_crosshair ) - self.components.get_action("image_crosshair_roi").action.triggered.disconnect( + self.components.get_action("image_crosshair_roi").action.toggled.disconnect( self.target_widget.toggle_roi_panels ) diff --git a/bec_widgets/widgets/plots/plot_base.py b/bec_widgets/widgets/plots/plot_base.py index f2041a90..c145b7b8 100644 --- a/bec_widgets/widgets/plots/plot_base.py +++ b/bec_widgets/widgets/plots/plot_base.py @@ -1095,12 +1095,21 @@ class PlotBase(BECWidget, QWidget): self.crosshair.deleteLater() self.crosshair = None - def toggle_crosshair(self) -> None: - """Toggle the crosshair on all plots.""" - if self.crosshair is None: - return self.hook_crosshair() + def toggle_crosshair(self, enabled: bool | None = None) -> None: + """Toggle the crosshair on all plots. - self.unhook_crosshair() + Args: + enabled (bool | None): If provided, explicitly enable (``True``) or disable + (``False``) the crosshair. If ``None``, the current state is flipped. + When connected to a checkable action's ``toggled`` signal the checked + state is forwarded here, keeping the crosshair in sync with the action. + """ + if enabled is None: + enabled = self.crosshair is None + if enabled: + self.hook_crosshair() + else: + self.unhook_crosshair() @SafeProperty( int, diff --git a/tests/unit_tests/test_image_view_next_gen.py b/tests/unit_tests/test_image_view_next_gen.py index 77b003bd..16ca82c2 100644 --- a/tests/unit_tests/test_image_view_next_gen.py +++ b/tests/unit_tests/test_image_view_next_gen.py @@ -922,6 +922,44 @@ def test_crosshair_roi_panels_visibility(qtbot, mocked_client): ) +def test_crosshair_roi_switch_is_mutually_exclusive(qtbot, mocked_client): + """ + Switching the crosshair/ROI tool via the switcher menu must disable the + previously active mode. In particular, switching from the ROI-crosshair back + to the plain crosshair has to hide the ROI panels (and vice versa); the two + modes are mutually exclusive. + """ + bec_image_view = create_widget(qtbot, Image, client=mocked_client) + switch = bec_image_view.toolbar.components.get_action("image_switch_crosshair") + crosshair_action = switch.actions["crosshair"].action + crosshair_roi_action = switch.actions["crosshair_roi"].action + + def panels_visible() -> bool: + return bec_image_view.side_panel_x.panel_height > 0 and ( + bec_image_view.side_panel_y.panel_width > 0 + ) + + # Activate the ROI-crosshair mode -> panels visible, crosshair hooked. + switch.set_default_action("crosshair_roi") + qtbot.waitUntil(panels_visible, timeout=500) + assert bec_image_view.crosshair is not None + assert crosshair_roi_action.isChecked() + assert not crosshair_action.isChecked() + + # Switch to the plain crosshair -> ROI panels must hide (the regression). + switch.set_default_action("crosshair") + qtbot.waitUntil(lambda: not panels_visible(), timeout=500) + assert bec_image_view.crosshair is not None # crosshair still active + assert crosshair_action.isChecked() + assert not crosshair_roi_action.isChecked() + + # Switch back to the ROI-crosshair -> panels visible again. + switch.set_default_action("crosshair_roi") + qtbot.waitUntil(panels_visible, timeout=500) + assert crosshair_roi_action.isChecked() + assert not crosshair_action.isChecked() + + def test_roi_plot_data_from_image(qtbot, mocked_client): """ Check that ROI plots receive correct slice data from the 2D image. diff --git a/tests/unit_tests/test_modular_toolbar.py b/tests/unit_tests/test_modular_toolbar.py index 307485d0..c1086161 100644 --- a/tests/unit_tests/test_modular_toolbar.py +++ b/tests/unit_tests/test_modular_toolbar.py @@ -481,6 +481,12 @@ def test_switchable_toolbar_action_switching(toolbar_fixture, switchable_toolbar action_for_2 = act break assert action_for_2 is not None, "Menu action for 'Action 2' not found." + + # The drop-down menu must reflect the current selection (action1 initially). + assert switchable_toolbar_action.menu_actions.keys() == {"action1", "action2"} + assert switchable_toolbar_action.menu_actions["action1"].isChecked() + assert not switchable_toolbar_action.menu_actions["action2"].isChecked() + # Trigger the QAction to switch to action2 action_for_2.trigger() qtbot.wait(100) @@ -488,6 +494,69 @@ def test_switchable_toolbar_action_switching(toolbar_fixture, switchable_toolbar assert switchable_toolbar_action.current_key == "action2" assert switchable_toolbar_action.main_button.toolTip() == "Action 2" + # The menu check indicators must follow the active selection, not stay stale. + assert not switchable_toolbar_action.menu_actions["action1"].isChecked() + assert switchable_toolbar_action.menu_actions["action2"].isChecked() + + # Switching back must move the check mark back to action1. + switchable_toolbar_action.set_default_action("action1") + assert switchable_toolbar_action.current_key == "action1" + assert switchable_toolbar_action.menu_actions["action1"].isChecked() + assert not switchable_toolbar_action.menu_actions["action2"].isChecked() + + +def test_switchable_toolbar_action_is_exclusive(toolbar_fixture, switchable_toolbar_action): + """A switchable action must never leave more than one of its actions enabled.""" + toolbar = toolbar_fixture + switch = switchable_toolbar_action + toolbar.add_action("switch_action", switch) + toolbar.show_bundles(["switch_action"]) + + # The actions are managed by an exclusive group by default. + assert switch.exclusive is True + assert switch.action_group is not None + + action1 = switch.actions["action1"].action + action2 = switch.actions["action2"].action + + # Forcing both checked must collapse to only the most recently checked one. + action1.setChecked(True) + action2.setChecked(True) + assert action2.isChecked() + assert not action1.isChecked() + + # Re-checking the other one flips exclusively back. + action1.setChecked(True) + assert action1.isChecked() + assert not action2.isChecked() + + # ExclusiveOptional: it is still allowed to have no action enabled at all. + action1.setChecked(False) + assert not action1.isChecked() + assert not action2.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) + action2 = MaterialIconAction(icon_name="counter_2", tooltip="Action 2", checkable=True) + switch = SwitchableToolBarAction( + actions={"action1": action1, "action2": action2}, + initial_action="action1", + checkable=True, + exclusive=False, + ) + toolbar = toolbar_fixture + toolbar.add_action("switch_action", switch) + toolbar.show_bundles(["switch_action"]) + + assert switch.action_group is None + action1.action.setChecked(True) + action2.action.setChecked(True) + # Without the exclusive group both can be checked simultaneously. + assert action1.action.isChecked() + assert action2.action.isChecked() + def test_long_pressbutton(toolbar_fixture, switchable_toolbar_action, qtbot): toolbar = toolbar_fixture