Merge pull request #1095 from davep/mount-relative

Add support for mounting widgets relative to other widgets

src/textual/_node_list.py

@@ -39,6 +39,20 @@ class NodeList(Sequence):
     def __contains__(self, widget: Widget) -> bool:
         return widget in self._nodes
 
+    def index(self, widget: Widget) -> int:
+        """Return the index of the given widget.
+
+        Args:
+            widget (Widget): The widget to find in the node list.
+
+        Returns:
+            int: The index of the widget in the node list.
+
+        Raises:
+            ValueError: If the widget is not in the node list.
+        """
+        return self._nodes.index(widget)
+
     def _append(self, widget: Widget) -> None:
         """Append a Widget.
 
@@ -50,6 +64,17 @@ class NodeList(Sequence):
             self._nodes_set.add(widget)
             self._updates += 1
 
+    def _insert(self, index: int, widget: Widget) -> None:
+        """Insert a Widget.
+
+        Args:
+            widget (Widget): A widget.
+        """
+        if widget not in self._nodes_set:
+            self._nodes.insert(index, widget)
+            self._nodes_set.add(widget)
+            self._updates += 1
+
     def _remove(self, widget: Widget) -> None:
         """Remove a widget from the list.
 

src/textual/app.py

@@ -692,9 +692,13 @@ class App(Generic[ReturnType], DOMNode):
         # Wait until the app has performed all startup routines.
         await app_ready_event.wait()
 
-        # Context manager returns pilot object to manipulate the app
-        yield Pilot(app)
+        # Get the app in an active state.
+        app._set_active()
 
+        # Context manager returns pilot object to manipulate the app
+        try:
+            yield Pilot(app)
+        finally:
             # Shutdown the app cleanly
             await app._shutdown()
             await app_task
@@ -840,27 +844,55 @@ class App(Generic[ReturnType], DOMNode):
         self._require_stylesheet_update.add(self.screen if node is None else node)
         self.check_idle()
 
-    def mount(self, *widgets: Widget) -> AwaitMount:
-        """Mount the given widgets.
+    def mount(
+        self,
+        *widgets: Widget,
+        before: int | str | Widget | None = None,
+        after: int | str | Widget | None = None,
+    ) -> AwaitMount:
+        """Mount the given widgets relative to the app's screen.
 
         Args:
             *widgets (Widget): The widget(s) to mount.
+            before (int | str | Widget, optional): Optional location to mount before.
+            after (int | str | Widget, optional): Optional location to mount after.
 
         Returns:
             AwaitMount: An awaitable object that waits for widgets to be mounted.
-        """
-        return AwaitMount(self._register(self.screen, *widgets))
 
-    def mount_all(self, widgets: Iterable[Widget]) -> AwaitMount:
+        Raises:
+            MountError: If there is a problem with the mount request.
+
+        Note:
+            Only one of ``before`` or ``after`` can be provided. If both are
+            provided a ``MountError`` will be raised.
+        """
+        return self.screen.mount(*widgets, before=before, after=after)
+
+    def mount_all(
+        self,
+        widgets: Iterable[Widget],
+        before: int | str | Widget | None = None,
+        after: int | str | Widget | None = None,
+    ) -> AwaitMount:
         """Mount widgets from an iterable.
 
         Args:
             widgets (Iterable[Widget]): An iterable of widgets.
+            before (int | str | Widget, optional): Optional location to mount before.
+            after (int | str | Widget, optional): Optional location to mount after.
 
         Returns:
             AwaitMount: An awaitable object that waits for widgets to be mounted.
+
+        Raises:
+            MountError: If there is a problem with the mount request.
+
+        Note:
+            Only one of ``before`` or ``after`` can be provided. If both are
+            provided a ``MountError`` will be raised.
         """
