fix: Use an event filter to avoid scrolling (#433)

* Use an event filter to avoid scrolling

* Delete FIXME

Was already fixed

* Use NoWheelTableWidget instead

Thanks @tlambert03 for the suggestion/starter code

* Use the NoWheelTableWidget for Device Properties

* Update tests

* Don't pass tuple to QObject.findChildren()

Author: gselzer

src/pymmcore_widgets/_util.py

@@ -2,20 +2,24 @@
 
 import re
 from pathlib import Path
-from typing import TYPE_CHECKING, Any
+from typing import TYPE_CHECKING
 
 import useq
 from psygnal import SignalInstance
 from pymmcore_plus import CMMCorePlus
-from qtpy.QtCore import QMarginsF, QObject, Qt
-from qtpy.QtGui import QPainter, QPaintEvent, QPen, QResizeEvent
+from qtpy.QtCore import QEvent, QMarginsF, QObject, Qt
+from qtpy.QtGui import QPainter, QPaintEvent, QPen, QResizeEvent, QWheelEvent
 from qtpy.QtWidgets import (
+    QAbstractSlider,
+    QAbstractSpinBox,
     QComboBox,
     QDialog,
     QDialogButtonBox,
     QGraphicsScene,
     QGraphicsView,
     QLabel,
+    QScrollBar,
+    QTableWidget,
     QVBoxLayout,
     QWidget,
 )
@@ -24,6 +28,42 @@
 if TYPE_CHECKING:
     from collections.abc import Sequence
     from contextlib import AbstractContextManager
+    from typing import Any
+
+
+class NoWheelTableWidget(QTableWidget):
+    """QTableWidget that prevents scrolling behavior of child widgets."""
+
+    def __init__(self, *args: Any, **kwargs: Any) -> None:
+        super().__init__(*args, **kwargs)
+        if view := self.viewport():
+            view.installEventFilter(self)
+
+    def setCellWidget(self, row: int, column: int, widget: QWidget | None) -> None:
+        if widget is not None:
+            # Note that PySide does not allow a tuple as an arg to findChildren
+            for widget_type in (QAbstractSpinBox, QAbstractSlider, QComboBox):
+                for child in widget.findChildren(widget_type):
+                    child.installEventFilter(self)
+            widget.installEventFilter(self)
+        super().setCellWidget(row, column, widget)
+
+    def eventFilter(self, object: QObject | None, event: QEvent | None) -> bool:
+        # Many "value widgets" (e.g. Combo boxes, sliders) use scroll events to
+        # change their value. The purpose of this widget is to prevent that.
+        # Note that if that widget provides a scrollbar (e.g. the ListView
+        # dropdown of a Combo Box), scroll events going to that scrollbar should
+        # be let through.
+        if isinstance(event, QWheelEvent) and not isinstance(object, QScrollBar):
+            # Scroll the vertical scrollbar manually, to avoid recursion error.
+            if sb := self.verticalScrollBar():
+                delta = event.angleDelta().y()
+                sb.setValue(sb.value() - delta)
+            # Consume the event so child widgets don't process it
+            return True
+
+        # otherwise process normally
+        return False
 
 
 class ComboMessageBox(QDialog):

src/pymmcore_widgets/config_presets/_group_preset_widget/_group_preset_table_widget.py

@@ -20,7 +20,7 @@
     QWidget,
 )
 
-from pymmcore_widgets._util import block_core, load_system_config
+from pymmcore_widgets._util import NoWheelTableWidget, block_core, load_system_config
 from pymmcore_widgets.control._presets_widget import PresetsWidget
 from pymmcore_widgets.device_properties._property_widget import PropertyWidget
 
@@ -32,18 +32,18 @@
 UNNAMED_PRESET = "NewPreset"
 
 
-class _MainTable(QTableWidget):
+class _MainTable(NoWheelTableWidget):
     """Set table properties for Group and Preset TableWidget."""
 
     def __init__(self) -> None:
         super().__init__()
-        hdr = self.horizontalHeader()
-        hdr.setStretchLastSection(True)
-        hdr.setDefaultAlignment(Qt.AlignmentFlag.AlignHCenter)
-        vh = self.verticalHeader()
-        vh.setVisible(False)
-        vh.setSectionResizeMode(vh.ResizeMode.Fixed)
-        vh.setDefaultSectionSize(24)
+        if (hdr := self.horizontalHeader()) is not None:
+            hdr.setStretchLastSection(True)
+            hdr.setDefaultAlignment(Qt.AlignmentFlag.AlignHCenter)
+        if (vh := self.verticalHeader()) is not None:
+            vh.setVisible(False)
+            vh.setSectionResizeMode(vh.ResizeMode.Fixed)
+            vh.setDefaultSectionSize(24)
         self.setSizeAdjustPolicy(QAbstractScrollArea.SizeAdjustPolicy.AdjustToContents)
         self.setEditTriggers(QTableWidget.EditTrigger.NoEditTriggers)
         self.setSelectionBehavior(QTableWidget.SelectionBehavior.SelectRows)

src/pymmcore_widgets/control/_presets_widget.py

@@ -4,22 +4,13 @@
 
 from pymmcore_plus import CMMCorePlus, DeviceType
 from qtpy.QtCore import Qt
-from qtpy.QtGui import QBrush, QWheelEvent
+from qtpy.QtGui import QBrush
 from qtpy.QtWidgets import QComboBox, QHBoxLayout, QWidget
 from superqt.utils import signals_blocked
 
 from pymmcore_widgets._util import block_core
 
 
-class _PresetComboBox(QComboBox):
-    """A QComboBox tailored to selecting group presets."""
-
-    def wheelEvent(self, e: QWheelEvent | None) -> None:
-        # Scrolling over the combobox can easily lead to accidents, e.g. switching the
-        # objective lens.
-        pass
-
-
 class PresetsWidget(QWidget):
     """A Widget to create a QCombobox containing the presets of the specified group.
 
@@ -61,7 +52,7 @@ def __init__(
         # since they must be all the same
         self.dev_prop = self._get_preset_dev_prop(self._group, self._presets[0])
 
-        self._combo = _PresetComboBox()
+        self._combo = QComboBox()
         self._combo.currentTextChanged.connect(self._update_tooltip)
         self._combo.addItems(self._presets)
         self._combo.setCurrentText(self._mmc.getCurrentConfig(self._group))

src/pymmcore_widgets/device_properties/_device_property_table.py

@@ -10,6 +10,7 @@
 from superqt.fonticon import icon
 
 from pymmcore_widgets._icons import ICONS
+from pymmcore_widgets._util import NoWheelTableWidget
 
 from ._property_widget import PropertyWidget
 
@@ -19,7 +20,7 @@
 logger = getLogger(__name__)
 
 
-class DevicePropertyTable(QTableWidget):
+class DevicePropertyTable(NoWheelTableWidget):
     """Table of all currently loaded device properties.
 
     This table is used by `PropertyBrowser` to display all properties in the system,

tests/test_presets_widget.py

@@ -3,8 +3,6 @@
 from typing import TYPE_CHECKING
 
 import pytest
-from qtpy.QtCore import QPoint, QPointF, Qt
-from qtpy.QtGui import QWheelEvent
 
 from pymmcore_widgets.control._presets_widget import PresetsWidget
 
@@ -81,25 +79,3 @@ def test_preset_widget(qtbot: QtBot, global_mmcore: CMMCorePlus) -> None:
 
     global_mmcore.deleteConfigGroup("Camera")
     assert "Camera" not in global_mmcore.getAvailableConfigGroups()
