Add get_child_by_id and get_widget_by_id

CHANGELOG.md

@@ -7,16 +7,15 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 
 ## [0.5.0] - Unreleased
 
-
 ### Added
 
+- Add get_child_by_id and get_widget_by_id, remove get_child https://github.com/Textualize/textual/pull/1146
 - Add easing parameter to Widget.scroll_* methods https://github.com/Textualize/textual/pull/1144
 
 ### Changed
 
 - Watchers are now called immediately when setting the attribute if they are synchronous. https://github.com/Textualize/textual/pull/1145
 
-
 ## [0.4.0] - 2022-11-08
 
 https://textual.textualize.io/blog/2022/11/08/version-040/#version-040

src/textual/_node_list.py

@@ -21,6 +21,12 @@ def __init__(self) -> None:
         # The nodes in the list
         self._nodes: list[Widget] = []
         self._nodes_set: set[Widget] = set()
+
+        # We cache widgets by their IDs too for a quick lookup
+        # Note that only widgets with IDs are cached like this, so
+        # this cache will likely hold fewer values than self._nodes.
+        self._nodes_by_id: dict[str, Widget] = {}
+
         # Increments when list is updated (used for caching)
         self._updates = 0
 
@@ -53,6 +59,9 @@ def index(self, widget: Widget) -> int:
         """
         return self._nodes.index(widget)
 
+    def _get_by_id(self, widget_id: str) -> Widget | None:
+        return self._nodes_by_id.get(widget_id)
+
     def _append(self, widget: Widget) -> None:
         """Append a Widget.
 
@@ -62,6 +71,9 @@ def _append(self, widget: Widget) -> None:
         if widget not in self._nodes_set:
             self._nodes.append(widget)
             self._nodes_set.add(widget)
+            widget_id = widget.id
+            if widget_id is not None and widget_id not in self._nodes_by_id:
+                self._nodes_by_id[widget_id] = widget
             self._updates += 1
 
     def _insert(self, index: int, widget: Widget) -> None:
@@ -73,6 +85,9 @@ def _insert(self, index: int, widget: Widget) -> None:
         if widget not in self._nodes_set:
             self._nodes.insert(index, widget)
             self._nodes_set.add(widget)
+            widget_id = widget.id
+            if widget_id is not None and widget_id not in self._nodes_by_id:
+                self._nodes_by_id[widget_id] = widget
             self._updates += 1
 
     def _remove(self, widget: Widget) -> None:
@@ -86,13 +101,17 @@ def _remove(self, widget: Widget) -> None:
         if widget in self._nodes_set:
             del self._nodes[self._nodes.index(widget)]
             self._nodes_set.remove(widget)
+            widget_id = widget.id
+            if widget_id in self._nodes_by_id:
+                del self._nodes_by_id[widget_id]
             self._updates += 1
 
     def _clear(self) -> None:
         """Clear the node list."""
         if self._nodes:
             self._nodes.clear()
             self._nodes_set.clear()
+            self._nodes_by_id.clear()
             self._updates += 1
 
     def __iter__(self) -> Iterator[Widget]:

src/textual/app.py

@@ -892,7 +892,7 @@ async def _on_css_change(self) -> None:
     def render(self) -> RenderableType:
         return Blank(self.styles.background)
 
-    def get_child(self, id: str) -> DOMNode:
+    def get_child_by_id(self, id: str) -> Widget:
         """Shorthand for self.screen.get_child(id: str)
         Returns the first child (immediate descendent) of this DOMNode
         with the given ID.
@@ -906,7 +906,26 @@ def get_child(self, id: str) -> DOMNode:
         Raises:
             NoMatches: if no children could be found for this ID
         """