-        return self.mount(*widgets)
+        return self.mount(*widgets, before=before, after=after)
 
     def is_screen_installed(self, screen: Screen | str) -> bool:
         """Check if a given screen has been installed.
@@ -1308,23 +1340,68 @@ class App(Generic[ReturnType], DOMNode):
             self._require_stylesheet_update.clear()
             self.stylesheet.update_nodes(nodes, animate=True)
 
-    def _register_child(self, parent: DOMNode, child: Widget) -> bool:
+    def _register_child(
+        self, parent: DOMNode, child: Widget, before: int | None, after: int | None
+    ) -> None:
+        """Register a widget as a child of another.
+
+        Args:
+            parent (DOMNode): Parent node.
+            child (Widget): The child widget to register.
+            widgets: The widget to register.
+            before (int, optional): A location to mount before.
+            after (int, option): A location to mount after.
+        """
+
+        # Let's be 100% sure that we've not been asked to do a before and an
+        # after at the same time. It's possible that we can remove this
+        # check later on, but for the purposes of development right now,
+        # it's likely a good idea to keep it here to check assumptions in
+        # the rest of the code.
+        if before is not None and after is not None:
+            raise AppError("Only one of 'before' and 'after' may be specified.")
+
+        # If we don't already know about this widget...
         if child not in self._registry:
+
+            # Now to figure out where to place it. If we've got a `before`...
+            if before is not None:
+                # ...it's safe to NodeList._insert before that location.
+                parent.children._insert(before, child)
+            elif after is not None and after != -1:
+                # In this case we've got an after. -1 holds the special
+                # position (for now) of meaning "okay really what I mean is
+                # do an append, like if I'd asked to add with no before or
+                # after". So... we insert before the next item in the node
+                # list, iff after isn't -1.
+                parent.children._insert(after + 1, child)
+            else:
+                # At this point we appear to not be adding before or after,
+                # or we've got a before/after value that really means
+                # "please append". So...
                 parent.children._append(child)
+
+            # Now that the widget is in the NodeList of its parent, sort out
+            # the rest of the admin.
             self._registry.add(child)
             child._attach(parent)
             child._post_register(self)
             child._start_messages()
-            return True
-        return False
 
-    def _register(self, parent: DOMNode, *widgets: Widget) -> list[Widget]:
+    def _register(
+        self,
+        parent: DOMNode,
+        *widgets: Widget,
+        before: int | None = None,
+        after: int | None = None,
+    ) -> list[Widget]:
         """Register widget(s) so they may receive events.
 
         Args:
             parent (DOMNode): Parent node.
             *widgets: The widget(s) to register.
-
+            before (int, optional): A location to mount before.
+            after (int, option): A location to mount after.
         Returns:
             list[Widget]: List of modified widgets.
 
@@ -1333,13 +1410,19 @@ class App(Generic[ReturnType], DOMNode):
         if not widgets:
             return []
 
-        apply_stylesheet = self.stylesheet.apply
+        new_widgets = list(widgets)
+        if before is not None or after is not None:
+            # There's a before or after, which means there's going to be an
+            # insertion, so make it easier to get the new things in the
+            # correct order.
+            new_widgets = reversed(new_widgets)
 
-        for widget in widgets:
+        apply_stylesheet = self.stylesheet.apply
+        for widget in new_widgets:
             if not isinstance(widget, Widget):
                 raise AppError(f"Can't register {widget!r}; expected a Widget instance")
             if widget not in self._registry:
-                self._register_child(parent, widget)
+                self._register_child(parent, widget, before, after)
                 if widget.children:
                     self._register(widget, *widget.children)
                 apply_stylesheet(widget)

src/textual/widget.py

@@ -374,16 +374,100 @@ class Widget(DOMNode):
         if self._scrollbar_corner is not None:
             yield self._scrollbar_corner
 
-    def mount(self, *widgets: Widget) -> AwaitMount:
-        """Mount child widgets (making this widget a container).
+    class MountError(Exception):
+        """Error raised when there was a problem with the mount request."""
+
+    def _find_mount_point(self, spot: int | str | "Widget") -> tuple["Widget", int]:
+        """Attempt to locate the point where the caller wants to mount something.
+
+        Args:
+            spot (int | str | Widget): The spot to find.
+
+        Returns:
+            tuple[Widget, int]: The parent and the location in its child list.
+
+        Raises:
+            Widget.MountError: If there was an error finding where to mount a widget.
+
+        The rules of this method are:
+
+        - Given an ``int``, parent is ``self`` and location is the integer value.
+        - Given a ``Widget``, parent is the widget's parent and location is
+          where the widget is found in the parent's ``children``. If it
+          can't be found a ``MountError`` will be raised.
+        - Given a string, it is used to perform a ``query_one`` and then the
+          result is used as if a ``Widget`` had been given.
+        """
+
+        # A numeric location means at that point in our child list.
+        if isinstance(spot, int):
+            return self, spot
+
+        # If we've got a string, that should be treated like a query that
+        # can be passed to query_one. So let's use that to get a widget to
+        # work on.
+        if isinstance(spot, str):
+            spot = self.query_one(spot, Widget)
+
+        # At this point we should have a widget, either because we got given
+        # one, or because we pulled one out of the query. First off, does it
+        # have a parent? There's no way we can use it as a sibling to make
+        # mounting decisions if it doesn't have a parent.
+        if spot.parent is None:
+            raise self.MountError(
+                f"Unable to find relative location of {spot!r} because it has no parent"
+            )
+
+        # We've got a widget. It has a parent. It has (zero or more)
+        # children. We should be able to go looking for the widget's
+        # location amongst its parent's children.
+        try:
+            return spot.parent, spot.parent.children.index(spot)
+        except ValueError:
+            raise self.MountError(f"{spot!r} is not a child of {self!r}") from None
+
+    def mount(
+        self,
+        *widgets: Widget,
+        before: int | str | Widget | None = None,
+        after: int | str | Widget | None = None,
+    ) -> AwaitMount:
+        """Mount widgets below this widget (making this widget a container).
 
         Args:
             *widgets (Widget): The widget(s) to mount.
+            before (int | str | Widget, optional): Optional location to mount before.
+            after (int | str | Widget, optional): Optional location to mount after.
 
         Returns:
             AwaitMount: An awaitable object that waits for widgets to be mounted.
+
+        Raises:
+            MountError: If there is a problem with the mount request.
+
+        Note:
+            Only one of ``before`` or ``after`` can be provided. If both are
+            provided a ``MountError`` will be raised.
         """
-        return AwaitMount(self.app._register(self, *widgets))
+
+        # Saying you want to mount before *and* after something is an error.
+        if before is not None and after is not None:
+            raise self.MountError(
+                "Only one of `before` or `after` can be handled -- not both"
+            )
+
+        # Decide the final resting place depending on what we've been asked
+        # to do.
+        if before is not None:
+            parent, before = self._find_mount_point(before)
+        elif after is not None:
+            parent, after = self._find_mount_point(after)
+        else:
+            parent = self
+
+        return AwaitMount(
+            self.app._register(parent, *widgets, before=before, after=after)
+        )
 
     def compose(self) -> ComposeResult:
         """Called by Textual to create child widgets.

tests/test_node_list.py

@@ -1,3 +1,5 @@
+import pytest
+
 from textual.widget import Widget
 from textual._node_list import NodeList
 
@@ -19,6 +21,16 @@ def test_repeat_add_one():
         nodes._append(widget)
     assert len(nodes)==1
 
+def test_insert():
+    nodes = NodeList()
+    widget1 = Widget()
+    widget2 = Widget()
+    widget3 = Widget()
+    nodes._append(widget1)
+    nodes._append(widget3)
+    nodes._insert(1,widget2)
+    assert list(nodes) == [widget1,widget2,widget3]
+
 def test_truthy():
     """Does a node list act as a truthy object?"""
     nodes = NodeList()
@@ -35,6 +47,15 @@ def test_contains():
     assert widget in nodes
     assert Widget() not in nodes
 
+def test_index():
+    """Can we get the index of a widget in the list?"""
+    widget = Widget()
+    nodes = NodeList()
+    with pytest.raises(ValueError):
+        _ = nodes.index(widget)
+    nodes._append(widget)
+    assert nodes.index(widget) == 0
+
 def test_remove():
     """Can we remove a widget we've added?"""
     widget = Widget()

tests/test_widget_mount_point.py

@@ -0,0 +1,40 @@
+import pytest
+
+from textual.widget import Widget
+
+
+class Content(Widget):
+    pass
+
+
+class Body(Widget):
+    pass
+
+
+def test_find_dom_spot():
+
+    # Build up a "fake" DOM for an application.
+    screen = Widget(name="Screen")
+    header = Widget(name="Header", id="header")
+    body = Body(id="body")
+    content = [Content(id=f"item{n}") for n in range(1000)]
+    body._add_children(*content)
+    footer = Widget(name="Footer", id="footer")
+    screen._add_children(header, body, footer)
+
+    # Just as a quick double-check, make sure the main components are in
+    # their intended place.
+    assert list(screen.children) == [header, body, footer]
+
+    # Now check that we find what we're looking for in the places we expect
+    # to find them.
+    assert screen._find_mount_point(1) == (screen, 1)
+    assert screen._find_mount_point(body) == screen._find_mount_point(1)
+    assert screen._find_mount_point("Body") == screen._find_mount_point(body)
+    assert screen._find_mount_point("#body") == screen._find_mount_point(1)
+
+    # Finally, let's be sure that we get an error if, for some odd reason,
+    # we go looking for a widget that isn't actually part of the DOM we're
+    # looking in.
+    with pytest.raises(Widget.MountError):
+        _ = screen._find_mount_point(Widget())

tests/test_widget_mounting.py

