Ensure that widget IDs can be reused.

changes/2190.bugfix.rst

@@ -0,0 +1 @@
+New widgets with an ID matching an ID that was previously used no longer cause an error.

changes/2190.feature.1.rst

@@ -0,0 +1 @@
+Widgets and Windows can now be sorted. The ID of the widget is used for the sorting order.

changes/2190.feature.2.rst

@@ -0,0 +1 @@
+The main window generated by the default ``startup()`` method of an app now has an ID of ``main``.

changes/2190.removal.rst

@@ -0,0 +1 @@
+Widgets must now be added to a window to appear in widget ID lookups.

core/src/toga/app.py

@@ -6,18 +6,27 @@
 import sys
 import warnings
 import webbrowser
-from collections.abc import Collection, Iterator, Mapping, MutableSet
+from collections.abc import (
+    Collection,
+    ItemsView,
+    Iterator,
+    KeysView,
+    Mapping,
+    MutableSet,
+    ValuesView,
+)
 from email.message import Message
 from typing import TYPE_CHECKING, Any, Protocol
 from warnings import warn
+from weakref import WeakValueDictionary
 
 from toga.command import Command, CommandSet
 from toga.documents import Document
 from toga.handlers import wrapped_handler
 from toga.icons import Icon
 from toga.paths import Paths
 from toga.platform import get_platform_factory
-from toga.widgets.base import Widget, WidgetRegistry
+from toga.widgets.base import Widget
 from toga.window import Window
 
 if TYPE_CHECKING:
@@ -131,6 +140,61 @@ def __len__(self) -> int:
         return len(self.elements)
 
 
+class WidgetRegistry:
+    # WidgetRegistry is implemented as a wrapper around a WeakValueDictionary, because
+    # it provides a mapping from ID to widget. The mapping is weak so the registry
+    # doesn't retain a strong reference to the widget, preventing memory cleanup.
+    #
+    # The lookup methods (__getitem__(), __iter__(), __len()__, keys(), items(), and
+    # values()) are all proxied to to underlying data store. Private methods exist for
+    # internal use, but those methods shouldn't be used by end-users.
+
+    def __init__(self, *args, **kwargs):
+        self._registry = WeakValueDictionary(*args, **kwargs)
+
+    def __len__(self) -> int:
+        return len(self._registry)
+
+    def __getitem__(self, key: str) -> Widget:
+        return self._registry[key]
+
+    def __contains__(self, key: str) -> bool:
+        return key in self._registry
+
+    def __iter__(self) -> Iterator[Widget]:
+        return self.values()
+
+    def __repr__(self) -> str:
+        return (
+            "{"
+            + ", ".join(f"{k!r}: {v!r}" for k, v in sorted(self._registry.items()))
+            + "}"
+        )
+
+    def items(self) -> ItemsView:
+        return self._registry.items()
+
+    def keys(self) -> KeysView:
+        return self._registry.keys()
+
+    def values(self) -> ValuesView:
+        return self._registry.values()
+
+    # Private methods for internal use
+    def _update(self, widgets: list[Widget]) -> None:
+        for widget in widgets:
+            self._add(widget)
+
+    def _add(self, widget: Widget) -> None:
+        if widget.id in self._registry:
+            # Prevent adding the same widget twice or adding 2 widgets with the same id
+            raise KeyError(f"There is already a widget with the id {widget.id!r}")
+        self._registry[widget.id] = widget
+
+    def _remove(self, id: str) -> None:
+        del self._registry[id]
+
+
 class MainWindow(Window):
     _WINDOW_CLASS = "MainWindow"
 
@@ -523,6 +587,10 @@ def widgets(self) -> Mapping[str, Widget]:
 
         Can be used to look up widgets by ID over the entire app (e.g.,
         ``app.widgets["my_id"]``).
+
+        Only returns widgets that are currently part of a layout. A widget that has been
+        created, but not assigned as part of window content will not be returned by
+        widget lookup.
         """
         return self._widgets
 
@@ -616,7 +684,7 @@ def startup(self) -> None:
         however, any override *must* ensure the :any:`main_window` has been assigned
         before it returns.
         """
-        self.main_window = MainWindow(title=self.formal_name)
+        self.main_window = MainWindow(title=self.formal_name, id="main")
 
         if self._startup_method:
             self.main_window.content = self._startup_method(self)

core/src/toga/widgets/base.py

@@ -1,8 +1,7 @@
 from __future__ import annotations
 
 from builtins import id as identifier
-from typing import TYPE_CHECKING, Iterator, NoReturn
-from weakref import WeakValueDictionary
+from typing import TYPE_CHECKING
 
 from travertino.node import Node
 
@@ -14,42 +13,6 @@
     from toga.window import Window
 
 