-
-
-def test_preset_widget_no_scroll(qtbot: QtBot, global_mmcore: CMMCorePlus) -> None:
-    # Test that the widget does not respond to scroll events when unfocused.
-    # It is easy to accidentally scroll the widget when trying to scroll the group
-    # preset window, which could lead to costly accidents (e.g. switching objectives)
-    wdg = PresetsWidget("Camera")
-    qtbot.addWidget(wdg)
-
-    previous = wdg._combo.currentText()
-    e = QWheelEvent(
-        QPointF(0, 0),  # pos
-        QPointF(0, 0),  # globalPos
-        QPoint(0, 0),  # pixelDelta
-        QPoint(0, -120),  # angleDelta
-        Qt.MouseButton.NoButton,  # buttons
-        Qt.KeyboardModifier.NoModifier,  # modifiers
-        Qt.ScrollPhase.NoScrollPhase,  # phase
-        False,  # inverted
-    )
-    wdg._combo.wheelEvent(e)
-    assert wdg._combo.currentText() == previous

tests/test_utils.py

@@ -0,0 +1,74 @@
+from __future__ import annotations
+
+from typing import TYPE_CHECKING
+
+from qtpy.QtCore import QCoreApplication, QPoint, QPointF, Qt
+from qtpy.QtGui import QWheelEvent
+from qtpy.QtWidgets import QApplication, QComboBox
+from superqt import QDoubleSlider
+
+from pymmcore_widgets._util import NoWheelTableWidget
+
+if TYPE_CHECKING:
+    from pymmcore_plus import CMMCorePlus
+    from pytestqt.qtbot import QtBot
+
+
+WHEEL_UP = QWheelEvent(
+    QPointF(0, 0),  # pos
+    QPointF(0, 0),  # globalPos
+    QPoint(0, 0),  # pixelDelta
+    QPoint(0, 120),  # angleDelta
+    Qt.MouseButton.NoButton,  # buttons
+    Qt.KeyboardModifier.NoModifier,  # modifiers
+    Qt.ScrollPhase.NoScrollPhase,  # phase
+    False,  # inverted
+)
+
+
+def test_no_wheel_table_scroll(qtbot: QtBot, global_mmcore: CMMCorePlus) -> None:
+    tbl = NoWheelTableWidget()
+    qtbot.addWidget(tbl)
+    tbl.show()
+
+    # Create enough widgets to scroll
+    sb = tbl.verticalScrollBar()
+    assert sb is not None
+    while sb.maximum() == 0:
+        new_row = tbl.rowCount()
+        tbl.insertRow(new_row)
+        tbl.setCellWidget(new_row, 0, QComboBox(tbl))
+        QApplication.processEvents()
+
+    # Test Combo Box
+    combo = QComboBox(tbl)
+    combo.addItems(["combo0", "combo1", "combo2"])
+    combo.setCurrentIndex(1)
+    combo_row = tbl.rowCount()
+    tbl.insertRow(combo_row)
+    tbl.setCellWidget(combo_row, 0, combo)
+
+    sb.setValue(sb.maximum())
+    # Synchronous event emission and allows us to pass through the event filter
+    QCoreApplication.sendEvent(combo, WHEEL_UP)
+    # Assert the table widget scrolled but the combo didn't change
+    assert sb.value() < sb.maximum()
+    assert combo.currentIndex() == 1
+
+    # Test Slider
+    slider = QDoubleSlider(tbl)
+    slider.setRange(0, 1)
+    slider.setValue(0)
+    slider_row = tbl.rowCount()
+    tbl.insertRow(slider_row)
+    tbl.setCellWidget(slider_row, 0, slider)
+
+    sb.setValue(sb.maximum())
+    # Synchronous event emission and allows us to pass through the event filter
+    QCoreApplication.sendEvent(slider, WHEEL_UP)
+    # Assert the table widget scrolled but the slider didn't change
+    assert sb.value() < sb.maximum()
+    assert slider.value() == 0
+
+    # I can't know how to hear any more about tables
+    tbl.close()