introduce _on_delete(), refactoring, re-activate tests

nicegui/binding.py

@@ -2,7 +2,7 @@ import asyncio
 import time
 from collections import defaultdict
 from collections.abc import Mapping
-from typing import Any, Callable, DefaultDict, Dict, List, Optional, Set, Tuple, Type, Union
+from typing import Any, Callable, DefaultDict, Dict, Iterable, List, Optional, Set, Tuple, Type, Union
 
 from . import globals  # pylint: disable=redefined-builtin
 
@@ -107,7 +107,7 @@ class BindableProperty:
             self.on_change(owner, value)
 
 
-def remove(objects: List[Any], type_: Type) -> None:
+def remove(objects: Iterable[Any], type_: Type) -> None:
     active_links[:] = [
         (source_obj, source_name, target_obj, target_name, transform)
         for source_obj, source_name, target_obj, target_name, transform in active_links

nicegui/element.py

@@ -4,7 +4,7 @@ import inspect
 import re
 from copy import copy, deepcopy
 from pathlib import Path
-from typing import TYPE_CHECKING, Any, Callable, Dict, Iterator, List, Optional, Sequence, Union
+from typing import TYPE_CHECKING, Any, Callable, Dict, Iterable, Iterator, List, Optional, Sequence, Union
 
 from typing_extensions import Self
 
@@ -304,19 +304,24 @@ class Element(Visibility):
         target_id = globals._socket_id or self.client.id  # pylint: disable=protected-access
         outbox.enqueue_message('run_method', data, target_id)
 
-    def _collect_descendant_ids(self) -> List[int]:
-        ids: List[int] = [self.id]
+    def _collect_descendants(self, *, include_self: bool = False) -> List[Element]:
+        elements: List[Element] = [self] if include_self else []
         for child in self:
-            ids.extend(child._collect_descendant_ids())  # pylint: disable=protected-access
-        return ids
+            elements.extend(child._collect_descendants(include_self=True))  # pylint: disable=protected-access
+        return elements
+
+    @staticmethod
+    def _delete_elements(elements: Iterable[Element]) -> None:
+        binding.remove(elements, Element)
+        for element in elements:
+            element._deleted = True  # pylint: disable=protected-access
+            del element.client.elements[element.id]
+            outbox.enqueue_delete(element)
 
     def clear(self) -> None:
         """Remove all child elements."""
-        descendants = [self.client.elements[id] for id in self._collect_descendant_ids()[1:]]
-        binding.remove(descendants, Element)
-        for element in descendants:
-            element.delete()
-            del self.client.elements[element.id]
+        descendants = self._collect_descendants()
+        self._delete_elements(descendants)
         for slot in self.slots.values():
             slot.children.clear()
         self.update()
@@ -344,19 +349,25 @@ class Element(Visibility):
         if isinstance(element, int):
             children = list(self)
             element = children[element]
-        binding.remove([element], Element)
-        element.delete()
-        del self.client.elements[element.id]
-        for slot in self.slots.values():
-            slot.children[:] = [e for e in slot if e.id != element.id]
+        elements = element._collect_descendants(include_self=True)  # pylint: disable=protected-access
+        self._delete_elements(elements)
+        assert element.parent_slot is not None
+        element.parent_slot.children.remove(element)
         self.update()
 
     def delete(self) -> None:
-        """Perform cleanup when the element is deleted."""
-        self._deleted = True
-        outbox.enqueue_delete(self)
+        """Delete the element."""
+        self._delete_elements([self])
+        assert self.parent_slot is not None
+        self.parent_slot.children.remove(self)
+
+    def _on_delete(self) -> None:
+        """Called when the element is deleted.
+
+        This method can be overridden in subclasses to perform cleanup tasks.
+        """
 
     @property
-    def is_deleted(self) -> None:
+    def is_deleted(self) -> bool:
         """Whether the element has been deleted."""
         return self._deleted

nicegui/elements/scene.py

@@ -184,9 +184,9 @@ class Scene(Element,
                         self.camera.look_at_x, self.camera.look_at_y, self.camera.look_at_z,
                         self.camera.up_x, self.camera.up_y, self.camera.up_z, duration)
 
-    def delete(self) -> None:
+    def _on_delete(self) -> None:
         binding.remove(list(self.objects.values()), Object3D)
-        super().delete()
+        super()._on_delete()
 
     def delete_objects(self, predicate: Callable[[Object3D], bool] = lambda _: True) -> None:
         for obj in list(self.objects.values()):

nicegui/elements/upload.py

@@ -69,6 +69,6 @@ class Upload(DisableableElement, component='upload.js'):
     def reset(self) -> None:
         self.run_method('reset')
 
-    def delete(self) -> None:
+    def _on_delete(self) -> None:
         app.remove_route(self._props['url'])
-        super().delete()
+        super()._on_delete()

nicegui/nicegui.py

@@ -229,7 +229,6 @@ async def prune_slot_stacks() -> None:
 
 
 def delete_client(client_id: str) -> None:
-    binding.remove(list(globals.clients[client_id].elements.values()), Element)
-    for element in globals.clients[client_id].elements.values():
-        element.delete()
+    elements = globals.clients[client_id].elements.values()
+    Element._delete_elements(elements)  # pylint: disable=protected-access
     del globals.clients[client_id]

tests/test_element_delete.py

@@ -1,5 +1,3 @@
-import pytest
-
 from nicegui import binding, ui
 
 from .screen import Screen
@@ -70,7 +68,6 @@ def test_clear(screen: Screen):
     assert len(binding.active_links) == 0
 
 
-@pytest.mark.skip(reason='needs fix in element.py')  # TODO
 def test_remove_parent(screen: Screen):
     texts = {'a': 'Label A', 'b': 'Label B', 'c': 'Label C'}
     with ui.element() as container:
@@ -95,11 +92,9 @@ def test_remove_parent(screen: Screen):
     assert b.id not in container.client.elements
     assert c.id not in container.client.elements
     assert len(container.default_slot.children) == 0
-    assert len(row.default_slot.children) == 0
     assert len(binding.active_links) == 0
 
 
-@pytest.mark.skip(reason='needs fix in element.py')  # TODO
 def test_delete_element(screen: Screen):
     texts = {'a': 'Label A', 'b': 'Label B', 'c': 'Label C'}
     with ui.row() as row: