From 8e1b0aa9e99af40439d8b7ca5314700d48bb0bf9 Mon Sep 17 00:00:00 2001 From: Will McGugan Date: Thu, 26 Sep 2024 20:28:32 +0100 Subject: [PATCH 01/14] refreshing invisible widgets is a no-op --- CHANGELOG.md | 1 + src/textual/_compositor.py | 7 +------ src/textual/dom.py | 8 ++++++-- src/textual/screen.py | 10 ++++++---- src/textual/widget.py | 9 +++++++++ 5 files changed, 23 insertions(+), 12 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a6bbb814d0..8e0e0222a2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/). - Added `x_axis` and `y_axis` parameters to `Widget.scroll_to_region` https://github.com/Textualize/textual/pull/5047 - Added `Tree.move_cursor_to_line` https://github.com/Textualize/textual/pull/5052 +- Added `DOMNode.is_on_screen` property ### Changed diff --git a/src/textual/_compositor.py b/src/textual/_compositor.py index 6136db8e1d..d4a27c6a60 100644 --- a/src/textual/_compositor.py +++ b/src/textual/_compositor.py @@ -311,9 +311,6 @@ def __init__(self) -> None: # Mapping of line numbers on to lists of widget and regions self._layers_visible: list[list[tuple[Widget, Region, Region]]] | None = None - # New widgets added between updates - self._new_widgets: set[Widget] = set() - def clear(self) -> None: """Remove all references to widgets (used when the screen closes).""" self._full_map.clear() @@ -392,8 +389,7 @@ def reflow(self, parent: Widget, size: Size) -> ReflowResult: new_widgets = map.keys() # Newly visible widgets - shown_widgets = (new_widgets - old_widgets) | self._new_widgets - self._new_widgets.clear() + shown_widgets = new_widgets - old_widgets # Newly hidden widgets hidden_widgets = self.widgets - widgets @@ -490,7 +486,6 @@ def full_map(self) -> CompositorMap: self._full_map_invalidated = False map, _widgets = self._arrange_root(self.root, self.size, visible_only=False) # Update any widgets which became visible in the interim - self._new_widgets.update(map.keys() - self._full_map.keys()) self._full_map = map self._visible_widgets = None self._visible_map = None diff --git a/src/textual/dom.py b/src/textual/dom.py index c955eb9fb3..75dfac924c 100644 --- a/src/textual/dom.py +++ b/src/textual/dom.py @@ -489,6 +489,11 @@ def is_modal(self) -> bool: """Is the node a modal?""" return False + @property + def is_on_screen(self) -> bool: + """Check if the node was displayed in the last screen update.""" + return False + def automatic_refresh(self) -> None: """Perform an automatic refresh. @@ -497,8 +502,7 @@ def automatic_refresh(self) -> None: during an automatic refresh. """ - if self.display and self.visible: - self.refresh() + self.refresh() def __init_subclass__( cls, diff --git a/src/textual/screen.py b/src/textual/screen.py index 997d5c1c90..4b6c49d425 100644 --- a/src/textual/screen.py +++ b/src/textual/screen.py @@ -147,6 +147,7 @@ class Screen(Generic[ScreenResultType], Widget): DEFAULT_CSS = """ Screen { + layout: vertical; overflow-y: auto; background: $surface; @@ -162,11 +163,11 @@ class Screen(Generic[ScreenResultType], Widget): background: ansi_default; color: ansi_default; - &.-screen-suspended { + &.-screen-suspended { + text-style: dim; ScrollBar { text-style: not dim; } - text-style: dim; } } } @@ -1124,8 +1125,9 @@ async def _on_update(self, message: messages.Update) -> None: widget = message.widget assert isinstance(widget, Widget) - self._dirty_widgets.add(widget) - self.check_idle() + if widget in self._compositor.widgets: + self._dirty_widgets.add(widget) + self.check_idle() async def _on_layout(self, message: messages.Layout) -> None: message.stop() diff --git a/src/textual/widget.py b/src/textual/widget.py index b7bcfa7df3..07d52e899f 100644 --- a/src/textual/widget.py +++ b/src/textual/widget.py @@ -1927,6 +1927,15 @@ def _has_relative_children_height(self) -> bool: return True return False + @property + def is_on_screen(self) -> bool: + """Check if the node was displayed in the last screen update.""" + try: + self.screen.find_widget(self) + except (NoScreen, errors.NoWidget): + return False + return True + def animate( self, attribute: str, From c57757ac8177ffc2e2771a14a9601a3331b2c111 Mon Sep 17 00:00:00 2001 From: Will McGugan Date: Thu, 26 Sep 2024 20:37:57 +0100 Subject: [PATCH 02/14] snapshot --- tests/snapshot_tests/test_snapshots.py | 33 ++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/tests/snapshot_tests/test_snapshots.py b/tests/snapshot_tests/test_snapshots.py index 427e55a82a..fb5baeba31 100644 --- a/tests/snapshot_tests/test_snapshots.py +++ b/tests/snapshot_tests/test_snapshots.py @@ -22,6 +22,7 @@ Footer, Log, OptionList, + Placeholder, SelectionList, ) from textual.widgets import ProgressBar, Label, Switch @@ -2019,3 +2020,35 @@ def action_toggle_console(self) -> None: app = MRE() assert snap_compare(app, press=["space", "space", "z"]) + + +def test_updates_with_auto_refresh(snap_compare): + """Regression test for https://github.com/Textualize/textual/issues/5056""" + + class MRE(App): + BINDINGS = [ + ("z", "toggle_widget('RichLog')", "Console"), + ("x", "toggle_widget('ProgressBar')", "Progress bar"), + ] + CSS = """ + Placeholder { height: 15; } + RichLog { border-top: dashed blue; height: 6; } + .hidden { display: none; } + """ + + def compose(self): + with VerticalScroll(): + for i in range(10): + yield Placeholder() + yield ProgressBar(classes="hidden") + yield RichLog(classes="hidden") + yield Footer() + + def on_ready(self) -> None: + self.query_one(RichLog).write("\n".join(f"line #{i}" for i in range(5))) + + def action_toggle_widget(self, widget_type: str) -> None: + self.query_one(widget_type).toggle_class("hidden") + + app = MRE() + assert snap_compare(app, press=["z", "z"]) From 2b43823607565707d89b686af8deeff9eb979128 Mon Sep 17 00:00:00 2001 From: Will McGugan Date: Thu, 26 Sep 2024 20:41:30 +0100 Subject: [PATCH 03/14] snapshot --- src/textual/dom.py | 3 +- .../test_updates_with_auto_refresh.svg | 158 ++++++++++++++++++ 2 files changed, 160 insertions(+), 1 deletion(-) create mode 100644 tests/snapshot_tests/__snapshots__/test_snapshots/test_updates_with_auto_refresh.svg diff --git a/src/textual/dom.py b/src/textual/dom.py index 75dfac924c..96061a2759 100644 --- a/src/textual/dom.py +++ b/src/textual/dom.py @@ -502,7 +502,8 @@ def automatic_refresh(self) -> None: during an automatic refresh. """ - self.refresh() + if self.is_on_screen: + self.refresh() def __init_subclass__( cls, diff --git a/tests/snapshot_tests/__snapshots__/test_snapshots/test_updates_with_auto_refresh.svg b/tests/snapshot_tests/__snapshots__/test_snapshots/test_updates_with_auto_refresh.svg new file mode 100644 index 0000000000..1358e0698c --- /dev/null +++ b/tests/snapshot_tests/__snapshots__/test_snapshots/test_updates_with_auto_refresh.svg @@ -0,0 +1,158 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + MRE + + + + + + + + + + + + +▃▃ + + + +Placeholder + + + + + + + + + + + + + + +Placeholder + z Console  x Progress bar ^p palette + + + From 69cfaf11ac1a628db4eb71384856475c05ce1ca1 Mon Sep 17 00:00:00 2001 From: Will McGugan Date: Thu, 26 Sep 2024 20:45:55 +0100 Subject: [PATCH 04/14] version bump --- CHANGELOG.md | 11 +++++++++++ pyproject.toml | 2 +- 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8e0e0222a2..916e2098bb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,16 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](http://keepachangelog.com/) and this project adheres to [Semantic Versioning](http://semver.org/). +## [0.81.1] - 2024-09-26 + +### Fixed + +- Fixed issue with screen not updating when auto_refresh was enabled https://github.com/Textualize/textual/pull/5063 + +### Added + +- Added `DOMNode.is_on_screen` property https://github.com/Textualize/textual/pull/5063 + ## [0.81.0] - 2024-09-25 ### Added @@ -2407,6 +2417,7 @@ https://textual.textualize.io/blog/2022/11/08/version-040/#version-040 - New handler system for messages that doesn't require inheritance - Improved traceback handling +[0.81.1]: https://github.com/Textualize/textual/compare/v0.81.0...v0.81.1 [0.81.0]: https://github.com/Textualize/textual/compare/v0.80.1...v0.81.0 [0.80.1]: https://github.com/Textualize/textual/compare/v0.80.0...v0.80.1 [0.80.0]: https://github.com/Textualize/textual/compare/v0.79.0...v0.80.0 diff --git a/pyproject.toml b/pyproject.toml index c2a0c63605..2705749190 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [tool.poetry] name = "textual" -version = "0.81.0" +version = "0.81.1" homepage = "https://github.com/Textualize/textual" repository = "https://github.com/Textualize/textual" documentation = "https://textual.textualize.io/" From 8ef10991da54b7726edf132003131699284151f6 Mon Sep 17 00:00:00 2001 From: Will McGugan Date: Thu, 26 Sep 2024 20:59:27 +0100 Subject: [PATCH 05/14] check full map for clear --- src/textual/screen.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/textual/screen.py b/src/textual/screen.py index 4b6c49d425..584827ba2a 100644 --- a/src/textual/screen.py +++ b/src/textual/screen.py @@ -1125,7 +1125,8 @@ async def _on_update(self, message: messages.Update) -> None: widget = message.widget assert isinstance(widget, Widget) - if widget in self._compositor.widgets: + compositor = self._compositor + if widget in compositor.widgets or widget in compositor.full_map: self._dirty_widgets.add(widget) self.check_idle() From 3dbe844298446949970ff04e094743deeb5fa033 Mon Sep 17 00:00:00 2001 From: Will McGugan Date: Thu, 26 Sep 2024 21:12:35 +0100 Subject: [PATCH 06/14] fix changelog --- CHANGELOG.md | 1 - 1 file changed, 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 916e2098bb..a986d61c6d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,7 +21,6 @@ and this project adheres to [Semantic Versioning](http://semver.org/). - Added `x_axis` and `y_axis` parameters to `Widget.scroll_to_region` https://github.com/Textualize/textual/pull/5047 - Added `Tree.move_cursor_to_line` https://github.com/Textualize/textual/pull/5052 -- Added `DOMNode.is_on_screen` property ### Changed From 4d66b9649568dd5eba99ca8690324247b15baef3 Mon Sep 17 00:00:00 2001 From: Will McGugan Date: Sat, 28 Sep 2024 18:58:19 +0100 Subject: [PATCH 07/14] more elegant API --- CHANGELOG.md | 2 +- pyproject.toml | 2 +- src/textual/_compositor.py | 15 +++++++++++++++ src/textual/screen.py | 3 +-- 4 files changed, 18 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a986d61c6d..22739755b6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,7 +5,7 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](http://keepachangelog.com/) and this project adheres to [Semantic Versioning](http://semver.org/). -## [0.81.1] - 2024-09-26 +## Unreleased ### Fixed diff --git a/pyproject.toml b/pyproject.toml index 2705749190..c2a0c63605 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [tool.poetry] name = "textual" -version = "0.81.1" +version = "0.81.0" homepage = "https://github.com/Textualize/textual" repository = "https://github.com/Textualize/textual" documentation = "https://textual.textualize.io/" diff --git a/src/textual/_compositor.py b/src/textual/_compositor.py index d4a27c6a60..cfe7e27fa7 100644 --- a/src/textual/_compositor.py +++ b/src/textual/_compositor.py @@ -798,6 +798,21 @@ def layers_visible(self) -> list[list[tuple[Widget, Region, Region]]]: self._layers_visible = layers_visible return self._layers_visible + def __contains__(self, widget: Widget) -> bool: + """Check if the widget was included in the last update. + + Args: + widget: A widget. + + Returns: + `True` if the widget was in the last refresh, or `False` if it wasn't. + """ + return ( + widget in self.widgets + or (self._visible_map is not None and widget in self._visible_map) + or widget in self.full_map + ) + def get_offset(self, widget: Widget) -> Offset: """Get the offset of a widget. diff --git a/src/textual/screen.py b/src/textual/screen.py index 584827ba2a..5ad0ae4a73 100644 --- a/src/textual/screen.py +++ b/src/textual/screen.py @@ -1125,8 +1125,7 @@ async def _on_update(self, message: messages.Update) -> None: widget = message.widget assert isinstance(widget, Widget) - compositor = self._compositor - if widget in compositor.widgets or widget in compositor.full_map: + if self in self._compositor: self._dirty_widgets.add(widget) self.check_idle() From 06efcaac36fb294d268496ac38cccdb8e190d53d Mon Sep 17 00:00:00 2001 From: Will McGugan Date: Sat, 28 Sep 2024 18:58:43 +0100 Subject: [PATCH 08/14] more elegant API --- src/textual/_compositor.py | 1 + 1 file changed, 1 insertion(+) diff --git a/src/textual/_compositor.py b/src/textual/_compositor.py index cfe7e27fa7..2b09180db1 100644 --- a/src/textual/_compositor.py +++ b/src/textual/_compositor.py @@ -807,6 +807,7 @@ def __contains__(self, widget: Widget) -> bool: Returns: `True` if the widget was in the last refresh, or `False` if it wasn't. """ + # Try to avoid a recalculation of full_map if possible. return ( widget in self.widgets or (self._visible_map is not None and widget in self._visible_map) From fd103957a2f48eda438488465eb5d6d65ab46c85 Mon Sep 17 00:00:00 2001 From: Will McGugan Date: Sat, 28 Sep 2024 18:59:40 +0100 Subject: [PATCH 09/14] not ready yet --- CHANGELOG.md | 1 - 1 file changed, 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 22739755b6..065da7b513 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2416,7 +2416,6 @@ https://textual.textualize.io/blog/2022/11/08/version-040/#version-040 - New handler system for messages that doesn't require inheritance - Improved traceback handling -[0.81.1]: https://github.com/Textualize/textual/compare/v0.81.0...v0.81.1 [0.81.0]: https://github.com/Textualize/textual/compare/v0.80.1...v0.81.0 [0.80.1]: https://github.com/Textualize/textual/compare/v0.80.0...v0.80.1 [0.80.0]: https://github.com/Textualize/textual/compare/v0.79.0...v0.80.0 From 16802978f2db8b68de2593d25f8e596be44172c2 Mon Sep 17 00:00:00 2001 From: Will McGugan Date: Mon, 30 Sep 2024 14:34:01 +0100 Subject: [PATCH 10/14] Update tests/snapshot_tests/test_snapshots.py Co-authored-by: Darren Burns --- tests/snapshot_tests/test_snapshots.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/tests/snapshot_tests/test_snapshots.py b/tests/snapshot_tests/test_snapshots.py index 0256006b02..2b79a434fe 100644 --- a/tests/snapshot_tests/test_snapshots.py +++ b/tests/snapshot_tests/test_snapshots.py @@ -2102,7 +2102,12 @@ def action_toggle_console(self) -> None: def test_updates_with_auto_refresh(snap_compare): - """Regression test for https://github.com/Textualize/textual/issues/5056""" + """Regression test for https://github.com/Textualize/textual/issues/5056 + + After hiding and unhiding the RichLog, you should be able to see 1.5 fully rendered placeholder widgets. + Prior to this fix, the bottom portion of the screen did not + refresh after the RichLog was hidden/unhidden while in the presence of the auto-refreshing ProgressBar widget. + """ class MRE(App): BINDINGS = [ From 5ed7a71fe4bc16e6b38a2d13f943242c41e5af9d Mon Sep 17 00:00:00 2001 From: Will McGugan Date: Mon, 30 Sep 2024 14:34:08 +0100 Subject: [PATCH 11/14] Update tests/snapshot_tests/test_snapshots.py Co-authored-by: Darren Burns --- tests/snapshot_tests/test_snapshots.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/snapshot_tests/test_snapshots.py b/tests/snapshot_tests/test_snapshots.py index 2b79a434fe..26757e374d 100644 --- a/tests/snapshot_tests/test_snapshots.py +++ b/tests/snapshot_tests/test_snapshots.py @@ -2116,7 +2116,7 @@ class MRE(App): ] CSS = """ Placeholder { height: 15; } - RichLog { border-top: dashed blue; height: 6; } + RichLog { height: 6; } .hidden { display: none; } """ From 0e0caacf2f15bb606b6dee2fb21eacc6e02cec88 Mon Sep 17 00:00:00 2001 From: Will McGugan Date: Mon, 30 Sep 2024 14:34:17 +0100 Subject: [PATCH 12/14] Update tests/snapshot_tests/test_snapshots.py Co-authored-by: Darren Burns --- tests/snapshot_tests/test_snapshots.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/snapshot_tests/test_snapshots.py b/tests/snapshot_tests/test_snapshots.py index 26757e374d..d4420301bc 100644 --- a/tests/snapshot_tests/test_snapshots.py +++ b/tests/snapshot_tests/test_snapshots.py @@ -2112,7 +2112,6 @@ def test_updates_with_auto_refresh(snap_compare): class MRE(App): BINDINGS = [ ("z", "toggle_widget('RichLog')", "Console"), - ("x", "toggle_widget('ProgressBar')", "Progress bar"), ] CSS = """ Placeholder { height: 15; } From e3afdf53953040d115d43946e41731772416558c Mon Sep 17 00:00:00 2001 From: Will McGugan Date: Mon, 30 Sep 2024 14:34:23 +0100 Subject: [PATCH 13/14] Update tests/snapshot_tests/test_snapshots.py Co-authored-by: Darren Burns --- tests/snapshot_tests/test_snapshots.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/snapshot_tests/test_snapshots.py b/tests/snapshot_tests/test_snapshots.py index d4420301bc..0c7d56cd6c 100644 --- a/tests/snapshot_tests/test_snapshots.py +++ b/tests/snapshot_tests/test_snapshots.py @@ -2125,7 +2125,6 @@ def compose(self): yield Placeholder() yield ProgressBar(classes="hidden") yield RichLog(classes="hidden") - yield Footer() def on_ready(self) -> None: self.query_one(RichLog).write("\n".join(f"line #{i}" for i in range(5))) From df01b7a6a6e88e401f072c3dfd84bf236d612d3c Mon Sep 17 00:00:00 2001 From: Will McGugan Date: Mon, 30 Sep 2024 14:55:47 +0100 Subject: [PATCH 14/14] snapshot fix --- .../test_updates_with_auto_refresh.svg | 124 +++++++++--------- 1 file changed, 60 insertions(+), 64 deletions(-) diff --git a/tests/snapshot_tests/__snapshots__/test_snapshots/test_updates_with_auto_refresh.svg b/tests/snapshot_tests/__snapshots__/test_snapshots/test_updates_with_auto_refresh.svg index 1358e0698c..fd44ac98e9 100644 --- a/tests/snapshot_tests/__snapshots__/test_snapshots/test_updates_with_auto_refresh.svg +++ b/tests/snapshot_tests/__snapshots__/test_snapshots/test_updates_with_auto_refresh.svg @@ -19,140 +19,136 @@ font-weight: 700; } - .terminal-2621110581-matrix { + .terminal-943331377-matrix { font-family: Fira Code, monospace; font-size: 20px; line-height: 24.4px; font-variant-east-asian: full-width; } - .terminal-2621110581-title { + .terminal-943331377-title { font-size: 18px; font-weight: bold; font-family: arial; } - .terminal-2621110581-r1 { fill: #c5c8c6 } -.terminal-2621110581-r2 { fill: #1e1e1e } -.terminal-2621110581-r3 { fill: #14191f } -.terminal-2621110581-r4 { fill: #e1e1e1 } -.terminal-2621110581-r5 { fill: #e8e0e7 } -.terminal-2621110581-r6 { fill: #eae3e5 } -.terminal-2621110581-r7 { fill: #fea62b;font-weight: bold } -.terminal-2621110581-r8 { fill: #a7a9ab } -.terminal-2621110581-r9 { fill: #e2e3e3 } -.terminal-2621110581-r10 { fill: #4c5055 } + .terminal-943331377-r1 { fill: #c5c8c6 } +.terminal-943331377-r2 { fill: #1e1e1e } +.terminal-943331377-r3 { fill: #14191f } +.terminal-943331377-r4 { fill: #e1e1e1 } +.terminal-943331377-r5 { fill: #e8e0e7 } +.terminal-943331377-r6 { fill: #eae3e5 } - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - MRE + MRE - - - - - - -▃▃ - - - -Placeholder - - - - - - - - - - - - - - -Placeholder - z Console  x Progress bar ^p palette + + + + + + +▁▁ + + + +Placeholder + + + + + + + + + + + + + + +Placeholder +