@@ -0,0 +1,113 @@
+import pytest
+
+from textual.app import App
+from textual.widgets import Static
+
+async def test_mount_via_app() -> None:
+    """Perform mount tests via the app."""
+
+    # Make a background set of widgets.
+    widgets = [Static(id=f"starter-{n}") for n in range( 10 )]
+
+    async with App().run_test() as pilot:
+        # Mount the first one and make sure it's there.
+        await pilot.app.mount(widgets[0])
+        assert len(pilot.app.screen.children) == 1
+        assert pilot.app.screen.children[0] == widgets[0]
+
+        # Mount the next 2 widgets via mount.
+        await pilot.app.mount(*widgets[1:3])
+        assert list(pilot.app.screen.children) == widgets[0:3]
+
+        # Finally mount the rest of the widgets via mount_all.
+        await pilot.app.mount_all(widgets[3:])
+        assert list(pilot.app.screen.children) == widgets
+
+    async with App().run_test() as pilot:
+        # Mount a widget before -1, which is "before the end".
+        penultimate = Static(id="penultimate")
+        await pilot.app.mount_all(widgets)
+        await pilot.app.mount(penultimate, before=-1)
+        assert pilot.app.screen.children[-2] == penultimate
+
+    async with App().run_test() as pilot:
+        # Mount a widget after -1, which is "at the end".
+        ultimate = Static(id="ultimate")
+        await pilot.app.mount_all(widgets)
+        await pilot.app.mount(ultimate, after=-1)
+        assert pilot.app.screen.children[-1] == ultimate
+
+    async with App().run_test() as pilot:
+        # Mount a widget before -2, which is "before the penultimate".
+        penpenultimate = Static(id="penpenultimate")
+        await pilot.app.mount_all(widgets)
+        await pilot.app.mount(penpenultimate, before=-2)
+        assert pilot.app.screen.children[-3] == penpenultimate
+
+    async with App().run_test() as pilot:
+        # Mount a widget after -2, which is "before the end".
+        penultimate = Static(id="penultimate")
+        await pilot.app.mount_all(widgets)
+        await pilot.app.mount(penultimate, after=-2)
+        assert pilot.app.screen.children[-2] == penultimate
+
+    async with App().run_test() as pilot:
+        # Mount a widget before 0, which is "at the start".
+        start = Static(id="start")
+        await pilot.app.mount_all(widgets)
+        await pilot.app.mount(start, before=0)
+        assert pilot.app.screen.children[0] == start
+
+    async with App().run_test() as pilot:
+        # Mount a widget after 0. You get the idea...
+        second = Static(id="second")
+        await pilot.app.mount_all(widgets)
+        await pilot.app.mount(second, after=0)
+        assert pilot.app.screen.children[1] == second
+
+    async with App().run_test() as pilot:
+        # Mount a widget relative to another via query.
+        queue_jumper = Static(id="queue-jumper")
+        await pilot.app.mount_all(widgets)
+        await pilot.app.mount(queue_jumper, after="#starter-5")
+        assert pilot.app.screen.children[6] == queue_jumper
+
+    async with App().run_test() as pilot:
+        # Mount a widget relative to another via query.
+        queue_jumper = Static(id="queue-jumper")
+        await pilot.app.mount_all(widgets)
+        await pilot.app.mount(queue_jumper, after=widgets[5])
+        assert pilot.app.screen.children[6] == queue_jumper
+
+    async with App().run_test() as pilot:
+        # Make sure we get told off for trying to before and after.
+        await pilot.app.mount_all(widgets)
+        with pytest.raises(Static.MountError):
+            await pilot.app.mount(Static(), before=2, after=2)
+
+    async with App().run_test() as pilot:
+        # Make sure we get told off trying to mount relative to something
+        # that isn't actually in the DOM.
+        await pilot.app.mount_all(widgets)
+        with pytest.raises(Static.MountError):
+            await pilot.app.mount(Static(), before=Static())
+        with pytest.raises(Static.MountError):
+            await pilot.app.mount(Static(), after=Static())
+
+    # TODO: At the moment query_one() simply takes a query and returns the
+    # .first() item. As such doing a query_one() that gets more than one
+    # thing isn't an error, it just skims off the first thing. OTOH the
+    # intention of before= and after= with a selector is that an exception
+    # will be thrown -- the exception being the own that should be thrown
+    # from query_one(). So, this test here is a TODO test because we'll wait
+    # for a change to query_one() and then its exception will just bubble
+    # up.
+    #
+    # See https://github.com/Textualize/textual/issues/1096
+    #
+    # async with App().run_test() as pilot:
+    #     # Make sure we get an error if we try and mount with a selector that
+    #     # results in more than one hit.
+    #     await pilot.app.mount_all(widgets)
+    #     with pytest.raises( ?Something? ):
+    #         await pilot.app.mount(Static(), before="Static")