simplify loading (#3816)

* simplify loading

* ws removal

* no need for this

* simplify

* gutter

* tests and refactor

* ws

* ws

* restore

* move to compositor

* words

* tweak timeout in snapshot

CHANGELOG.md

@@ -11,11 +11,16 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 
 - Fixed `DataTable.update_cell` not raising an error with an invalid column key https://github.com/Textualize/textual/issues/3335
 - Fixed `Input` showing suggestions when not focused https://github.com/Textualize/textual/pull/3808
+- Fixed loading indicator not covering scrollbars https://github.com/Textualize/textual/pull/3816
 
 ### Removed
 
 - Removed renderables/align.py which was no longer used
 
+### Added
+
+- Added `get_loading_widget` to Widget and App customize the loading widget. https://github.com/Textualize/textual/pull/3816
+
 ## [0.44.1] - 2023-12-4
 
 ### Fixed

src/textual/_compositor.py

@@ -594,7 +594,11 @@ def add_widget(
             # Widgets with scrollbars (containers or scroll view) require additional processing
             if widget.is_scrollable:
                 # The region that contains the content (container region minus scrollbars)
-                child_region = widget._get_scrollable_region(container_region)
+                child_region = (
+                    container_region
+                    if widget.loading
+                    else widget._get_scrollable_region(container_region)
+                )
 
                 # The region covered by children relative to parent widget
                 total_region = child_region.reset_offset

src/textual/app.py

@@ -776,7 +776,10 @@ def focused(self) -> Widget | None:
         Returns:
             The currently focused widget, or `None` if nothing is focused.
         """
-        return self.screen.focused
+        focused = self.screen.focused
+        if focused is not None and focused.loading:
+            return None
+        return focused
 
     @property
     def namespace_bindings(self) -> dict[str, tuple[DOMNode, Binding]]:
@@ -993,6 +996,18 @@ def _log(
         except Exception as error:
             self._handle_exception(error)
 
+    def get_loading_widget(self) -> Widget:
+        """Get a widget to be used as a loading indicator.
+
+        Extend this method if you want to display the loading state a little differently.
+
+        Returns:
+            A widget to display a loading state.
+        """
+        from .widgets import LoadingIndicator
+
+        return LoadingIndicator()
+
     def call_from_thread(
         self,
         callback: Callable[..., CallThreadReturnType | Awaitable[CallThreadReturnType]],

src/textual/screen.py

@@ -354,7 +354,7 @@ def focus_chain(self) -> list[Widget]:
             if node is None:
                 pop()
             else:
-                if node.disabled:
+                if node._check_disabled():
                     continue
                 node_styles_visibility = node.styles.get_rule("visibility")
                 node_is_visible = (

src/textual/widget.py

@@ -437,6 +437,8 @@ def allow_vertical_scroll(self) -> bool:
 
         May be overridden if you want different logic regarding allowing scrolling.
         """
+        if self._check_disabled():
+            return False
         return self.is_scrollable and self.show_vertical_scrollbar
 
     @property
@@ -445,6 +447,8 @@ def allow_horizontal_scroll(self) -> bool:
 
         May be overridden if you want different logic regarding allowing scrolling.
         """
+        if self._check_disabled():
+            return False
         return self.is_scrollable and self.show_horizontal_scrollbar
 
     @property
@@ -481,6 +485,15 @@ def opacity(self) -> float:
                 break
         return opacity
 
+    def _check_disabled(self) -> bool:
+        """Check if the widget is disabled either explicitly by setting `disabled`,
+        or implicitly by setting `loading`.
+
+        Returns:
+            True if the widget should be disabled.
+        """
+        return self.disabled or self.loading
+
     @property
     def tooltip(self) -> RenderableType | None:
         """Tooltip for the widget, or `None` for no tooltip."""
@@ -528,6 +541,17 @@ def __exit__(
         else:
             self.app._composed[-1].append(composed)
 
+    def get_loading_widget(self) -> Widget:
+        """Get a widget to display a loading indicator.
+
+        The default implementation will defer to App.get_loading_widget.
+
+        Returns:
+            A widget in place of this widget to indicate a loading.
+        """
+        loading_widget = self.app.get_loading_widget()
+        return loading_widget
+
     def set_loading(self, loading: bool) -> Awaitable:
         """Set or reset the loading state of this widget.
 
@@ -539,13 +563,15 @@ def set_loading(self, loading: bool) -> Awaitable:
         Returns:
             An optional awaitable.
         """
-        from textual.widgets import LoadingIndicator
 
         if loading:
-            loading_indicator = LoadingIndicator()
-            return loading_indicator.apply(self)
+            loading_indicator = self.get_loading_widget()
+            loading_indicator.add_class("-textual-loading-indicator")
+            await_mount = self.mount(loading_indicator)
+            return await_mount
         else:
-            return LoadingIndicator.clear(self)
+            await_remove = self.query(".-textual-loading-indicator").remove()
+            return await_remove
 
     async def _watch_loading(self, loading: bool) -> None:
         """Called when the 'loading' reactive is changed."""
@@ -1569,7 +1595,12 @@ def _self_or_ancestors_disabled(self) -> bool:
     @property
     def focusable(self) -> bool:
         """Can this widget currently be focused?"""
-        return self.can_focus and self.visible and not self._self_or_ancestors_disabled
+        return (
+            not self.loading
+            and self.can_focus
+            and self.visible
+            and not self._self_or_ancestors_disabled
+        )
 
     @property
     def _focus_sort_key(self) -> tuple[int, int]:

src/textual/widgets/_loading_indicator.py

@@ -1,17 +1,14 @@
 from __future__ import annotations
 
 from time import time
-from typing import Awaitable, ClassVar
-from weakref import WeakKeyDictionary
 
 from rich.console import RenderableType
 from rich.style import Style
 from rich.text import Text
 
 from ..color import Gradient
-from ..css.query import NoMatches
 from ..events import Mount
-from ..widget import AwaitMount, Widget
+from ..widget import Widget
 
 
 class LoadingIndicator(Widget):
@@ -25,23 +22,13 @@ class LoadingIndicator(Widget):
         content-align: center middle;
         color: $accent;
     }
-    LoadingIndicator.-overlay {
+    LoadingIndicator.-textual-loading-indicator {
         layer: _loading;
         background: $boost;
+        dock: top;
     }
     """
 
-    _widget_state: ClassVar[
-        WeakKeyDictionary[Widget, tuple[bool, str, str]]
-    ] = WeakKeyDictionary()
-    """Widget state that must be restore after loading.
-    
-    The tuples indicate the original values of the:
-     - widget disabled state;
-     - widget style overflow_x rule; and
-     - widget style overflow_y rule.
-    """
-
     def __init__(
         self,
         name: str | None = None,
@@ -62,57 +49,6 @@ def __init__(
         self._start_time: float = 0.0
         """The time the loading indicator was mounted (a Unix timestamp)."""
 
-    def apply(self, widget: Widget) -> AwaitMount:
-        """Apply the loading indicator to a `widget`.
-
-        This will overlay the given widget with a loading indicator.
-
-        Args:
-            widget: A widget.
-
-        Returns:
-            An awaitable for mounting the indicator.
-        """
-        self.add_class("-overlay")
-        await_mount = widget.mount(self)
-        self._widget_state[widget] = (
-            widget.disabled,
-            widget.styles.overflow_x,
-            widget.styles.overflow_y,
-        )
-        widget.styles.overflow_x = "hidden"
-        widget.styles.overflow_y = "hidden"
-        widget.disabled = True
-        return await_mount
-
-    @classmethod
-    def clear(cls, widget: Widget) -> Awaitable[None]:
-        """Clear any loading indicator from the given widget.
-
-        Args:
-            widget: Widget to clear the loading indicator from.
-
-        Returns:
-            Optional awaitable.
-        """
-        try:
-            await_remove = widget.get_child_by_type(cls).remove()
-        except NoMatches:
-
-            async def null() -> None:
-                """Nothing to remove"""
-                return None
-
-            await_remove = null()
-
-        if widget in cls._widget_state:
-            disabled, overflow_x, overflow_y = cls._widget_state[widget]
-            widget.disabled = disabled
-            widget.styles.overflow_x = overflow_x
-            widget.styles.overflow_y = overflow_y
-
-        return await_remove
-
     def _on_mount(self, _: Mount) -> None:
         self._start_time = time()
         self.auto_refresh = 1 / 16

tests/snapshot_tests/snapshot_apps/loading.py

@@ -0,0 +1,56 @@
+from rich.console import RenderableType
+
+from textual.app import App, ComposeResult
+from textual.containers import Horizontal
+from textual.widget import Widget
+from textual.widgets import OptionList
+
+
+class SimpleLoadingIndicator(Widget):
+    """A loading indicator that doesn't animate."""
+
+    DEFAULT_CSS = """
+    SimpleLoadingIndicator {
+        width: 100%;
+        height: 100%;
+        min-height: 1;
+        content-align: center middle;
+        color: $accent;        
+    }
+    SimpleLoadingIndicator.-textual-loading-indicator {
+        layer: _loading;
+        background: $boost;     
+        dock: top;
+    }
+    """
+
+    def render(self) -> RenderableType:
+        return "Loading!"
+
+
+class LoadingOverlayRedux(App[None]):
+    CSS = """
+    OptionList {
+        scrollbar-gutter: stable;
+        width: 1fr;
+        height: 1fr;
+    }
+    """
+
+    BINDINGS = [("space", "toggle")]
+
+    def get_loading_widget(self) -> Widget:
+        return SimpleLoadingIndicator()
+
+    def compose(self) -> ComposeResult:
+        with Horizontal():
+            yield OptionList(*[("hello world " * 5) for _ in range(100)])
+            yield OptionList(*[("foo bar" * 5) for _ in range(100)])
+
+    def action_toggle(self) -> None:
+        option_list = self.query(OptionList).first()
+        option_list.loading = not option_list.loading
+
+
+if __name__ == "__main__":
+    LoadingOverlayRedux().run()

tests/snapshot_tests/snapshot_apps/notifications_above_loading.py

@@ -18,14 +18,12 @@ class LoadingOverlayApp(App[None]):
 
     def compose(self) -> ComposeResult:
         with VerticalScroll():
-            yield Label("another big label\n"*30)  # Ensure there's a scrollbar.
+            yield Label("another big label\n" * 30)  # Ensure there's a scrollbar.
 
     def on_mount(self):
-        self.notify(
-            "This is a big notification.\n" * 10,
-            timeout=9_999_999
-        )
+        self.notify("This is a big notification.\n" * 10, timeout=10)
         self.query_one(VerticalScroll).loading = True
 
+
 if __name__ == "__main__":
     LoadingOverlayApp().run()

tests/snapshot_tests/test_snapshots.py

@@ -898,9 +898,11 @@ def test_unscoped_css(snap_compare) -> None:
 def test_big_buttons(snap_compare) -> None:
     assert snap_compare(SNAPSHOT_APPS_DIR / "big_button.py")
 
+
 def test_keyline(snap_compare) -> None:
     assert snap_compare(SNAPSHOT_APPS_DIR / "keyline.py")
 
+
 def test_button_outline(snap_compare):
     """Outline style rendered incorrectly when applied to a `Button` widget.
 
@@ -934,3 +936,16 @@ def test_vertical_max_height(snap_compare):
     """Test vertical max height takes border in to account."""
     assert snap_compare(SNAPSHOT_APPS_DIR / "vertical_max_height.py")
 
+
+def test_loading_indicator(snap_compare):
+    """Test loading indicator."""
+    # https://github.com/Textualize/textual/pull/3816
+    assert snap_compare(SNAPSHOT_APPS_DIR / "loading.py", press=["space"])
+
+
+def test_loading_indicator_disables_widget(snap_compare):
+    """Test loading indicator disabled widget."""
+    # https://github.com/Textualize/textual/pull/3816
+    assert snap_compare(
+        SNAPSHOT_APPS_DIR / "loading.py", press=["space", "down", "down", "space"]
+    )

tests/test_loading.py

@@ -25,12 +25,9 @@ async def test_loading_disables_and_remove_scrollbars():
     async with app.run_test() as pilot:
         vs = app.query_one(VerticalScroll)
         # Sanity checks:
-        assert not vs.disabled
-        assert vs.styles.overflow_y != "hidden"
+        assert not vs._check_disabled()
 
         await pilot.press("l")
         await pilot.pause()
 
-        assert vs.disabled
-        assert vs.styles.overflow_x == "hidden"
-        assert vs.styles.overflow_y == "hidden"
+        assert vs._check_disabled()