-        return self.screen.get_child(id)
+        return self.screen.get_child_by_id(id)
+
+    def get_widget_by_id(self, id: str) -> Widget:
+        """Shorthand for self.screen.get_widget_by_id(id)
+        Return the first descendant widget with the given ID.
+
+        Performs a breadth-first search rooted at the current screen.
+        It will not return the Screen if that matches the ID.
+        To get the screen, use `self.screen`.
+
+        Args:
+            id (str): The ID to search for in the subtree
+
+        Returns:
+            DOMNode: The first descendant encountered with this ID.
+
+        Raises:
+            NoMatches: if no children could be found for this ID
+        """
+        return self.screen.get_widget_by_id(id)
 
     def update_styles(self, node: DOMNode | None = None) -> None:
         """Request update of styles.

src/textual/dom.py

@@ -674,7 +674,7 @@ def walk_depth_first() -> Iterable[DOMNode]:
                         push(iter(node.children))
 
         def walk_breadth_first() -> Iterable[DOMNode]:
-            """Walk the tree breadth first (children first)."""
+            """Walk the tree breadth first (children first) (level order traversal)"""
             queue: deque[DOMNode] = deque()
             popleft = queue.popleft
             extend = queue.extend
@@ -700,23 +700,6 @@ def walk_breadth_first() -> Iterable[DOMNode]:
             nodes.reverse()
         return nodes
 
-    def get_child(self, id: str) -> DOMNode:
-        """Return the first child (immediate descendent) of this node with the given ID.
-
-        Args:
-            id (str): The ID of the child.
-
-        Returns:
-            DOMNode: The first child of this node with the ID.
-
-        Raises:
-            NoMatches: if no children could be found for this ID
-        """
-        for child in self.children:
-            if child.id == id:
-                return child
-        raise NoMatches(f"No child found with id={id!r}")
-
     ExpectType = TypeVar("ExpectType", bound="Widget")
 
     @overload

src/textual/widget.py

@@ -1,6 +1,7 @@
 from __future__ import annotations
 
 from asyncio import Lock, wait, create_task
+from collections import Counter
 from fractions import Fraction
 from itertools import islice
 from operator import attrgetter
@@ -41,6 +42,7 @@
 from ._types import Lines
 from .binding import NoBinding
 from .box_model import BoxModel, get_box_model
+from .css.query import NoMatches
 from .css.scalar import ScalarOffset
 from .dom import DOMNode, NoScreen
 from .geometry import Offset, Region, Size, Spacing, clamp
@@ -333,6 +335,44 @@ def offset(self) -> Offset:
     def offset(self, offset: Offset) -> None:
         self.styles.offset = ScalarOffset.from_offset(offset)
 
+    def get_child_by_id(self, id: str) -> Widget:
+        """Return the first child (immediate descendent) of this node with the given ID.
+
+        Args:
+            id (str): The ID of the child.
+
+        Returns:
+            DOMNode: The first child of this node with the ID.
+
+        Raises:
+            NoMatches: if no children could be found for this ID
+        """
+        child = self.children._get_by_id(id)
+        if child is not None:
+            return child
+        raise NoMatches(f"No child found with id={id!r}")
+
+    def get_widget_by_id(self, id: str) -> Widget:
+        """Return the first descendant widget with the given ID.
+        Performs a breadth-first search rooted at this node, but this node won't match
+        if it has a matching id.
+
+        Args:
+            id (str): The ID to search for in the subtree
+
+        Returns:
+            DOMNode: The first descendant encountered with this ID.
+
+        Raises:
+            NoMatches: if no children could be found for this ID
+        """
+        for child in self.walk_children(method="depth"):
+            try:
+                return child.get_child_by_id(id)
+            except NoMatches:
+                pass
+        raise NoMatches(f"No descendant found with id={id!r}")
+
     def get_component_rich_style(self, name: str) -> Style:
         """Get a *Rich* style for a component.
 
