Fix unwanted signal triggers (#133)

* Add tests that check no signals are triggered
* Disable signals when updating attributes for standarditems and treewidgetitems
* Bump version to v0.8.5

Author: berendkleinhaneveld

collagraph/renderers/pyside/objects/standarditem.py

@@ -41,18 +41,27 @@ def remove(self, el):
 
 @PySideRenderer.register_set_attr(QStandardItem)
 def set_attribute(self, attr, value):
-    if attr == "model_index":
-        if model := self.model():
-            index = model.indexFromItem(self)
-            row, column = value
-            if index.row() != row or index.column() != column:
-                if parent := self.parent():
-                    # `it` is `self`
-                    it = parent.takeChild(index.row(), index.column())
-                    parent.setChild(row, column, self)
-                else:
-                    # `it` is `self`
-                    it = model.takeItem(index.row(), index.column())
-                    model.setItem(row, column, it)
+    # Before setting any attribute, make sure to disable
+    # all signals for the associated model
+    model = self.model()
+    if model:
+        model.blockSignals(True)
+
+    if attr == "model_index" and model:
+        index = model.indexFromItem(self)
+        row, column = value
+        if index.row() != row or index.column() != column:
+            if parent := self.parent():
+                # `it` is `self`
+                it = parent.takeChild(index.row(), index.column())
+                parent.setChild(row, column, self)
+            else:
+                # `it` is `self`
+                it = model.takeItem(index.row(), index.column())
+                model.setItem(row, column, it)
 
     qobject_set_attribute(self, attr, value)
+
+    # And don't forget to enable signals when done
+    if model:
+        model.blockSignals(True)

collagraph/renderers/pyside/objects/treewidgetitem.py

@@ -35,21 +35,29 @@ def remove(self, el: QTreeWidgetItem):
 
 @PySideRenderer.register_set_attr(QTreeWidgetItem)
 def set_attribute(self, attr, value):
-    if attr == "content":
-        for col, data in value.items():
-            self.setText(col, data)
-        return
-    elif attr == "expanded":
-        if not self.parent():
-            self._expanded = value
-        else:
-            self.setExpanded(value)
-        return
-    elif attr == "selected":
-        if not self.parent():
-            self._selected = value
-        else:
-            self.setSelected(value)
-        return
-
-    qobject_set_attribute(self, attr, value)
+    # Before setting any attribute, make sure to disable
+    # all signals for the tree widget
+    tree_widget = self.treeWidget()
+    if tree_widget:
+        tree_widget.blockSignals(True)
+
+    match attr:
+        case "content":
+            for col, data in value.items():
+                self.setText(col, data)
+        case "expanded":
+            if not self.parent():
+                self._expanded = value
+            else:
+                self.setExpanded(value)
+        case "selected":
+            if not self.parent():
+                self._selected = value
+            else:
+                self.setSelected(value)
+        case _:
+            qobject_set_attribute(self, attr, value)
+
+    # And don't forget to enable signals when done
+    if tree_widget:
+        tree_widget.blockSignals(True)

pyproject.toml

@@ -1,6 +1,6 @@
 [project]
 name = "collagraph"
-version = "0.8.4"
+version = "0.8.5"
 description = "Reactive user interfaces"
 authors = [
     { name = "Berend Klein Haneveld", email = "berendkleinhaneveld@gmail.com" },

tests/pyside/test_pyside_trigger_signals.py

@@ -0,0 +1,177 @@
+from functools import partial
+
+import pytest
+from observ import reactive
+
+pytest.importorskip("PySide6")
+
+from PySide6 import QtWidgets
+
+import collagraph as cg
+
+TREE_WIDGET = """
+<treewidget @item-changed="item_changed">
+  <treewidgetitem
+    v-for="item in items"
+    :content="{0: item}"
+  />
+</treewidget>
+
+<script>
+import collagraph as cg
+
+class Tree(cg.Component):
+    changed = 0
+
+    def init(self):
+        pass
+
+    def item_changed(self, item, column):
+        Tree.changed += 1
+</script>
+"""
+
+TREE_VIEW = """
+<treeview>
+  <itemmodel @item_changed="item_changed">
+    <standarditem
+      v-for="item in items"
+      :text="item"
+    />
+  </itemmodel>
+</treewidget>
+
+<script>
+import collagraph as cg
+
+class Tree(cg.Component):
+    changed = 0
+
+    def init(self):
+        pass
+
+    def item_changed(self, item):
+        Tree.changed += 1
+</script>
+"""
+
+
+def tree_widget_nr_items_found(container, count):
+    tree_widget = container.findChild(QtWidgets.QTreeWidget)
+    assert tree_widget
+    root_item = tree_widget.invisibleRootItem()
+    assert root_item.childCount() == count
+
+
+def tree_view_nr_items_found(container, count):
+    tree_widget = container.findChild(QtWidgets.QTreeView)
+    assert tree_widget
+    model = tree_widget.model()
+    assert model.rowCount() == count
+
+
+def test_qtreewidgetitem_change_list_no_trigger(qtbot, parse_source):
+    """
+    When a qstandarditem is adjusted by the renderer, it should not trigger any signals.
+    """
+    Tree, _ = parse_source(TREE_WIDGET)
+    state = reactive({"items": ["a", "b", "c", "d"]})
+
+    renderer = cg.PySideRenderer(autoshow=False)
+    gui = cg.Collagraph(renderer=renderer)
+    container = renderer.create_element("widget")
+    gui.render(Tree, container, state=state)
+    assert Tree.changed == 0
+
+    qtbot.waitUntil(partial(tree_widget_nr_items_found, container, 4), timeout=500)
+    assert Tree.changed == 0
+
+    state["items"].pop(2)
+    assert Tree.changed == 0
+
+    qtbot.waitUntil(partial(tree_widget_nr_items_found, container, 3), timeout=500)
+    assert Tree.changed == 0
+
+
+def test_qtreewidgetitem_change_item_no_trigger(qtbot, parse_source):
+    """
+    When a qstandarditem is adjusted by the renderer, it should not trigger any signals.
+    """
+    Tree, _ = parse_source(TREE_WIDGET)
+    state = reactive({"items": ["a", "b", "c", "d"]})
+
+    renderer = cg.PySideRenderer(autoshow=False)
+    gui = cg.Collagraph(renderer=renderer)
+    container = renderer.create_element("widget")
+    gui.render(Tree, container, state=state)
+    assert Tree.changed == 0
+
+    qtbot.waitUntil(partial(tree_widget_nr_items_found, container, 4), timeout=500)
+    assert Tree.changed == 0
+
+    def item_in_list(index: int, value: str):
+        tree_widget = container.findChild(QtWidgets.QTreeView)
+        assert tree_widget
+        root_item = tree_widget.invisibleRootItem()
+        item = root_item.child(index)
+        assert item.text(0) == value, item.text(0)
+
+    state["items"][2] = "e"
+    assert Tree.changed == 0
+
+    qtbot.waitUntil(partial(item_in_list, 2, "e"), timeout=500)
+    assert Tree.changed == 0
+
+
+def test_qstandarditem_change_list_no_trigger(qtbot, parse_source):
+    """
+    When a qstandarditem is adjusted by the renderer, it should not trigger any signals.
+    """
+    Tree, _ = parse_source(TREE_VIEW)
+    state = reactive({"items": ["a", "b", "c", "d"]})
+
+    renderer = cg.PySideRenderer(autoshow=False)
+    gui = cg.Collagraph(renderer=renderer)
+    container = renderer.create_element("widget")
+    gui.render(Tree, container, state=state)
+    assert Tree.changed == 0
+
+    qtbot.waitUntil(partial(tree_view_nr_items_found, container, 4), timeout=500)
+    assert Tree.changed == 0
+
+    state["items"].pop(2)
+    assert Tree.changed == 0
+
+    qtbot.waitUntil(partial(tree_view_nr_items_found, container, 3), timeout=500)
+    assert Tree.changed == 0
+
+
+def test_qstandarditem_change_item_no_trigger(qtbot, parse_source):
+    """
+    When a qstandarditem is adjusted by the renderer, it should not trigger any signals.
+    """
+    Tree, _ = parse_source(TREE_VIEW)
+    state = reactive({"items": ["a", "b", "c", "d"]})
+
+    renderer = cg.PySideRenderer(autoshow=False)
+    gui = cg.Collagraph(renderer=renderer)
+    container = renderer.create_element("widget")
+    gui.render(Tree, container, state=state)
+    assert Tree.changed == 0
+
+    qtbot.waitUntil(partial(tree_view_nr_items_found, container, 4), timeout=500)
+    assert Tree.changed == 0
+
+    def item_in_list(index: int, value: str):
+        tree_widget = container.findChild(QtWidgets.QTreeView)
+        assert tree_widget
+        model = tree_widget.model()
+        assert model.hasIndex(index, 0)
+        item_index = model.index(index, 0)
+        assert model.itemData(item_index)[0] == value, model.itemData(item_index)
+
+    state["items"][2] = "e"
+    assert Tree.changed == 0
+
+    qtbot.waitUntil(partial(item_in_list, 2, "e"), timeout=500)
+    assert Tree.changed == 0