Merge pull request #6139 from Textualize/fix-progress-bar-crash

fix for progress bar crash

Author: willmcgugan

CHANGELOG.md

@@ -15,6 +15,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 - The :hover pseudo-class no applies to the first widget under the mouse with a hover style set https://github.com/Textualize/textual/pull/6132
 - The footer key hover background is more visible https://github.com/Textualize/textual/pull/6132
 - Made `App.delay_update` public https://github.com/Textualize/textual/pull/6137
+- Pilot.click will return True if the initial mouse down is on the specified target https://github.com/Textualize/textual/pull/6139
 
 ### Added
 

src/textual/app.py

@@ -2099,7 +2099,7 @@ async def run_app(app: App[ReturnType]) -> None:
 
         # Launch the app in the "background"
 
-        app_task = create_task(run_app(app), name=f"run_test {app}")
+        self._task = app_task = create_task(run_app(app), name=f"run_test {app}")
 
         # Wait until the app has performed all startup routines.
         await app_ready_event.wait()

src/textual/message_pump.py

@@ -457,19 +457,26 @@ def call_after_refresh(self, callback: Callback, *args: Any, **kwargs: Any) -> b
         message = messages.InvokeLater(partial(callback, *args, **kwargs))
         return self.post_message(message)
 
-    async def wait_for_refresh(self) -> None:
+    async def wait_for_refresh(self) -> bool:
         """Wait for the next refresh.
 
         This method should only be called from a task other than the one running this widget.
         If called from the same task, it will return immediately to avoid blocking the event loop.
 
-        """
+        Returns:
+            `True` if waiting for refresh was successful, or `False` if the call was a null-op
+                due to calling it within the node's own task.
 
-        if self._task is None or asyncio.current_task() is not self._task:
-            return
+        """
+        assert (
+            self._task is not None
+        ), "Node must be running before calling wait_for_refresh"
+        if asyncio.current_task() is self._task:
+            return False
         refreshed_event = asyncio.Event()
         self.call_after_refresh(refreshed_event.set)
         await refreshed_event.wait()
+        return True
 
     def call_later(self, callback: Callback, *args: Any, **kwargs: Any) -> bool:
         """Schedule a callback to run after all messages are processed in this object.

src/textual/pilot.py