@@ -460,6 +500,31 @@ def mount(
             provided a ``MountError`` will be raised.
         """
 
+        # Check for duplicate IDs in the incoming widgets
+        ids_to_mount = [widget.id for widget in widgets if widget.id is not None]
+        unique_ids = set(ids_to_mount)
+        num_unique_ids = len(unique_ids)
+        num_widgets_with_ids = len(ids_to_mount)
+        if num_unique_ids != num_widgets_with_ids:
+            counter = Counter(widget.id for widget in widgets)
+            for widget_id, count in counter.items():
+                if count > 1:
+                    raise MountError(
+                        f"Tried to insert {count!r} widgets with the same ID {widget_id!r}. "
+                        f"Widget IDs must be unique."
+                    )
+
+        # Check for duplicate IDs between the widgets to mount and existing children.
+        child_ids = self.children._nodes_by_id.keys()
+        intersection = unique_ids.intersection(child_ids)
+        if intersection:
+            conflicting_id = intersection.pop()
+            conflicting_widget = self.get_child_by_id(conflicting_id)
+            raise MountError(
+                f"Tried to insert a widget with ID {conflicting_id!r}, but a widget {conflicting_widget!r} "
+                f"already exists with that ID. Widget IDs must be unique."
+            )
+
         # Saying you want to mount before *and* after something is an error.
         if before is not None and after is not None:
             raise MountError(

tests/test_dom.py

@@ -1,7 +1,6 @@
 import pytest
 
 from textual.css.errors import StyleValueError
-from textual.css.query import NoMatches
 from textual.dom import DOMNode, BadIdentifier
 
 
@@ -26,37 +25,6 @@ def test_display_set_invalid_value():
         node.display = "blah"
 
 
-@pytest.fixture
-def parent():
-    parent = DOMNode(id="parent")
-    child1 = DOMNode(id="child1")
-    child2 = DOMNode(id="child2")
-    grandchild1 = DOMNode(id="grandchild1")
-    child1._add_child(grandchild1)
-
-    parent._add_child(child1)
-    parent._add_child(child2)
-
-    yield parent
-
-
-def test_get_child_gets_first_child(parent):
-    child = parent.get_child(id="child1")
-    assert child.id == "child1"
-    assert child.get_child(id="grandchild1").id == "grandchild1"
-    assert parent.get_child(id="child2").id == "child2"
-
-
-def test_get_child_no_matching_child(parent):
-    with pytest.raises(NoMatches):
-        parent.get_child(id="doesnt-exist")
-
-
-def test_get_child_only_immediate_descendents(parent):
-    with pytest.raises(NoMatches):
-        parent.get_child(id="grandchild1")
-
-
 def test_validate():
     with pytest.raises(BadIdentifier):
         DOMNode(id="23")

tests/test_widget.py

@@ -1,9 +1,12 @@
 import pytest
+import rich
 
-from textual.app import App
+from textual.app import App, ComposeResult
 from textual.css.errors import StyleValueError
+from textual.css.query import NoMatches
+from textual.dom import DOMNode
 from textual.geometry import Size
-from textual.widget import Widget
+from textual.widget import Widget, MountError
 
 
 @pytest.mark.parametrize(
@@ -64,3 +67,92 @@ def render(self) -> str:
     height = widget3.get_content_height(Size(20, 20), Size(80, 24), width)
     assert width == 3
     assert height == 3
+
+
+class GetByIdApp(App):
+    def compose(self) -> ComposeResult:
+        grandchild1 = Widget(id="grandchild1")
+        child1 = Widget(grandchild1, id="child1")
+        child2 = Widget(id="child2")
+
+        yield Widget(
+            child1,
+            child2,
+            id="parent",
+        )
+
+
+@pytest.fixture
+async def hierarchy_app():
+    app = GetByIdApp()
+    async with app.run_test():
+        yield app
+
+
+@pytest.fixture
+async def parent(hierarchy_app):
+    yield hierarchy_app.get_widget_by_id("parent")
+
+
+def test_get_child_by_id_gets_first_child(parent):
+    child = parent.get_child_by_id(id="child1")
+    assert child.id == "child1"
+    assert child.get_child_by_id(id="grandchild1").id == "grandchild1"
+    assert parent.get_child_by_id(id="child2").id == "child2"
+
+
+def test_get_child_by_id_no_matching_child(parent):
+    with pytest.raises(NoMatches):
+        parent.get_child_by_id(id="doesnt-exist")
+
+
+def test_get_child_by_id_only_immediate_descendents(parent):
+    with pytest.raises(NoMatches):
+        parent.get_child_by_id(id="grandchild1")
+
+
+def test_get_widget_by_id_no_matching_child(parent):
+    with pytest.raises(NoMatches):
+        parent.get_widget_by_id(id="i-dont-exist")
+
+
+def test_get_widget_by_id_non_immediate_descendants(parent):
+    result = parent.get_widget_by_id("grandchild1")
+    assert result.id == "grandchild1"
+
+
+def test_get_widget_by_id_immediate_descendants(parent):
+    result = parent.get_widget_by_id("child1")
+    assert result.id == "child1"
+
+
+def test_get_widget_by_id_doesnt_return_self(parent):
+    with pytest.raises(NoMatches):
+        parent.get_widget_by_id("parent")
+
+
+def test_get_widgets_app_delegated(hierarchy_app, parent):
+    # Check that get_child_by_id finds the parent, which is a child of the default Screen
+    queried_parent = hierarchy_app.get_child_by_id("parent")
+    assert queried_parent is parent
+
+    # Check that the grandchild (descendant of the default screen) is found
+    grandchild = hierarchy_app.get_widget_by_id("grandchild1")
+    assert grandchild.id == "grandchild1"
+
+
+def test_widget_mount_ids_must_be_unique_mounting_all_in_one_go(parent):
+    widget1 = Widget(id="hello")
+    widget2 = Widget(id="hello")
+
+    with pytest.raises(MountError):
+        parent.mount(widget1, widget2)
+
+
+def test_widget_mount_ids_must_be_unique_mounting_multiple_calls(parent):
+    widget1 = Widget(id="hello")
+    widget2 = Widget(id="hello")
+
+    parent.mount(widget1)
+    with pytest.raises(MountError):
+        parent.mount(widget2)