fix: Test and clean stage position boxes

src/pymmcore_widgets/control/_stage_widget.py

@@ -5,7 +5,8 @@
 
 from fonticon_mdi6 import MDI6
 from pymmcore_plus import CMMCorePlus, DeviceType, Keyword
-from qtpy.QtCore import QEvent, QObject, QPoint, Qt, QTimerEvent, Signal
+from qtpy.QtCore import QEvent, QObject, Qt, QTimerEvent, Signal
+from qtpy.QtGui import QContextMenuEvent
 from qtpy.QtWidgets import (
     QCheckBox,
     QDoubleSpinBox,
@@ -97,25 +98,6 @@ def __init__(
 
         # enable custom context menu handling for right-click events
         self.setContextMenuPolicy(Qt.ContextMenuPolicy.NoContextMenu)
-        self.installEventFilter(self)
-
-    def eventFilter(self, obj: QObject, event: QEvent) -> bool:
-        # listen to right-click events even when the spinbox is disabled
-        if obj is self and event.type() == QEvent.Type.ContextMenu:
-            self._on_context_menu(event.globalPos())
-            return True
-        return super().eventFilter(obj, event)  # type: ignore [no-any-return]
-
-    def _on_context_menu(self, global_pos: QPoint) -> None:
-        menu = QMenu(self)
-        # update label based on the current state
-        label = "Disable Editing" if self.isEnabled() else "Enable Editing"
-        toggle_action = menu.addAction(label)
-        toggle_action.triggered.connect(self._toggle_enabled)
-        menu.exec(global_pos)
-
-    def _toggle_enabled(self) -> None:
-        self.setEnabled(not self.isEnabled())
 
 
 class HaltButton(QPushButton):
@@ -280,17 +262,28 @@ def __init__(
         self._step = self._move_btns.step_size
 
         self._pos = QHBoxLayout()
+        self._pos_boxes: list[MoveStageSpinBox] = []
+        self._pos_menu = QMenu(self)
+        self._pos_toggle_action = self._pos_menu.addAction("Enable Editing")
+        self._pos_toggle_action.setCheckable(True)
+        self._pos_toggle_action.setChecked(absolute_positioning)
+        self._pos_toggle_action.triggered.connect(self.enable_absolute_positioning)
+
         if self._is_2axis:
             self._pos.addWidget(QLabel("X: "))
             self._x_pos = MoveStageSpinBox(label="X")
+            self._pos_boxes.append(self._x_pos)
             self._pos.addWidget(self._x_pos)
             self._x_pos.editingFinished.connect(self._move_x_absolute)
 
         self._pos.addWidget(QLabel(f"{self._Ylabel}: "))
         self._y_pos = MoveStageSpinBox(label="Y")
+        self._pos_boxes.append(self._y_pos)
         self._y_pos.editingFinished.connect(self._move_y_absolute)
         self._pos.addWidget(self._y_pos)
 
+        for box in self._pos_boxes:
+            box.installEventFilter(self)
         self._pos.setAlignment(Qt.AlignmentFlag.AlignCenter)
 
         self._halt = HaltButton(device, self._mmc, self)
@@ -363,16 +356,23 @@ def setStep(self, step: float) -> None:
         self._step.setValue(step)
 
     def enable_absolute_positioning(self, enabled: bool) -> None:
-        if self._is_2axis:
-            self._x_pos.setEnabled(enabled)
-        self._y_pos.setEnabled(enabled)
+        """Toggles whether the position spinboxes can be edited by the user.
+
+        Parameters
+        ----------
+        enabled: bool:
+            If True, the position spinboxes will be enabled for user editing.
+            If False, the position spinboxes will be disabled for user editing.
+        """
+        self._pos_toggle_action.setChecked(enabled)
+        for box in self._pos_boxes:
+            box.setEnabled(enabled)
 
     def _enable_wdg(self, enabled: bool) -> None:
         self._step.setEnabled(enabled)
         self._move_btns.setEnabled(enabled)
-        if self._is_2axis:
-            self._x_pos.setEnabled(enabled)
-        self._y_pos.setEnabled(enabled)
+        for box in self._pos_boxes:
+            box.setEnabled(enabled and self._pos_toggle_action.isChecked())
         self.snap_checkbox.setEnabled(enabled)
         self._set_as_default_btn.setEnabled(enabled)
         self._poll_cb.setEnabled(enabled)
@@ -427,6 +427,15 @@ def timerEvent(self, event: QTimerEvent | None) -> None:
             self._update_position_from_core()
         super().timerEvent(event)
 
+    def eventFilter(self, obj: QObject | None, event: QEvent | None) -> bool:
+        # NB QAbstractSpinBox has its own Context Menu handler, which conflicts
+        # with the one we want to generate. So we intercept the event here >:)
+        # See https://stackoverflow.com/a/71126504
+        if obj in self._pos_boxes and isinstance(event, QContextMenuEvent):
+            self._pos_menu.exec_(event.globalPos())
+            return True
+        return super().eventFilter(obj, event)  # type: ignore [no-any-return]
+
     def _update_position_from_core(self) -> None:
         if self._device not in self._mmc.getLoadedDevicesOfType(self._dtype):
             return
@@ -452,7 +461,7 @@ def _move_stage_relative(self, x: float, y: float) -> None:
             else:
                 self._mmc.setRelativePosition(self._device, y)
         except Exception as e:
-            self._mmc.logMessage(f"Error moving stage: {e}")
+            self._mmc.logMessage(f"Error moving stage: {e}")  # pragma: no cover
         else:
             if self.snap_checkbox.isChecked():
                 self._mmc.snap()
@@ -463,7 +472,7 @@ def _move_x_absolute(self) -> None:
             y = self._mmc.getYPosition(self._device)
             self._mmc.setXYPosition(self._device, x, y)
         except Exception as e:
-            self._mmc.logMessage(f"Error moving stage: {e}")
+            self._mmc.logMessage(f"Error moving stage: {e}")  # pragma: no cover
         else:
             if self.snap_checkbox.isChecked():
                 self._mmc.snap()
@@ -477,7 +486,7 @@ def _move_y_absolute(self) -> None:
             else:
                 self._mmc.setPosition(self._device, y)
         except Exception as e:
-            self._mmc.logMessage(f"Error moving stage: {e}")
+            self._mmc.logMessage(f"Error moving stage: {e}")  # pragma: no cover
         else:
             if self.snap_checkbox.isChecked():
                 self._mmc.snap()

tests/test_stage_widget.py

@@ -73,6 +73,12 @@ def test_stage_widget(qtbot: QtBot, global_mmcore: CMMCorePlus) -> None:
     with qtbot.waitSignal(global_mmcore.events.imageSnapped):
         global_mmcore.waitForDevice("XY")
         xy_up_3.widget().click()
+    with qtbot.waitSignal(global_mmcore.events.imageSnapped):
+        stage_xy._x_pos.setValue(10)
+        stage_xy._x_pos.editingFinished.emit()
+    with qtbot.waitSignal(global_mmcore.events.imageSnapped):
+        stage_xy._y_pos.setValue(10)
+        stage_xy._y_pos.editingFinished.emit()
 
     # test Z stage
     stage_z = StageWidget("Z", levels=3)
@@ -123,6 +129,9 @@ def test_stage_widget(qtbot: QtBot, global_mmcore: CMMCorePlus) -> None:
     with qtbot.waitSignal(global_mmcore.events.imageSnapped):
         global_mmcore.waitForDevice("Z")
         z_up_2.widget().click()
+    with qtbot.waitSignal(global_mmcore.events.imageSnapped):
+        stage_xy._y_pos.setValue(10)
+        stage_xy._y_pos.editingFinished.emit()
 
     # disconnect
     assert global_mmcore.getFocusDevice() == "Z"
@@ -136,6 +145,50 @@ def test_stage_widget(qtbot: QtBot, global_mmcore: CMMCorePlus) -> None:
     assert not stage_z1._set_as_default_btn.isChecked()
 
 
+def test_enable_position_buttons(qtbot: QtBot, global_mmcore: CMMCorePlus) -> None:
+    # Absolute positioning disabled
+    stage_xy = StageWidget("XY", levels=3)
+    # Phase 1: position buttons cannot be enabled before the menu action is toggled
+    qtbot.addWidget(stage_xy)
+    assert not stage_xy._x_pos.isEnabled()
+    assert not stage_xy._y_pos.isEnabled()
+    stage_xy._enable_wdg(False)
+    assert not stage_xy._x_pos.isEnabled()
+    assert not stage_xy._y_pos.isEnabled()
+    stage_xy._enable_wdg(True)
+    assert not stage_xy._x_pos.isEnabled()
+    assert not stage_xy._y_pos.isEnabled()
+    # Phase 2: Trigger menu action, buttons can now be enabled
+    stage_xy._pos_toggle_action.trigger()
+    assert stage_xy._x_pos.isEnabled()
+    assert stage_xy._y_pos.isEnabled()
+    stage_xy._enable_wdg(False)
+    assert not stage_xy._x_pos.isEnabled()
+    assert not stage_xy._y_pos.isEnabled()
+    stage_xy._enable_wdg(True)
+    assert stage_xy._x_pos.isEnabled()
+    assert stage_xy._y_pos.isEnabled()
+    stage_xy._pos_toggle_action.trigger()
+    assert not stage_xy._x_pos.isEnabled()
+    assert not stage_xy._y_pos.isEnabled()
+    # Phase 3: Set absolute positioning using API
+    # Should be identical to Phase 2
+    stage_xy.enable_absolute_positioning(True)
+    assert stage_xy._pos_toggle_action.isChecked()
+    assert stage_xy._x_pos.isEnabled()
+    assert stage_xy._y_pos.isEnabled()
+    stage_xy._enable_wdg(False)
+    assert not stage_xy._x_pos.isEnabled()
+    assert not stage_xy._y_pos.isEnabled()
+    stage_xy._enable_wdg(True)
+    assert stage_xy._x_pos.isEnabled()
+    assert stage_xy._y_pos.isEnabled()
+    stage_xy.enable_absolute_positioning(False)
+    assert not stage_xy._pos_toggle_action.isChecked()
+    assert not stage_xy._x_pos.isEnabled()
+    assert not stage_xy._y_pos.isEnabled()
+
+
 def test_invert_axis(qtbot: QtBot, global_mmcore: CMMCorePlus) -> None:
     stage_xy = StageWidget("XY", levels=3)
     qtbot.addWidget(stage_xy)