@@ -227,8 +227,8 @@ async def click(
             OutOfBounds: If the position to be clicked is outside of the (visible) screen.
 
         Returns:
-            True if no selector was specified or if the click landed on the selected
-                widget, False otherwise.
+            `True` if no selector was specified or if the selected widget was under the mouse
+                when the click was initiated. `False` is the selected widget was not under the pointer.
         """
         try:
             return await self._post_mouse_events(
@@ -284,8 +284,8 @@ async def double_click(
             OutOfBounds: If the position to be clicked is outside of the (visible) screen.
 
         Returns:
-            True if no selector was specified or if the clicks landed on the selected
-                widget, False otherwise.
+            `True` if no selector was specified or if the selected widget was under the mouse
+                when the click was initiated. `False` is the selected widget was not under the pointer.
         """
         return await self.click(widget, offset, shift, meta, control, times=2)
 
@@ -329,8 +329,8 @@ async def triple_click(
             OutOfBounds: If the position to be clicked is outside of the (visible) screen.
 
         Returns:
-            True if no selector was specified or if the clicks landed on the selected
-                widget, False otherwise.
+            `True` if no selector was specified or if the selected widget was under the mouse
+                when the click was initiated. `False` is the selected widget was not under the pointer.
         """
         return await self.click(widget, offset, shift, meta, control, times=3)
 
@@ -414,7 +414,7 @@ async def _post_mouse_events(
         elif isinstance(widget, Widget):
             target_widget = widget
         else:
-            target_widget = app.screen.query_one(widget)
+            target_widget = screen.query_one(widget)
 
         message_arguments = _get_mouse_message_arguments(
             target_widget,
@@ -434,6 +434,7 @@ async def _post_mouse_events(
         widget_at = None
         for chain in range(1, times + 1):
             for mouse_event_cls in events:
+                await self.pause()
                 # Get the widget under the mouse before the event because the app might
                 # react to the event and move things around. We override on each iteration
                 # because we assume the final event in `events` is the actual event we care
@@ -444,16 +445,17 @@ async def _post_mouse_events(
                 if mouse_event_cls is Click:
                     kwargs = {**kwargs, "chain": chain}
 
-                widget_at, _ = app.get_widget_at(*offset)
+                if widget_at is None:
+                    widget_at, _ = app.get_widget_at(*offset)
                 event = mouse_event_cls(**kwargs)
                 # Bypass event processing in App.on_event. Because App.on_event
                 # is responsible for updating App.mouse_position, and because
                 # that's useful to other things (tooltip handling, for example),
                 # we patch the offset in there as well.
                 app.mouse_position = offset
                 screen._forward_event(event)
-                await self.pause()
 
+        await self.pause()
         return widget is None or widget_at is target_widget
 
     async def _wait_for_screen(self, timeout: float = 30.0) -> bool:

src/textual/widgets/_footer.py

@@ -1,6 +1,5 @@
 from __future__ import annotations
 
-import asyncio
 from collections import defaultdict
 from itertools import groupby
 from typing import TYPE_CHECKING
@@ -327,7 +326,7 @@ def compose(self) -> ComposeResult:
                     tooltip=binding.tooltip or binding.description,
                 )
 
-    async def bindings_changed(self, screen: Screen) -> None:
+    def bindings_changed(self, screen: Screen) -> None:
         self._bindings_ready = True
         if not screen.app.app_focus:
             return
@@ -348,9 +347,7 @@ def _on_mouse_scroll_up(self, event: events.MouseScrollUp) -> None:
                 event.stop()
                 event.prevent_default()
 
-    async def on_mount(self) -> None:
-        await asyncio.sleep(0)
-        self.call_next(self.bindings_changed, self.screen)
+    def on_mount(self) -> None:
         self.screen.bindings_updated_signal.subscribe(self, self.bindings_changed)
 
     def on_unmount(self) -> None:

src/textual/widgets/_progress_bar.py

@@ -127,7 +127,11 @@ def render_indeterminate(self) -> RenderResult:
         else:
             speed = 30  # Cells per second.
             # Compute the position of the bar.
-            start = (speed * self._clock.time) % (2 * total_imaginary_width)
+            start = (
+                (speed * self._clock.time) % (2 * total_imaginary_width)
+                if total_imaginary_width
+                else 0
+            )
             if start > total_imaginary_width:
                 # If the bar is to the right of its width, wrap it back from right to left.
                 start = 2 * total_imaginary_width - start  # = (tiw - (start - tiw))

tests/snapshot_tests/test_snapshots.py

@@ -4630,7 +4630,6 @@ def compose(self) -> ComposeResult:
 
 def test_long_textarea_placeholder(snap_compare) -> None:
     """Test multi-line placeholders are wrapped and rendered.
-
     You should see a TextArea at 50% width, with several lines of wrapped text.
     """
 

tests/test_modal.py

@@ -68,7 +68,7 @@ class ModalApp(App):
 
     def compose(self) -> ComposeResult:
         yield Header()
-        yield Label(TEXT * 8)
+        yield Label(TEXT)
         yield Footer()
 
     def action_request_quit(self) -> None:
@@ -90,12 +90,10 @@ async def test_modal_pop_screen():
     async with app.run_test() as pilot:
         # Pause to ensure the footer is fully composed to avoid flakiness in CI
         await pilot.pause()
-        await app.wait_for_refresh()
-        await pilot.pause()
-        # Check clicking the footer brings up the quit screen
-        await pilot.click(Footer, offset=(1, 0))
-        await pilot.pause()
-        assert isinstance(pilot.app.screen, QuitScreen)
+        await pilot.pause()  # Required in Windows
+        assert await pilot.click("FooterKey")
+        assert await app.wait_for_refresh()
+        assert isinstance(app.screen, QuitScreen)
         # Check activating the quit button exits the app
         await pilot.press("enter")
         assert pilot.app._exit

tests/test_progress_bar.py

@@ -3,7 +3,7 @@
 from rich.console import Console
 from rich.text import Text
 
-from textual.app import App
+from textual.app import App, ComposeResult
 from textual.color import Gradient
 from textual.css.query import NoMatches
 from textual.renderables.bar import _apply_gradient
@@ -181,3 +181,25 @@ def test_apply_gradient():
     _apply_gradient(text, gradient, 1)
     console = Console()
     assert text.get_style_at_offset(console, 0).color.triplet == (255, 0, 0)
+
+
+async def test_progress_bar_width_1fr(snap_compare):
+    """Regression test for https://github.com/Textualize/textual/issues/6127
+
+    Just shouldn't crash...
+    """
+
+    class WideBarApp(App[None]):
+
+        CSS = """
+        ProgressBar Bar {
+            width: 1fr;
+        }
+        """
+
+        def compose(self) -> ComposeResult:
+            yield ProgressBar()
+
+    app = WideBarApp()
+    async with app.run_test() as pilot:
+        await pilot.pause()

tests/test_reactive.py

@@ -545,7 +545,7 @@ def on_mount(self):
         def update(self):
             self.holder.attr = "hello world"
 
-        async def callback(self):
+        def callback(self):
             nonlocal from_app
             from_app = True