From b5acfef6279d7a87e8d138de637136ec2eb8d38a Mon Sep 17 00:00:00 2001 From: Will McGugan Date: Wed, 2 Oct 2024 14:32:07 +0100 Subject: [PATCH 1/7] new loading indicator --- CHANGELOG.md | 1 + src/textual/_compositor.py | 38 +++++--- src/textual/widget.py | 107 +++++++++++++--------- src/textual/widgets/_loading_indicator.py | 5 +- 4 files changed, 93 insertions(+), 58 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ab342eef44..1c7e14437a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/). ### Fixed - Fixed issue with screen not updating when auto_refresh was enabled https://github.com/Textualize/textual/pull/5063 +- Fixed issues regarding loading indicator https://github.com/Textualize/textual/issues/3935 ### Added diff --git a/src/textual/_compositor.py b/src/textual/_compositor.py index 2b09180db1..7a3f0c8fe7 100644 --- a/src/textual/_compositor.py +++ b/src/textual/_compositor.py @@ -664,6 +664,17 @@ def add_widget( get_layer_index = layers_to_index.get + if widget._cover_widget: + map[widget._cover_widget] = _MapGeometry( + region.shrink(widget.styles.gutter), + order, + clip, + region.size, + container_size, + virtual_region, + dock_gutter, + ) + # Add all the widgets for sub_region, _, sub_widget, z, fixed, overlay in reversed( placements @@ -681,18 +692,17 @@ def add_widget( widget_region = self._constrain( sub_widget.styles, widget_region, no_clip ) - - add_widget( - sub_widget, - sub_region, - widget_region, - ((1, 0, 0),) if overlay else widget_order, - layer_order, - no_clip if overlay else sub_clip, - visible, - arrange_result.scroll_spacing, - ) - + if widget._cover_widget is None: + add_widget( + sub_widget, + sub_region, + widget_region, + ((1, 0, 0),) if overlay else widget_order, + layer_order, + no_clip if overlay else sub_clip, + visible, + arrange_result.scroll_spacing, + ) layer_order -= 1 if visible: @@ -715,7 +725,7 @@ def add_widget( ) map[widget] = _MapGeometry( - region + layout_offset, + (region + layout_offset), order, clip, total_region.size, @@ -737,7 +747,7 @@ def add_widget( if styles.constrain != "none": widget_region = self._constrain(styles, widget_region, no_clip) - map[widget] = _MapGeometry( + map[widget._render_widget] = _MapGeometry( widget_region, order, clip, diff --git a/src/textual/widget.py b/src/textual/widget.py index e55b937da7..e1673dcdf3 100644 --- a/src/textual/widget.py +++ b/src/textual/widget.py @@ -14,7 +14,6 @@ from typing import ( TYPE_CHECKING, AsyncGenerator, - Awaitable, ClassVar, Collection, Generator, @@ -58,7 +57,6 @@ from textual._styles_cache import StylesCache from textual._types import AnimationLevel from textual.actions import SkipAction -from textual.await_complete import AwaitComplete from textual.await_remove import AwaitRemove from textual.box_model import BoxModel from textual.cache import FIFOCache @@ -333,6 +331,38 @@ class Widget(DOMNode): loading: Reactive[bool] = Reactive(False) """If set to `True` this widget will temporarily be replaced with a loading indicator.""" + virtual_size: Reactive[Size] = Reactive(Size(0, 0), layout=True) + """The virtual (scrollable) [size][textual.geometry.Size] of the widget.""" + + has_focus: Reactive[bool] = Reactive(False, repaint=False) + """Does this widget have focus? Read only.""" + + mouse_hover: Reactive[bool] = Reactive(False, repaint=False) + """Is the mouse over this widget? Read only.""" + + scroll_x: Reactive[float] = Reactive(0.0, repaint=False, layout=False) + """The scroll position on the X axis.""" + + scroll_y: Reactive[float] = Reactive(0.0, repaint=False, layout=False) + """The scroll position on the Y axis.""" + + scroll_target_x = Reactive(0.0, repaint=False) + """Scroll target destination, X coord.""" + + scroll_target_y = Reactive(0.0, repaint=False) + """Scroll target destination, Y coord.""" + + show_vertical_scrollbar: Reactive[bool] = Reactive(False, layout=True) + """Show a vertical scrollbar?""" + + show_horizontal_scrollbar: Reactive[bool] = Reactive(False, layout=True) + """Show a horizontal scrollbar?""" + + border_title: str | Text | None = _BorderTitle() # type: ignore + """A title to show in the top border (if there is one).""" + border_subtitle: str | Text | None = _BorderTitle() # type: ignore + """A title to show in the bottom border (if there is one).""" + # Default sort order, incremented by constructor _sort_order: ClassVar[int] = 0 @@ -430,38 +460,8 @@ def __init__( """An anchored child widget, or `None` if no child is anchored.""" self._anchor_animate: bool = False """Flag to enable animation when scrolling anchored widgets.""" - - virtual_size: Reactive[Size] = Reactive(Size(0, 0), layout=True) - """The virtual (scrollable) [size][textual.geometry.Size] of the widget.""" - - has_focus: Reactive[bool] = Reactive(False, repaint=False) - """Does this widget have focus? Read only.""" - - mouse_hover: Reactive[bool] = Reactive(False, repaint=False) - """Is the mouse over this widget? Read only.""" - - scroll_x: Reactive[float] = Reactive(0.0, repaint=False, layout=False) - """The scroll position on the X axis.""" - - scroll_y: Reactive[float] = Reactive(0.0, repaint=False, layout=False) - """The scroll position on the Y axis.""" - - scroll_target_x = Reactive(0.0, repaint=False) - """Scroll target destination, X coord.""" - - scroll_target_y = Reactive(0.0, repaint=False) - """Scroll target destination, Y coord.""" - - show_vertical_scrollbar: Reactive[bool] = Reactive(False, layout=True) - """Show a vertical scrollbar?""" - - show_horizontal_scrollbar: Reactive[bool] = Reactive(False, layout=True) - """Show a horizontal scrollbar?""" - - border_title: str | Text | None = _BorderTitle() # type: ignore - """A title to show in the top border (if there is one).""" - border_subtitle: str | Text | None = _BorderTitle() # type: ignore - """A title to show in the bottom border (if there is one).""" + self._cover_widget: Widget | None = None + """Widget to render over this widget (used by loading indicator).""" @property def is_mounted(self) -> bool: @@ -587,6 +587,31 @@ def is_maximized(self) -> bool: except NoScreen: return False + @property + def _render_widget(self) -> Widget: + return self._cover_widget if self._cover_widget is not None else self + + def _cover(self, widget: Widget) -> None: + """Set a widget used to replace the visuals of this widget (used for loading indicator). + + Args: + widget: A newly constructed, but unmounted widget. + """ + self._uncover() + self._cover_widget = widget + widget._parent = self + widget._start_messages() + widget._post_register(self.app) + self.app.stylesheet.apply(widget) + self.refresh(layout=True) + + def _uncover(self) -> None: + """Remove any widget, previously set via [`_cover`][textual.widget.Widget._cover].""" + if self._cover_widget is not None: + self._cover_widget.remove() + self._cover_widget = None + self.refresh(layout=True) + def anchor(self, *, animate: bool = False) -> None: """Anchor the widget, which scrolls it into view (like [scroll_visible][textual.widget.Widget.scroll_visible]), but also keeps it in view if the widget's size changes, or the size of its container changes. @@ -716,7 +741,7 @@ def get_loading_widget(self) -> Widget: loading_widget = self.app.get_loading_widget() return loading_widget - def set_loading(self, loading: bool) -> Awaitable: + def set_loading(self, loading: bool) -> None: """Set or reset the loading state of this widget. A widget in a loading state will display a LoadingIndicator that obscures the widget. @@ -728,19 +753,16 @@ def set_loading(self, loading: bool) -> Awaitable: An optional awaitable. """ LOADING_INDICATOR_CLASS = "-textual-loading-indicator" - LOADING_INDICATOR_QUERY = f".{LOADING_INDICATOR_CLASS}" - remove_indicator = self.query_children(LOADING_INDICATOR_QUERY).remove() if loading: loading_indicator = self.get_loading_widget() loading_indicator.add_class(LOADING_INDICATOR_CLASS) - await_mount = self.mount(loading_indicator) - return AwaitComplete(remove_indicator, await_mount).call_next(self) + self._cover(loading_indicator) else: - return remove_indicator + self._uncover() - async def _watch_loading(self, loading: bool) -> None: + def _watch_loading(self, loading: bool) -> None: """Called when the 'loading' reactive is changed.""" - await self.set_loading(loading) + self.set_loading(loading) ExpectType = TypeVar("ExpectType", bound="Widget") @@ -3993,6 +4015,7 @@ def _on_scroll_to_region(self, message: messages.ScrollToRegion) -> None: self.scroll_to_region(message.region, animate=True) def _on_unmount(self) -> None: + self._uncover() self.workers.cancel_node(self) def action_scroll_home(self) -> None: diff --git a/src/textual/widgets/_loading_indicator.py b/src/textual/widgets/_loading_indicator.py index 6e93a7b94a..fdc31793c0 100644 --- a/src/textual/widgets/_loading_indicator.py +++ b/src/textual/widgets/_loading_indicator.py @@ -18,11 +18,12 @@ class LoadingIndicator(Widget): DEFAULT_CSS = """ LoadingIndicator { - width: 100%; - height: 100%; + # width: 100%; + # height: 100%; min-height: 1; content-align: center middle; color: $accent; + text-style: not reverse; } LoadingIndicator.-textual-loading-indicator { layer: _loading; From ed6f2ff4bf818120ec2517c526f7200f3a95ad13 Mon Sep 17 00:00:00 2001 From: Will McGugan Date: Wed, 2 Oct 2024 14:33:51 +0100 Subject: [PATCH 2/7] changlog --- CHANGELOG.md | 2 +- src/textual/_compositor.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1c7e14437a..001c665420 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,7 +10,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/). ### Fixed - Fixed issue with screen not updating when auto_refresh was enabled https://github.com/Textualize/textual/pull/5063 -- Fixed issues regarding loading indicator https://github.com/Textualize/textual/issues/3935 +- Fixed issues regarding loading indicator https://github.com/Textualize/textual/pull/5079 ### Added diff --git a/src/textual/_compositor.py b/src/textual/_compositor.py index 7a3f0c8fe7..9c9eff51e2 100644 --- a/src/textual/_compositor.py +++ b/src/textual/_compositor.py @@ -725,7 +725,7 @@ def add_widget( ) map[widget] = _MapGeometry( - (region + layout_offset), + region + layout_offset, order, clip, total_region.size, From 990c9b84610d8e6ac8b1a468d8bad3f3facdfe46 Mon Sep 17 00:00:00 2001 From: Will McGugan Date: Wed, 2 Oct 2024 14:35:16 +0100 Subject: [PATCH 3/7] docstring --- src/textual/widget.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/textual/widget.py b/src/textual/widget.py index e1673dcdf3..4177ea3302 100644 --- a/src/textual/widget.py +++ b/src/textual/widget.py @@ -589,6 +589,8 @@ def is_maximized(self) -> bool: @property def _render_widget(self) -> Widget: + """The widget the compositor should render.""" + # Will return the "cover widget" if one is set, otherwise self. return self._cover_widget if self._cover_widget is not None else self def _cover(self, widget: Widget) -> None: From 7b825bb456b7be2e74764cd957129c4238b264e9 Mon Sep 17 00:00:00 2001 From: Will McGugan Date: Wed, 2 Oct 2024 14:42:55 +0100 Subject: [PATCH 4/7] test fixes --- tests/animations/test_loading_indicator_animation.py | 7 +++---- tests/test_widget.py | 10 +++++----- 2 files changed, 8 insertions(+), 9 deletions(-) diff --git a/tests/animations/test_loading_indicator_animation.py b/tests/animations/test_loading_indicator_animation.py index 3f1df80a55..38bb6a7c27 100644 --- a/tests/animations/test_loading_indicator_animation.py +++ b/tests/animations/test_loading_indicator_animation.py @@ -4,7 +4,6 @@ """ from textual.app import App -from textual.widgets import LoadingIndicator async def test_loading_indicator_is_not_static_on_full() -> None: @@ -15,7 +14,7 @@ async def test_loading_indicator_is_not_static_on_full() -> None: async with app.run_test() as pilot: app.screen.loading = True await pilot.pause() - indicator = app.query_one(LoadingIndicator) + indicator = app.screen._cover_widget assert str(indicator.render()) != "Loading..." @@ -27,7 +26,7 @@ async def test_loading_indicator_is_not_static_on_basic() -> None: async with app.run_test() as pilot: app.screen.loading = True await pilot.pause() - indicator = app.query_one(LoadingIndicator) + indicator = app.screen._cover_widget assert str(indicator.render()) != "Loading..." @@ -39,5 +38,5 @@ async def test_loading_indicator_is_static_on_none() -> None: async with app.run_test() as pilot: app.screen.loading = True await pilot.pause() - indicator = app.query_one(LoadingIndicator) + indicator = app.screen._cover_widget assert str(indicator.render()) == "Loading..." diff --git a/tests/test_widget.py b/tests/test_widget.py index 146e4fbe96..d5e79c0c11 100644 --- a/tests/test_widget.py +++ b/tests/test_widget.py @@ -425,23 +425,23 @@ def compose(self) -> ComposeResult: app = pilot.app label = app.query_one(Label) assert label.loading == False - assert len(label.query(LoadingIndicator)) == 0 + assert label._cover_widget is None label.loading = True await pilot.pause() - assert len(label.query(LoadingIndicator)) == 1 + assert label._cover_widget is not None label.loading = True # Setting to same value is a null-op await pilot.pause() - assert len(label.query(LoadingIndicator)) == 1 + assert label._cover_widget is not None label.loading = False await pilot.pause() - assert len(label.query(LoadingIndicator)) == 0 + assert label._cover_widget is None label.loading = False # Setting to same value is a null-op await pilot.pause() - assert len(label.query(LoadingIndicator)) == 0 + assert label._cover_widget is None async def test_is_mounted_property(): From c995724b641c190fc06c7f60b35291e9db4f0d8d Mon Sep 17 00:00:00 2001 From: Will McGugan Date: Wed, 2 Oct 2024 14:46:07 +0100 Subject: [PATCH 5/7] not none --- src/textual/_compositor.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/textual/_compositor.py b/src/textual/_compositor.py index 9c9eff51e2..d7c32ac0a1 100644 --- a/src/textual/_compositor.py +++ b/src/textual/_compositor.py @@ -664,7 +664,7 @@ def add_widget( get_layer_index = layers_to_index.get - if widget._cover_widget: + if widget._cover_widget is not None: map[widget._cover_widget] = _MapGeometry( region.shrink(widget.styles.gutter), order, From 85b0f4efbbb909c87136419730d2f74871fa51f8 Mon Sep 17 00:00:00 2001 From: Will McGugan Date: Wed, 2 Oct 2024 14:47:18 +0100 Subject: [PATCH 6/7] changelog --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 001c665420..d9af5065a1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,10 @@ and this project adheres to [Semantic Versioning](http://semver.org/). - Added support for keymaps (user configurable key bindings) https://github.com/Textualize/textual/pull/5038 - Added descriptions to bindings for all internal widgets, and updated casing to be consistent https://github.com/Textualize/textual/pull/5062 +### Changed + +- Breaking change: `Widget.set_loading` no longer return an awaitable https://github.com/Textualize/textual/pull/5079 + ## [0.81.0] - 2024-09-25 ### Added From 13b5e3239d825036afbd716bf617c09358caf14f Mon Sep 17 00:00:00 2001 From: Will McGugan Date: Wed, 2 Oct 2024 14:47:47 +0100 Subject: [PATCH 7/7] restore redundant style --- src/textual/widgets/_loading_indicator.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/textual/widgets/_loading_indicator.py b/src/textual/widgets/_loading_indicator.py index fdc31793c0..0dae7324dc 100644 --- a/src/textual/widgets/_loading_indicator.py +++ b/src/textual/widgets/_loading_indicator.py @@ -18,8 +18,8 @@ class LoadingIndicator(Widget): DEFAULT_CSS = """ LoadingIndicator { - # width: 100%; - # height: 100%; + width: 100%; + height: 100%; min-height: 1; content-align: center middle; color: $accent;