-class WidgetRegistry(WeakValueDictionary):
-    # WidgetRegistry is implemented as a subclass of WeakValueDictionary, because it
-    # provides a mapping from ID to widget. However, it exposes a set-like API; add()
-    # and update() take instances to be added, and iteration is over values. The
-    # mapping is weak so the registry doesn't retain a strong reference to the widget,
-    # preventing memory cleanup.
-
-    def __init__(self, *args, **kwargs):
-        super().__init__(*args, **kwargs)
-
-    def __setitem__(self, key: str, value: Widget) -> NoReturn:
-        # We do not want to allow setting items directly but to use the "add"
-        # method instead.
-        raise RuntimeError("Widgets cannot be directly added to a registry")
-
-    def update(self, widgets: list[Widget]) -> None:
-        for widget in widgets:
-            self.add(widget)
-
-    def add(self, widget: Widget) -> None:
-        if widget.id in self:
-            # Prevent from adding the same widget twice
-            # or adding 2 widgets with the same id
-            raise KeyError(f"There is already a widget with the id {widget.id!r}")
-        super().__setitem__(widget.id, widget)
-
-    def remove(self, id: str) -> None:
-        del self[id]
-
-    def __iter__(self) -> Iterator[Widget]:
-        return iter(self.values())
-
-    def __repr__(self) -> str:
-        return "{" + ", ".join(f"{k!r}: {v!r}" for k, v in self.items()) + "}"
-
-
 class Widget(Node):
     _MIN_WIDTH = 100
     _MIN_HEIGHT = 100
@@ -82,6 +45,9 @@ def __init__(
     def __repr__(self) -> str:
         return f"<{self.__class__.__name__}:0x{identifier(self):x}>"
 
+    def __lt__(self, other) -> bool:
+        return self.id < other.id
+
     @property
     def id(self) -> str:
         """The DOM identifier for the widget.
@@ -126,13 +92,16 @@ def add(self, *children: Widget) -> None:
                 if child.parent:
                     child.parent.remove(child)
 
-                # add to new parent
-                super().add(child)
-
-                # set app and window
+                # Set app and window. This is done *before* changing any parenting
+                # relationships, so that the widget registry can verify the widget ID is
+                # unique. App must be set before window to ensure the widget registry
+                # can be found.
                 child.app = self.app
                 child.window = self.window
 
+                # add to new parent
+                super().add(child)
+
                 self._impl.add_child(child._impl)
 
         # Whatever layout we're a part of needs to be refreshed
@@ -156,13 +125,16 @@ def insert(self, index: int, child: Widget) -> None:
             if child.parent:
                 child.parent.remove(child)
 
-            # add to new parent
-            super().insert(index, child)
-
-            # set app and window
+            # Set app and window. This is done *before* changing any parenting
+            # relationships, so that the widget registry can verify the widget ID is
+            # unique. App must be set before window to ensure the widget registry
+            # can be found.
             child.app = self.app
             child.window = self.window
 
+            # add to new parent
+            super().insert(index, child)
+
             self._impl.insert_child(index, child._impl)
 
         # Whatever layout we're a part of needs to be refreshed
@@ -187,8 +159,10 @@ def remove(self, *children: Widget) -> None:
                 removed = True
                 super().remove(child)
 
-                child.app = None
+                # Remove from the window before removing from the app
+                # so that the widget can be removed from the app-level registry.
                 child.window = None
+                child.app = None
 
                 self._impl.remove_child(child._impl)
 
@@ -225,43 +199,39 @@ def app(self, app: App | None) -> None:
                 # If app is the same as the previous app, return
                 return
 
-            # Deregister the widget from the old app
-            self._app.widgets.remove(self.id)
-
         self._app = app
         self._impl.set_app(app)
         for child in self.children:
             child.app = app
 
-        if app is not None:
-            # Add this widget to the application widget registry
-            app.widgets.add(self)
-
     @property
     def window(self) -> Window | None:
         """The window to which this widget belongs.
 
         When setting the window for a widget, all children of this widget will be
         recursively assigned to the same window.
+
+        If the widget has a value for :any:`window`, it *must* also have a value for
+        :any:`app`.
         """
         return self._window
 
     @window.setter
     def window(self, window: Window | None) -> None:
-        # Remove the widget from the widget registry it is currently a part of
-        if self.window is not None:
-            self.window.widgets.remove(self.id)
+        if self.window is not None and window is None:
+            # If the widget is currently in the registry, but is being removed from a
+            # window, remove the widget from the widget registry
+            self.window.app.widgets._remove(self.id)
+        elif self.window is None and window is not None:
+            # If the widget is being assigned to a window for the first time, add it to the widget registry
+            window.app.widgets._add(self)
 
         self._window = window
         self._impl.set_window(window)
 
         for child in self.children:
             child.window = window
 
-        if window is not None:
-            # Add this widget to the window's widget registry
-            window.widgets.add(self)
-
     @property
     def enabled(self) -> bool:
         """Is the widget currently enabled? i.e., can the user interact with the widget?"""

core/src/toga/widgets/scrollcontainer.py

@@ -81,8 +81,9 @@ def content(self) -> Widget:
     @content.setter
     def content(self, widget):
         if self._content:
-            self._content.app = None
+            # Clear the window before the app so that registry entries can be cleared
             self._content.window = None
+            self._content.app = None
 
         if widget:
             widget.app = self.app