Collapsible Widget API v1.1 (#23)

* WIP collapsible widget API v.1.1

* Updated tests for new functionality

* Update brainglobe_utils/qtpy/collapsible_widget.py

Co-authored-by: Alessandro Felder <[email protected]>

* Made collapsible=True explicit

* Added test for removing widget not found in container

* Remove TODOs

---------

Co-authored-by: Alessandro Felder <[email protected]>

brainglobe_utils/qtpy/collapsible_widget.py

@@ -1,7 +1,7 @@
-from typing import List, Optional, Union
+from typing import List, Optional
 
 from qtpy.QtCore import Qt, Signal
-from qtpy.QtWidgets import QGroupBox, QVBoxLayout, QWidget
+from qtpy.QtWidgets import QVBoxLayout, QWidget
 from superqt.collapsible import QCollapsible
 
 
@@ -59,62 +59,92 @@ def _on_toggle(self, state):
         self.toggled_signal_with_self.emit(self, state)
 
 
-class CollapsibleWidgetContainer(QGroupBox):
+class CollapsibleWidgetContainer(QWidget):
     """
     Container for multiple CollapsibleWidgets with the ability to add,
-    remove, and synchronize their states.
+    remove, and synchronize their states. Non-CollapsibleWidgets can also
+    be added.
 
     Methods
     -------
-    add_widget(QWidget or CollapsibleWidget)
+    add_widget(QWidget)
         Adds a widget to the CollapsibleWidgetContainer.
     remove_drawer(QWidget or CollapsibleWidget)
         Removes a widget from the CollapsibleWidgetContainer.
     _update_drawers(signalling_drawer, state)
         Private method to synchronize drawer states.
     """
 
-    def __init__(self):
+    def __init__(self, parent=None):
         """
         Initializes a new CollapsibleWidgetContainer instance.
         """
-        super().__init__()
+        super().__init__(parent)
         self.setLayout(QVBoxLayout())
 
         self.layout().setAlignment(Qt.AlignTop)
         self.layout().setSpacing(0)
         self.layout().setContentsMargins(0, 0, 0, 0)
         self.collapsible_widgets: List[CollapsibleWidget] = []
 
-    def add_widget(self, widget: Union[QWidget, CollapsibleWidget]):
+    def add_widget(
+        self, widget: QWidget, collapsible: bool = True, widget_title: str = ""
+    ):
         """
         Adds a QWidget or a CollapsibleWidget to the chest.
 
         Parameters
         ----------
-        widget : QWidget or CollapsibleWidget
+        widget : QWidget
             The widget instance to be added.
+        collapsible : bool, optional
+            Whether the widget should be collapsible.
+        widget_title : str, optional
+            The title of the widget.
         """
-        if isinstance(widget, CollapsibleWidget):
-            self.collapsible_widgets.append(widget)
-            widget.toggled_signal_with_self.connect(self._update_drawers)
-
-        self.layout().addWidget(widget, 0, Qt.AlignTop)
-
-    def remove_widget(self, widget: Union[QWidget, CollapsibleWidget]):
+        if collapsible:
+            collapsible_widget = CollapsibleWidget(widget_title, parent=self)
+            collapsible_widget.setContent(widget)
+            collapsible_widget.toggled_signal_with_self.connect(
+                self._update_drawers
+            )
+            collapsible_widget.collapse(animate=False)
+            self.collapsible_widgets.append(collapsible_widget)
+            self.layout().addWidget(collapsible_widget, 0, Qt.AlignTop)
+        else:
+            self.layout().addWidget(widget, 0, Qt.AlignTop)
+
+    def remove_widget(self, widget: QWidget):
         """
         Removes a widget from the chest.
 
         Parameters
         ----------
-        widget : QWidget or CollapsibleWidget
+        widget : QWidget
             The widget instance to be removed.
-        """
-        if isinstance(widget, CollapsibleWidget):
-            self.collapsible_widgets.remove(widget)
-            widget.toggled_signal_with_self.disconnect(self._update_drawers)
 
-        self.layout().removeWidget(widget)
+        Raises
+        ------
+        ValueError
+            If the widget is not found.
+        """
+        for i in range(self.layout().count()):
+            child_widget = self.layout().itemAt(i).widget()
+            if (
+                isinstance(child_widget, CollapsibleWidget)
+                and child_widget.content() is widget
+            ):
+                self.layout().removeWidget(child_widget)
+                self.collapsible_widgets.remove(child_widget)
+                child_widget.toggled_signal_with_self.disconnect(
+                    self._update_drawers
+                )
+                return
+            elif child_widget is widget:
+                self.layout().removeWidget(widget)
+                return
+
+        raise ValueError("Widget not found")
 
     def _update_drawers(
         self, signalling_widget: CollapsibleWidget, state: bool

tests/tests/test_qtpy/test_collapsible_widget.py

@@ -1,5 +1,12 @@
 import pytest
-from qtpy.QtWidgets import QLabel, QPushButton
+from qtpy.QtWidgets import (
+    QFormLayout,
+    QHBoxLayout,
+    QLabel,
+    QPushButton,
+    QVBoxLayout,
+    QWidget,
+)
 
 from brainglobe_utils.qtpy.collapsible_widget import (
     CollapsibleWidget,
@@ -9,6 +16,12 @@
 WIDGET_TITLE = "Title"
 
 
+@pytest.fixture(scope="class")
+def generic_widget() -> QWidget:
+    widget = QWidget()
+    return widget
+
+
 @pytest.fixture(scope="class")
 def collapsible_widget() -> CollapsibleWidget:
     collapsible_widget = CollapsibleWidget(WIDGET_TITLE)
@@ -86,61 +99,115 @@ def test_collapsible_widget_container(qtbot, collapsible_widget_container):
     assert len(collapsible_widget_container.collapsible_widgets) == 0
 
 
+@pytest.mark.parametrize(
+    "layout", [QVBoxLayout(), QHBoxLayout(), QFormLayout()]
+)
 def test_collapsible_widget_container_add_collapsible_widget(
-    qtbot, collapsible_widget_container, collapsible_widget
+    qtbot, collapsible_widget_container, generic_widget, layout
 ):
     qtbot.addWidget(collapsible_widget_container)
 
-    collapsible_widget_container.add_widget(collapsible_widget)
+    generic_widget.setLayout(layout)
+    generic_widget.layout().addWidget(QLabel("test"))
+    generic_widget.layout().addWidget(QPushButton("test"))
 
+    collapsible_widget_container.add_widget(
+        generic_widget, collapsible=True, widget_title=WIDGET_TITLE
+    )
+
+    # Check that the widget was added and is a CollapsibleWidget
     assert collapsible_widget_container.layout().count() == 1
+    assert isinstance(
+        collapsible_widget_container.layout().itemAt(0).widget(),
+        CollapsibleWidget,
+    )
+    # Check that the widget is collapsed and contains the generic widget
+    assert (
+        collapsible_widget_container.collapsible_widgets[0].content()
+        is generic_widget
+    )
+    assert (
+        collapsible_widget_container.layout().itemAt(0).widget().text()
+        == WIDGET_TITLE
+    )
     assert (
-        collapsible_widget_container.collapsible_widgets[0]
-        == collapsible_widget
+        not collapsible_widget_container.layout()
+        .itemAt(0)
+        .widget()
+        .isExpanded()
     )
     assert len(collapsible_widget_container.collapsible_widgets) == 1
 
 
-def test_collapsible_widget_container_add_other_widget(
-    qtbot, collapsible_widget_container
+@pytest.mark.parametrize("widget_type", [QLabel, QPushButton])
+def test_collapsible_widget_container_add_not_collapsible_widget(
+    qtbot, collapsible_widget_container, widget_type
 ):
     qtbot.addWidget(collapsible_widget_container)
 
-    collapsible_widget_container.add_widget(QLabel("test"))
+    collapsible_widget_container.add_widget(
+        widget_type(WIDGET_TITLE), collapsible=False
+    )
 
     assert collapsible_widget_container.layout().count() == 1
+    assert isinstance(
+        collapsible_widget_container.layout().itemAt(0).widget(), widget_type
+    )
     assert len(collapsible_widget_container.collapsible_widgets) == 0
 
 
+@pytest.mark.parametrize(
+    "layout, collapsible",
+    [
+        (QVBoxLayout(), True),
+        (QHBoxLayout(), True),
+        (QFormLayout(), True),
+        (QVBoxLayout(), False),
+        (QHBoxLayout(), False),
+        (QFormLayout(), False),
+    ],
+)
 def test_collapsible_widget_container_add_remove_widgets(
-    qtbot, collapsible_widget, collapsible_widget_container
+    qtbot, collapsible_widget_container, generic_widget, layout, collapsible
 ):
     qtbot.addWidget(collapsible_widget_container)
 
-    collapsible_widget_container.add_widget(collapsible_widget)
+    generic_widget.setLayout(layout)
+    generic_widget.layout().addWidget(QLabel("test"))
+    generic_widget.layout().addWidget(QPushButton("test"))
+
+    collapsible_widget_container.add_widget(
+        generic_widget, collapsible=collapsible, widget_title=WIDGET_TITLE
+    )
 
     assert collapsible_widget_container.layout().count() == 1
-    assert len(collapsible_widget_container.collapsible_widgets) == 1
+    # Convert collapsible to int (False -> 0, True -> 1)
+    # to check if collapsible_widgets is empty
+    assert len(collapsible_widget_container.collapsible_widgets) == int(
+        collapsible
+    )
 
-    collapsible_widget_container.remove_widget(collapsible_widget)
+    collapsible_widget_container.remove_widget(generic_widget)
 
     assert collapsible_widget_container.layout().count() == 0
     assert len(collapsible_widget_container.collapsible_widgets) == 0
 
 
 def test_collapsible_widget_container_add_remove_diff_widgets(
-    qtbot, collapsible_widget, collapsible_widget_container
+    qtbot, generic_widget, collapsible_widget_container
 ):
     qtbot.addWidget(collapsible_widget_container)
     other_widget = QLabel("test")
 
-    collapsible_widget_container.add_widget(collapsible_widget)
-    collapsible_widget_container.add_widget(other_widget)
+    collapsible_widget_container.add_widget(
+        generic_widget, collapsible=True, widget_title=WIDGET_TITLE
+    )
+    collapsible_widget_container.add_widget(other_widget, collapsible=False)
 
     assert collapsible_widget_container.layout().count() == 2
     assert len(collapsible_widget_container.collapsible_widgets) == 1
 
-    collapsible_widget_container.remove_widget(collapsible_widget)
+    collapsible_widget_container.remove_widget(generic_widget)
 
     assert collapsible_widget_container.layout().count() == 1
     assert len(collapsible_widget_container.collapsible_widgets) == 0
@@ -151,6 +218,13 @@ def test_collapsible_widget_container_add_remove_diff_widgets(
     assert len(collapsible_widget_container.collapsible_widgets) == 0
 
 
+def test_collapsible_widget_container_remove_widget_not_found(
+    qtbot, generic_widget, collapsible_widget_container
+):
+    with pytest.raises(ValueError):
+        collapsible_widget_container.remove_widget(generic_widget)
+
+
 @pytest.mark.parametrize(
     "num_collapsible_widgets, num_other_widgets, index_expanded",
     [(2, 4, 1), (5, 1, 3), (10, 0, 9)],
@@ -180,28 +254,27 @@ def test_collapsible_widget_container_update_drawers(
     # Add collapsible widgets and other widgets to the container alternating
     # until the correct number of each type of widget has been added
     for i in range(num_collapsible_widgets + num_other_widgets):
-        if i % 2 == 0:
-            if len(collapsible_widgets) == num_collapsible_widgets:
-                non_collapsible_widgets.append(QLabel("test"))
-                collapsible_widget_container.add_widget(
-                    non_collapsible_widgets[-1]
-                )
-            else:
-                collapsible_widgets.append(CollapsibleWidget(WIDGET_TITLE))
-                collapsible_widget_container.add_widget(
-                    collapsible_widgets[-1]
-                )
+        if i % 2 == 0 and len(non_collapsible_widgets) < num_other_widgets:
+            collapsible_widget_container.add_widget(
+                QLabel("test"), collapsible=False
+            )
+            non_collapsible_widgets.append(
+                collapsible_widget_container.layout().itemAt(i).widget()
+            )
+        elif len(collapsible_widgets) < num_collapsible_widgets:
+            collapsible_widget_container.add_widget(
+                QLabel("test"), collapsible=True
+            )
+            collapsible_widgets.append(
+                collapsible_widget_container.layout().itemAt(i).widget()
+            )
         else:
-            if len(non_collapsible_widgets) == num_other_widgets:
-                collapsible_widgets.append(CollapsibleWidget(WIDGET_TITLE))
-                collapsible_widget_container.add_widget(
-                    collapsible_widgets[-1]
-                )
-            else:
-                non_collapsible_widgets.append(QLabel("test"))
-                collapsible_widget_container.add_widget(
-                    non_collapsible_widgets[-1]
-                )
+            collapsible_widget_container.add_widget(
+                QLabel("test"), collapsible=False
+            )
+            non_collapsible_widgets.append(
+                collapsible_widget_container.layout().itemAt(i).widget()
+            )
 
     for _ in range(num_collapsible_widgets):
         collapsible_widgets[index_expanded]._toggle_btn.click()