change the design of ListEdit (#640)

Co-authored-by: Hanjin Liu <[email protected]>

src/magicgui/widgets/_concrete.py

@@ -6,7 +6,6 @@
 
 from __future__ import annotations
 
-import contextlib
 import datetime
 import inspect
 import math
@@ -596,6 +595,31 @@ def value(self, value: slice) -> None:
         self.step.value = value.step
 
 
+class _ListEditChildWidget(Container[Widget]):
+    """A widget to represent a single element of a ListEdit widget."""
+
+    def __init__(self, widget: ValueWidget):
+        btn = PushButton(text="-")
+        super().__init__(widgets=[widget, btn], layout="horizontal", labels=False)
+        self.btn_minus = btn
+        self.value_widget = widget
+        self.margins = (0, 0, 0, 0)
+
+        btn.changed.disconnect()
+        widget.changed.disconnect()
+        widget.changed.connect(self.changed.emit)
+        btn.max_height = btn.max_width = use_app().get_obj("get_text_width")("-") + 4
+
+    @property
+    def value(self) -> Any:
+        """Return value of the child widget."""
+        return self.value_widget.value
+
+    @value.setter
+    def value(self, value: Any) -> None:
+        """Set value of the child widget."""
+        self.value_widget.value = value
+
 @merge_super_sigs
 class ListEdit(Container[ValueWidget[_V]]):
     """A widget to represent a list of values.
@@ -644,24 +668,15 @@ def __init__(
         self._child_options = options or {}
 
         button_plus = PushButton(text="+", name="plus")
-        button_minus = PushButton(text="-", name="minus")
-
-        if self.layout == "horizontal":
-            button_plus.max_width = 40
-            button_minus.max_width = 40
 
         self.append(button_plus)  # type: ignore
-        self.append(button_minus)  # type: ignore
         button_plus.changed.disconnect()
-        button_minus.changed.disconnect()
         button_plus.changed.connect(lambda: self._append_value())
-        button_minus.changed.connect(self._pop_value)
 
         for a in _value:
             self._append_value(a)
 
         self.btn_plus = button_plus
-        self.btn_minus = button_minus
 
     @property
     def annotation(self) -> Any:
@@ -711,36 +726,33 @@ def __delitem__(self, key: int | slice) -> None:
 
     def _append_value(self, value: _V | _Undefined = Undefined) -> None:
         """Create a new child value widget and append it."""
-        i = len(self) - 2
-
-        widget = cast(
-            ValueWidget,
-            create_widget(
-                annotation=self._args_type,
-                name=f"value_{i}",
-                options=self._child_options,
-            ),
+        i = len(self) - 1
+
+        _value_widget = create_widget(
+            annotation=self._args_type,
+            name=f"value_{i}",
+            options=self._child_options,
         )
+        widget = _ListEditChildWidget(cast(ValueWidget, _value_widget))
+        # connect the minus-button-clicked event
+        widget.btn_minus.changed.connect(lambda: self.remove(widget))
 
-        self.insert(i, widget)
+        # _ListEditChildWidget is technically a ValueWidget.
+        self.insert(i, widget)  # type: ignore
 
         widget.changed.disconnect()
 
         # Value must be set after new widget is inserted because it could be
         # valid only after same parent is shared between widgets.
         if value is Undefined and i > 0:
+            # copy value from the previous child widget if possible
             value = self[i - 1].value
         if value is not Undefined:
             widget.value = value
 
         widget.changed.connect(lambda: self.changed.emit(self.value))
         self.changed.emit(self.value)
 
-    def _pop_value(self) -> None:
-        """Delete last child value widget."""
-        with contextlib.suppress(IndexError):
-            self.pop(-3)
-
     @property
     def value(self) -> list[_V]:
         """Return current value as a list object."""
@@ -749,7 +761,7 @@ def value(self) -> list[_V]:
     @value.setter
     def value(self, vals: Iterable[_V]) -> None:
         with self.changed.blocked():
-            del self[:-2]
+            del self[:-1]
             for v in vals:
                 self._append_value(v)
         self.changed.emit(self.value)
@@ -769,7 +781,7 @@ class ListDataView(Generic[_V]):
 
     def __init__(self, obj: ListEdit[_V]):
         self._obj = obj
-        self._widgets = list(obj[:-2])
+        self._widgets = list(obj[:-1])
 
     def __repr__(self) -> str:
         """Return list-like representation."""

tests/test_widgets.py

@@ -849,23 +849,23 @@ def test_list_edit():
     assert mock.call_count == 1
     mock.assert_called_with([1, 2, 3, 3])
 
-    list_edit.btn_minus.changed()
-    assert list_edit.value == [1, 2, 3]
-    assert list_edit.data == [1, 2, 3]
+    list_edit[1].btn_minus.changed()
+    assert list_edit.value == [1, 3, 3]
+    assert list_edit.data == [1, 3, 3]
     assert mock.call_count == 2
-    mock.assert_called_with([1, 2, 3])
+    mock.assert_called_with([1, 3, 3])
 
     list_edit.data[0] = 0
-    assert list_edit.value == [0, 2, 3]
-    assert list_edit.data == [0, 2, 3]
+    assert list_edit.value == [0, 3, 3]
+    assert list_edit.data == [0, 3, 3]
     assert mock.call_count == 3
-    mock.assert_called_with([0, 2, 3])
+    mock.assert_called_with([0, 3, 3])
 
     list_edit[0].value = 10
-    assert list_edit.value == [10, 2, 3]
-    assert list_edit.data == [10, 2, 3]
+    assert list_edit.value == [10, 3, 3]
+    assert list_edit.data == [10, 3, 3]
     assert mock.call_count == 4
-    mock.assert_called_with([10, 2, 3])
+    mock.assert_called_with([10, 3, 3])
 
     list_edit.data[:2] = [6, 5]  # type: ignore
     assert list_edit.value == [6, 5, 3]
@@ -912,10 +912,10 @@ def f3(x: List[int] = [0]):  # noqa: B006
         pass
 
     assert type(f3.x) is widgets.ListEdit
-    assert type(f3.x[0]) is widgets.Slider
-    assert f3.x[0].min == -10
-    assert f3.x[0].max == 10
-    assert f3.x[0].step == 5
+    assert type(f3.x[0].value_widget) is widgets.Slider
+    assert f3.x[0].value_widget.min == -10
+    assert f3.x[0].value_widget.max == 10
+    assert f3.x[0].value_widget.step == 5
 
     @magicgui
     def f4(x: List[int] = ()):  # type: ignore
@@ -926,7 +926,7 @@ def f4(x: List[int] = ()):  # type: ignore
     assert f4.x._args_type is int
     assert f4.x.value == []
     f4.x.btn_plus.changed()
-    assert type(f4.x[0]) is widgets.SpinBox
+    assert type(f4.x[0].value_widget) is widgets.SpinBox
     assert f4.x.value == [0]
 
     @magicgui
@@ -936,7 +936,7 @@ def f5(x: List[Annotated[int, {"max": 3}]]):
     assert type(f5.x) is widgets.ListEdit
     assert f5.x.annotation == List[int]
     f5.x.btn_plus.changed()
-    assert f5.x[0].max == 3
+    assert f5.x[0].value_widget.max == 3
 
 
 def test_tuple_edit():