From c5cb0100ff0960c6e8fa2b64cb6c5ef53adc4d82 Mon Sep 17 00:00:00 2001 From: Will McGugan Date: Fri, 24 Feb 2023 10:02:52 +0000 Subject: [PATCH 1/2] fix walk children --- src/textual/app.py | 3 +++ src/textual/reactive.py | 8 ++++---- src/textual/walk.py | 2 +- 3 files changed, 8 insertions(+), 5 deletions(-) diff --git a/src/textual/app.py b/src/textual/app.py index 318dc3223..ed3adc280 100644 --- a/src/textual/app.py +++ b/src/textual/app.py @@ -2153,6 +2153,9 @@ class App(Generic[ReturnType], DOMNode): if widget.parent is not None: widget.parent._nodes._remove(widget) + for node in pruned_remove: + node._detach() + # Return the list of widgets that should end up being sent off in a # prune event. return pruned_remove diff --git a/src/textual/reactive.py b/src/textual/reactive.py index 0553d076c..f03624a0c 100644 --- a/src/textual/reactive.py +++ b/src/textual/reactive.py @@ -239,9 +239,10 @@ class Reactive(Generic[ReactiveType]): ) ) - watch_function = getattr(obj, f"watch_{name}", None) - if callable(watch_function): - invoke_watcher(watch_function, old_value, value) + if obj.is_attached: + watch_function = getattr(obj, f"watch_{name}", None) + if callable(watch_function): + invoke_watcher(watch_function, old_value, value) # Process "global" watchers watchers: list[tuple[Reactable, Callable]] @@ -342,7 +343,6 @@ def _watch( callback: A callable to call when the attribute changes. init: True to call watcher initialization. Defaults to True. """ - if not hasattr(obj, "__watchers"): setattr(obj, "__watchers", {}) watchers: dict[str, list[tuple[Reactable, Callable]]] = getattr(obj, "__watchers") diff --git a/src/textual/walk.py b/src/textual/walk.py index d43d40b06..0dc672471 100644 --- a/src/textual/walk.py +++ b/src/textual/walk.py @@ -53,7 +53,7 @@ def walk_depth_first( """ from textual.dom import DOMNode - stack: list[Iterator[DOMNode]] = [iter(root._nodes)] + stack: list[Iterator[DOMNode]] = [iter(root.children)] pop = stack.pop push = stack.append check_type = filter_type or DOMNode From 5a77e3493dd723a1000c94d233813c40581064f9 Mon Sep 17 00:00:00 2001 From: Will McGugan Date: Fri, 24 Feb 2023 10:13:58 +0000 Subject: [PATCH 2/2] add test --- CHANGELOG.md | 2 ++ src/textual/reactive.py | 7 +++---- tests/test_screens.py | 16 ++++++++++++++++ 3 files changed, 21 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 042796b7b..52b700ef1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -30,6 +30,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/). - Numbers in a descendant-combined selector no longer cause an error https://github.com/Textualize/textual/issues/1836 - Fixed superfluous scrolling when focusing a docked widget https://github.com/Textualize/textual/issues/1816 +- Fixes walk_children which was returning more than one screen https://github.com/Textualize/textual/issues/1846 +- Fixed issue with watchers fired for detached nodes https://github.com/Textualize/textual/issues/1846 ## [0.11.1] - 2023-02-17 diff --git a/src/textual/reactive.py b/src/textual/reactive.py index f03624a0c..098ab031b 100644 --- a/src/textual/reactive.py +++ b/src/textual/reactive.py @@ -239,10 +239,9 @@ class Reactive(Generic[ReactiveType]): ) ) - if obj.is_attached: - watch_function = getattr(obj, f"watch_{name}", None) - if callable(watch_function): - invoke_watcher(watch_function, old_value, value) + watch_function = getattr(obj, f"watch_{name}", None) + if callable(watch_function): + invoke_watcher(watch_function, old_value, value) # Process "global" watchers watchers: list[tuple[Reactable, Callable]] diff --git a/tests/test_screens.py b/tests/test_screens.py index ca56df9c5..3edc5dce5 100644 --- a/tests/test_screens.py +++ b/tests/test_screens.py @@ -11,6 +11,22 @@ skip_py310 = pytest.mark.skipif( ) +async def test_screen_walk_children(): + """Test query only reports active screen.""" + + class ScreensApp(App): + pass + + app = ScreensApp() + async with app.run_test() as pilot: + screen1 = Screen() + screen2 = Screen() + pilot.app.push_screen(screen1) + assert list(pilot.app.query("*")) == [screen1] + pilot.app.push_screen(screen2) + assert list(pilot.app.query("*")) == [screen2] + + async def test_installed_screens(): class ScreensApp(App): SCREENS = {