Merge pull request #1514 from zauberzeug/clear-remove-delete

Fix and improve clear, remove and delete methods

nicegui/binding.py

@@ -2,7 +2,7 @@
 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 @@ def __set__(self, owner: Any, value: Any) -> None:
             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
@@ -126,3 +126,13 @@ def remove(objects: List[Any], type_: Type) -> None:
     for (obj_id, name), obj in list(bindable_properties.items()):
         if isinstance(obj, type_) and obj in objects:
             del bindable_properties[(obj_id, name)]
+
+
+def reset() -> None:
+    """Clear all bindings.
+
+    This function is intended for testing purposes only.
+    """
+    bindings.clear()
+    bindable_properties.clear()
+    active_links.clear()

nicegui/client.py

@@ -4,15 +4,15 @@
 import time
 import uuid
 from pathlib import Path
-from typing import TYPE_CHECKING, Any, Awaitable, Callable, Dict, List, Optional, Union
+from typing import TYPE_CHECKING, Any, Awaitable, Callable, Dict, Iterable, List, Optional, Union
 
 from fastapi import Request
 from fastapi.responses import Response
 from fastapi.templating import Jinja2Templates
 
 from nicegui import json
 
-from . import globals, outbox  # pylint: disable=redefined-builtin
+from . import binding, globals, outbox  # pylint: disable=redefined-builtin
 from .dependencies import generate_resources
 from .element import Element
 from .favicon import get_favicon_url
@@ -162,3 +162,18 @@ def on_connect(self, handler: Union[Callable[..., Any], Awaitable]) -> None:
     def on_disconnect(self, handler: Union[Callable[..., Any], Awaitable]) -> None:
         """Register a callback to be called when the client disconnects."""
         self.disconnect_handlers.append(handler)
+
+    def remove_elements(self, elements: Iterable[Element]) -> None:
+        """Remove the given elements from the client."""
+        binding.remove(elements, Element)
+        element_ids = [element.id for element in elements]
+        for element_id in element_ids:
+            del self.elements[element_id]
+        for element in elements:
+            element._on_delete()  # pylint: disable=protected-access
+            element._deleted = True  # pylint: disable=protected-access
+            outbox.enqueue_delete(element)
+
+    def remove_all_elements(self) -> None:
+        """Remove all elements from the client."""
+        self.remove_elements(self.elements.values())

nicegui/element.py

@@ -10,7 +10,7 @@
 
 from nicegui import json
 
-from . import binding, events, globals, outbox, storage  # pylint: disable=redefined-builtin
+from . import events, globals, outbox, storage  # pylint: disable=redefined-builtin
 from .dependencies import Component, Library, register_library, register_vue_component
 from .elements.mixins.visibility import Visibility
 from .event_listener import EventListener
@@ -304,19 +304,16 @@ def run_method(self, name: str, *args: Any) -> None:
         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
 
     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.client.remove_elements(descendants)
         for slot in self.slots.values():
             slot.children.clear()
         self.update()
@@ -344,17 +341,23 @@ def remove(self, element: Union[Element, int]) -> None:
         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.client.remove_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.client.remove_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) -> bool:

nicegui/elements/scene.py

@@ -184,9 +184,9 @@ def move_camera(self,
                         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 @@ async def upload_route(request: Request) -> Dict[str, str]:
     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

@@ -14,7 +14,6 @@
 from .app import App
 from .client import Client
 from .dependencies import js_components, libraries
-from .element import Element
 from .error import error_content
 from .helpers import is_file, safe_invoke
 from .json import NiceGUIJSONResponse
@@ -229,7 +228,4 @@ 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()
-    del globals.clients[client_id]
+    globals.clients.pop(client_id).remove_all_elements()

tests/conftest.py

@@ -9,7 +9,7 @@
 from selenium import webdriver
 from selenium.webdriver.chrome.service import Service
 
-from nicegui import Client, globals  # pylint: disable=redefined-builtin
+from nicegui import Client, binding, globals  # pylint: disable=redefined-builtin
 from nicegui.elements import plotly, pyplot
 from nicegui.page import page
 
@@ -57,6 +57,7 @@ def reset_globals() -> Generator[None, None, None]:
     globals.app.storage.clear()
     globals.index_client = Client(page('/'), shared=True).__enter__()
     globals.app.get('/')(globals.index_client.build_response)
+    binding.reset()
 
 
 @pytest.fixture(scope='session', autouse=True)

tests/test_element.py

@@ -105,40 +105,6 @@ def assert_props(*props: str) -> None:
     assert_props('standard')
 
 
-def test_remove_and_clear(screen: Screen):
-    with ui.row() as row:
-        ui.label('Label A')
-        b = ui.label('Label B')
-        ui.label('Label C')
-
-    ui.button('Remove B', on_click=lambda: row.remove(b))
-    ui.button('Remove 0', on_click=lambda: row.remove(0))
-    ui.button('Clear', on_click=row.clear)
-
-    screen.open('/')
-    screen.should_contain('Label A')
-    screen.should_contain('Label B')
-    screen.should_contain('Label C')
-
-    screen.click('Remove B')
-    screen.wait(0.5)
-    screen.should_contain('Label A')
-    screen.should_not_contain('Label B')
-    screen.should_contain('Label C')
-
-    screen.click('Remove 0')
-    screen.wait(0.5)
-    screen.should_not_contain('Label A')
-    screen.should_not_contain('Label B')
-    screen.should_contain('Label C')
-
-    screen.click('Clear')
-    screen.wait(0.5)
-    screen.should_not_contain('Label A')
-    screen.should_not_contain('Label B')
-    screen.should_not_contain('Label C')
-
-
 def test_move(screen: Screen):
     with ui.card() as a:
         ui.label('A')

tests/test_element_delete.py

@@ -0,0 +1,145 @@
+from nicegui import binding, ui
+
+from .screen import Screen
+
+
+def test_remove_element_by_reference(screen: Screen):
+    texts = {'a': 'Label A', 'b': 'Label B', 'c': 'Label C'}
+    with ui.row() as row:
+        ui.label().bind_text_from(texts, 'a')
+        b = ui.label().bind_text_from(texts, 'b')
+        ui.label().bind_text_from(texts, 'c')
+
+    ui.button('Remove', on_click=lambda: row.remove(b))
+
+    screen.open('/')
+    screen.click('Remove')
+    screen.wait(0.5)
+    screen.should_contain('Label A')
+    screen.should_not_contain('Label B')
+    screen.should_contain('Label C')
+    assert b.is_deleted
+    assert b.id not in row.client.elements
+    assert len(row.default_slot.children) == 2
+    assert len(binding.active_links) == 2
+
+
+def test_remove_element_by_index(screen: Screen):
+    texts = {'a': 'Label A', 'b': 'Label B', 'c': 'Label C'}
+    with ui.row() as row:
+        ui.label().bind_text_from(texts, 'a')
+        b = ui.label().bind_text_from(texts, 'b')
+        ui.label().bind_text_from(texts, 'c')
+
+    ui.button('Remove', on_click=lambda: row.remove(1))
+
+    screen.open('/')
+    screen.click('Remove')
+    screen.wait(0.5)
+    screen.should_contain('Label A')
+    screen.should_not_contain('Label B')
+    screen.should_contain('Label C')
+    assert b.is_deleted
+    assert b.id not in row.client.elements
+    assert len(row.default_slot.children) == 2
+    assert len(binding.active_links) == 2
+
+
+def test_clear(screen: Screen):
+    texts = {'a': 'Label A', 'b': 'Label B', 'c': 'Label C'}
+    with ui.row() as row:
+        a = ui.label().bind_text_from(texts, 'a')
+        b = ui.label().bind_text_from(texts, 'b')
+        c = ui.label().bind_text_from(texts, 'c')
+
+    ui.button('Clear', on_click=row.clear)
+
+    screen.open('/')
+    screen.click('Clear')
+    screen.wait(0.5)
+    screen.should_not_contain('Label A')
+    screen.should_not_contain('Label B')
+    screen.should_not_contain('Label C')
+    assert a.is_deleted
+    assert b.is_deleted
+    assert c.is_deleted
+    assert b.id not in row.client.elements
+    assert len(row.default_slot.children) == 0
+    assert len(binding.active_links) == 0
+
+
+def test_remove_parent(screen: Screen):
+    texts = {'a': 'Label A', 'b': 'Label B', 'c': 'Label C'}
+    with ui.element() as container:
+        with ui.row() as row:
+            a = ui.label().bind_text_from(texts, 'a')
+            b = ui.label().bind_text_from(texts, 'b')
+            c = ui.label().bind_text_from(texts, 'c')
+
+    ui.button('Remove parent', on_click=lambda: container.remove(row))
+
+    screen.open('/')
+    screen.click('Remove parent')
+    screen.wait(0.5)
+    screen.should_not_contain('Label A')
+    screen.should_not_contain('Label B')
+    screen.should_not_contain('Label C')
+    assert row.is_deleted
+    assert a.is_deleted
+    assert b.is_deleted
+    assert c.is_deleted
+    assert a.id not in container.client.elements
+    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(binding.active_links) == 0
+
+
+def test_delete_element(screen: Screen):
+    texts = {'a': 'Label A', 'b': 'Label B', 'c': 'Label C'}
+    with ui.row() as row:
+        ui.label().bind_text_from(texts, 'a')
+        b = ui.label().bind_text_from(texts, 'b')
+        ui.label().bind_text_from(texts, 'c')
+
+    ui.button('Delete', on_click=b.delete)
+
+    screen.open('/')
+    screen.click('Delete')
+    screen.wait(0.5)
+    screen.should_contain('Label A')
+    screen.should_not_contain('Label B')
+    screen.should_contain('Label C')
+    assert b.is_deleted
+    assert b.id not in row.client.elements
+    assert len(row.default_slot.children) == 2
+    assert len(binding.active_links) == 2
+
+
+def test_on_delete(screen: Screen):
+    deleted_labels = []
+
+    class CustomLabel(ui.label):
+
+        def __init__(self, text: str) -> None:
+            super().__init__(text)
+
+        def _on_delete(self) -> None:
+            deleted_labels.append(self.text)
+            super()._on_delete()
+
+    with ui.row() as row:
+        CustomLabel('Label A')
+        b = CustomLabel('Label B')
+        CustomLabel('Label C')
+
+    ui.button('Delete C', on_click=lambda: row.remove(2))
+    ui.button('Delete B', on_click=lambda: row.remove(b))
+    ui.button('Clear row', on_click=row.clear)
+
+    screen.open('/')
+    screen.click('Delete C')
+    screen.click('Delete B')
+    screen.click('Clear row')
+    screen.wait(0.5)
+    assert deleted_labels == ['Label C', 'Label B', 'Label A']

website/documentation.py

@@ -153,9 +153,16 @@ def captions_and_overlays_demo():
     load_demo(ui.grid)
 
     @text_demo('Clear Containers', '''
-        To remove all elements from a row, column or card container, use the `clear()` method.
+        To remove all elements from a row, column or card container, use can call
+        ```py
+        container.clear()
+        ```
 
-        Alternatively, you can remove individual elements with `remove(element)`, where `element` is an Element or an index.
+        Alternatively, you can remove individual elements by calling
+        
+        - `container.remove(element: Element)`,
+        - `container.remove(index: int)`, or
+        - `element.delete()`.
     ''')
     def clear_containers_demo():
         container = ui.row()