Merge pull request #653 from Textualize/add-remove

Add remove

sandbox/will/add_remove.py

@@ -0,0 +1,71 @@
+import random
+
+from textual import layout
+from textual.app import App, ComposeResult
+from textual.widgets import Button, Static
+
+
+class Thing(Static):
+    def on_show(self) -> None:
+        self.scroll_visible()
+
+
+class AddRemoveApp(App):
+    CSS = """
+    #buttons {
+        dock: top;
+        height: auto;
+    }
+    Button {
+        width: 1fr;
+    }
+    #items {
+        height: 100%;
+        overflow-y: scroll;
+    }
+    Thing {
+        height: 5;
+        background: $panel;
+        border: wide $primary;
+        margin: 0 1;
+        content-align: center middle;
+    }
+    """
+
+    def on_mount(self) -> None:
+        self.count = 0
+
+    def compose(self) -> ComposeResult:
+        yield layout.Vertical(
+            layout.Horizontal(
+                Button("Add", variant="success", id="add"),
+                Button("Remove", variant="error", id="remove"),
+                Button("Remove random", variant="warning", id="remove_random"),
+                id="buttons",
+            ),
+            layout.Vertical(id="items"),
+        )
+
+    def handle_pressed(self, event: Button.Pressed) -> None:
+        if event.button.id == "add":
+            self.count += 1
+            self.query("#items").first().mount(
+                Thing(f"Thing {self.count}", id=f"thing{self.count}")
+            )
+        elif event.button.id == "remove":
+            things = self.query("#items Thing")
+            if things:
+                things.last().remove()
+
+        elif event.button.id == "remove_random":
+            things = self.query("#items Thing")
+            if things:
+                random.choice(things).remove()
+
+        self.app.bell()
+
+
+app = AddRemoveApp()
+
+if __name__ == "__main__":
+    app.run()

src/textual/_node_list.py

@@ -40,12 +40,31 @@ class NodeList:
         return widget in self._nodes
 
     def _append(self, widget: Widget) -> None:
+        """Append a Widget.
+
+        Args:
+            widget (Widget): A widget.
+        """
         if widget not in self._nodes_set:
             self._nodes.append(widget)
             self._nodes_set.add(widget)
             self._updates += 1
 
+    def _remove(self, widget: Widget) -> None:
+        """Remove a widget from the list.
+
+        Removing a widget not in the list is a null-op.
+
+        Args:
+            widget (Widget): A Widget in the list.
+        """
+        if widget in self._nodes_set:
+            del self._nodes[self._nodes.index(widget)]
+            self._nodes_set.remove(widget)
+            self._updates += 1
+
     def _clear(self) -> None:
+        """Clear the node list."""
         if self._nodes:
             self._nodes.clear()
             self._nodes_set.clear()

src/textual/app.py

@@ -603,7 +603,7 @@ class App(Generic[ReturnType], DOMNode):
         as keyword args they will be assigned an id of the key.
 
         """
-        self.register(self.screen, *anon_widgets, **widgets)
+        self._register(self.screen, *anon_widgets, **widgets)
 
     def mount_all(self, widgets: Iterable[Widget]) -> None:
         """Mount widgets from an iterable.
@@ -612,7 +612,7 @@ class App(Generic[ReturnType], DOMNode):
             widgets (Iterable[Widget]): An iterable of widgets.
         """
         for widget in widgets:
-            self.register(self.screen, widget)
+            self._register(self.screen, widget)
 
     def push_screen(self, screen: Screen | None = None) -> Screen:
         """Push a new screen on the screen stack.
@@ -856,7 +856,7 @@ class App(Generic[ReturnType], DOMNode):
             return True
         return False
 
-    def register(
+    def _register(
         self, parent: DOMNode, *anon_widgets: Widget, **widgets: Widget
     ) -> None:
         """Mount widget(s) so they may receive events.
@@ -878,12 +878,22 @@ class App(Generic[ReturnType], DOMNode):
                     widget.id = widget_id
                 self._register_child(parent, widget)
                 if widget.children:
-                    self.register(widget, *widget.children)
+                    self._register(widget, *widget.children)
                 apply_stylesheet(widget)
 
         for _widget_id, widget in name_widgets:
             widget.post_message_no_wait(events.Mount(sender=parent))
 
+    def _unregister(self, widget: Widget) -> None:
+        """Unregister a widget.
+
+        Args:
+            widget (Widget): _description_
+        """
+        if isinstance(widget._parent, Widget):
+            widget._parent.children._remove(widget)
+        self.registry.discard(widget)
+
     async def _disconnect_devtools(self):
         await self.devtools.disconnect()
 
@@ -906,9 +916,6 @@ class App(Generic[ReturnType], DOMNode):
             child = self.registry.pop()
             await child.close_messages()
 
-    async def remove(self, child: MessagePump) -> None:
-        self.registry.remove(child)
-
     async def shutdown(self):
         await self._disconnect_devtools()
         driver = self._driver
@@ -1009,7 +1016,7 @@ class App(Generic[ReturnType], DOMNode):
         # If the event has been forwarded it may have bubbled up back to the App
         if isinstance(event, events.Mount):
             screen = Screen()
-            self.register(self, screen)
+            self._register(self, screen)
             self.push_screen(screen)
             await super().on_event(event)
 

src/textual/css/query.py

@@ -16,7 +16,7 @@ from __future__ import annotations
 
 import rich.repr
 
-from typing import Iterator, TYPE_CHECKING
+from typing import Iterator, overload, TYPE_CHECKING
 
 
 from .match import match
@@ -24,6 +24,7 @@ from .parse import parse_selectors
 
 if TYPE_CHECKING:
     from ..dom import DOMNode
+    from ..widget import Widget
 
 
 class NoMatchingNodesError(Exception):
@@ -36,14 +37,16 @@ class DOMQuery:
         self,
         node: DOMNode | None = None,
         selector: str | None = None,
-        nodes: list[DOMNode] | None = None,
+        nodes: list[Widget] | None = None,
     ) -> None:
+        from ..widget import Widget
+
         self._selector = selector
-        self._nodes: list[DOMNode] = []
+        self._nodes: list[Widget] = []
         if nodes is not None:
             self._nodes = nodes
         elif node is not None:
-            self._nodes = list(node.walk_children())
+            self._nodes = [node for node in node.walk_children()]
         else:
             self._nodes = []
 
@@ -58,9 +61,20 @@ class DOMQuery:
         """True if non-empty, otherwise False."""
         return bool(self._nodes)
 
-    def __iter__(self) -> Iterator[DOMNode]:
+    def __iter__(self) -> Iterator[Widget]:
         return iter(self._nodes)
 
+    @overload
+    def __getitem__(self, index: int) -> Widget:
+        ...
+
+    @overload
+    def __getitem__(self, index: slice) -> list[Widget]:
+        ...
+
+    def __getitem__(self, index: int | slice) -> Widget | list[Widget]:
+        return self._nodes[index]
+
     def __rich_repr__(self) -> rich.repr.Result:
         yield self._nodes
 
@@ -73,6 +87,7 @@ class DOMQuery:
         Returns:
             DOMQuery: New DOM Query.
         """
+
         selector_set = parse_selectors(selector)
         query = DOMQuery(
             nodes=[_node for _node in self._nodes if match(selector_set, _node)]
@@ -94,7 +109,7 @@ class DOMQuery:
         )
         return query
 
-    def first(self) -> DOMNode:
+    def first(self) -> Widget:
         """Get the first matched node.
 
         Returns:
@@ -107,6 +122,19 @@ class DOMQuery:
                 f"No nodes match the selector {self._selector!r}"
             )
 
+    def last(self) -> Widget:
+        """Get the last matched node.
+
+        Returns:
+            DOMNode: A DOM Node.
+        """
+        if self._nodes:
+            return self._nodes[-1]
+        else:
+            raise NoMatchingNodesError(
+                f"No nodes match the selector {self._selector!r}"
+            )
+
     def add_class(self, *class_names: str) -> DOMQuery:
         """Add the given class name(s) to nodes."""
         for node in self._nodes:
@@ -125,6 +153,12 @@ class DOMQuery:
             node.toggle_class(*class_names)
         return self
 
+    def remove(self) -> DOMQuery:
+        """Remove matched nodes from the DOM"""
+        for node in self._nodes:
+            node.remove()
+        return self
+
     def set_styles(self, css: str | None = None, **styles: str) -> DOMQuery:
         """Set styles on matched nodes.
 

src/textual/dom.py

@@ -254,7 +254,7 @@ class DOMNode(MessagePump):
         """
         Returns: ``True`` if this DOMNode is displayed (``display != "none"``), ``False`` otherwise.
         """
-        return self.styles.display != "none"
+        return self.styles.display != "none" and not (self._closing or self._closed)
 
     @display.setter
     def display(self, new_val: bool | str) -> None:

src/textual/events.py

@@ -130,6 +130,10 @@ class Unmount(Event, bubble=False):
     """Sent when a widget is unmounted, and may no longer receive messages."""
 
 
+class Remove(Event, bubble=False):
+    """Sent to a widget to ask it to remove itself from the DOM."""
+
+
 class Show(Event, bubble=False):
     """Sent when a widget has become visible."""
 

src/textual/message_pump.py

@@ -214,16 +214,17 @@ class MessagePump:
 
     async def close_messages(self) -> None:
         """Close message queue, and optionally wait for queue to finish processing."""
-        if self._closed:
+        if self._closed or self._closing:
             return
-
         self._closing = True
         await self._message_queue.put(MessagePriority(None))
-        for task in self._child_tasks:
+        cancel_tasks = list(self._child_tasks)
+        for task in cancel_tasks:
             task.cancel()
+        for task in cancel_tasks:
             await task
         self._child_tasks.clear()
-        if self._task is not None:
+        if self._task is not None and asyncio.current_task() != self._task:
             # Ensure everything is closed before returning
             await self._task
 

src/textual/widget.py

@@ -181,7 +181,17 @@ class Widget(DOMNode):
             self.scroll_to(0, 0, animate=False)
 
     def mount(self, *anon_widgets: Widget, **widgets: Widget) -> None:
-        self.app.register(self, *anon_widgets, **widgets)
+        """Mount child widgets (making this widget a container).
+
+        Widgets may be passed as positional arguments or keyword arguments. If keyword arguments,
+        the keys will be set as the Widget's id.
+
+        Example:
+            self.mount(Static("hello"), header=Header())
+
+
+        """
+        self.app._register(self, *anon_widgets, **widgets)
         self.screen.refresh()
 
     def compose(self) -> ComposeResult:
@@ -815,6 +825,17 @@ class Widget(DOMNode):
             )
         return delta
 
+    def scroll_visible(self) -> bool:
+        """Scroll the container to make this widget visible.
+
+        Returns:
+            bool: True if the parent was scrolled.
+        """
+        parent = self.parent
+        if isinstance(parent, Widget):
+            return parent.scroll_to_widget(self)
+        return False
+
     def __init_subclass__(
         cls,
         can_focus: bool = False,
@@ -1051,6 +1072,10 @@ class Widget(DOMNode):
 
         self.check_idle()
 
+    def remove(self) -> None:
+        """Remove the Widget from the DOM (effectively deleting it)"""
+        self.post_message_no_wait(events.Remove(self))
+
     def render(self) -> RenderableType:
         """Get renderable for widget.
 
@@ -1122,6 +1147,12 @@ class Widget(DOMNode):
     async def on_key(self, event: events.Key) -> None:
         await self.dispatch_key(event)
 
+    async def on_remove(self, event: events.Remove) -> None:
+        await self.close_messages()
+        self.app._unregister(self)
+        assert self.parent
+        self.parent.refresh(layout=True)
+
     def on_mount(self, event: events.Mount) -> None:
         widgets = list(self.compose())
         if widgets: