Merge pull request #1659 from Textualize/fix-1372

Keep track of scheduled animations

CHANGELOG.md

@@ -7,6 +7,11 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 
 ## [0.11.0] - Unreleased
 
+### Added
+
+- Added the coroutines `Animator.wait_until_complete` and `pilot.wait_for_scheduled_animations` that allow waiting for all current and scheduled animations https://github.com/Textualize/textual/issues/1658
+- Added the method `Animator.is_being_animated` that checks if an attribute of an object is being animated or is scheduled for animation
+
 ### Changed
 
 - Breaking change: `TreeNode` can no longer be imported from `textual.widgets`; it is now available via `from textual.widgets.tree import TreeNode`. https://github.com/Textualize/textual/pull/1637
@@ -15,6 +20,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 
 - Fixed stuck screen  https://github.com/Textualize/textual/issues/1632
 - Fixed relative units in `grid-rows` and `grid-columns` being computed with respect to the wrong dimension https://github.com/Textualize/textual/issues/1406
+- Fixed bug with animations that were triggered back to back, where the second one wouldn't start https://github.com/Textualize/textual/issues/1372
+- Fixed bug with animations that were scheduled where all but the first would be skipped https://github.com/Textualize/textual/issues/1372
 - Programmatically setting `overflow_x`/`overflow_y` refreshes the layout correctly https://github.com/Textualize/textual/issues/1616
 - Fixed double-paste into `Input` https://github.com/Textualize/textual/issues/1657
 - Added a workaround for an apparent Windows Terminal paste issue https://github.com/Textualize/textual/issues/1661

src/textual/_animator.py

@@ -21,6 +21,9 @@ else:  # pragma: no cover
 if TYPE_CHECKING:
     from textual.app import App
 
+    AnimationKey = tuple[int, str]
+    """Animation keys are the id of the object and the attribute being animated."""
+
 EasingFunction = Callable[[float], float]
 
 
@@ -166,10 +169,19 @@ class BoundAnimator:
 
 
 class Animator:
-    """An object to manage updates to a given attribute over a period of time."""
+    """An object to manage updates to a given attribute over a period of time.
+
+    Attrs:
+        _animations: Dictionary that maps animation keys to the corresponding animation
+            instances.
+        _scheduled: Keys corresponding to animations that have been scheduled but not yet
+            started.
+        app: The app that owns the animator object.
+    """
 
     def __init__(self, app: App, frames_per_second: int = 60) -> None:
-        self._animations: dict[tuple[object, str], Animation] = {}
+        self._animations: dict[AnimationKey, Animation] = {}
+        self._scheduled: set[AnimationKey] = set()
         self.app = app
         self._timer = Timer(
             app,
@@ -179,11 +191,15 @@ class Animator:
             callback=self,
             pause=True,
         )
+        # Flag if no animations are currently taking place.
         self._idle_event = asyncio.Event()
+        # Flag if no animations are currently taking place and none are scheduled.
+        self._complete_event = asyncio.Event()
 
     async def start(self) -> None:
         """Start the animator task."""
         self._idle_event.set()
+        self._complete_event.set()
         self._timer.start()
 
     async def stop(self) -> None:
@@ -194,11 +210,17 @@ class Animator:
             pass
         finally:
             self._idle_event.set()
+            self._complete_event.set()
 
     def bind(self, obj: object) -> BoundAnimator:
-        """Bind the animator to a given objects."""
+        """Bind the animator to a given object."""
         return BoundAnimator(self, obj)
 
+    def is_being_animated(self, obj: object, attribute: str) -> bool:
+        """Does the object/attribute pair have an ongoing or scheduled animation?"""
+        key = (id(obj), attribute)
+        return key in self._animations or key in self._scheduled
+
     def animate(
         self,
         obj: object,
@@ -237,6 +259,8 @@ class Animator:
             on_complete=on_complete,
         )
         if delay:
+            self._scheduled.add((id(obj), attribute))
+            self._complete_event.clear()
             self.app.set_timer(delay, animate_callback)
         else:
             animate_callback()
@@ -273,13 +297,14 @@ class Animator:
             duration is None and speed is not None
         ), "An Animation should have a duration OR a speed"
 
+        animation_key = (id(obj), attribute)
+        self._scheduled.discard(animation_key)
+
         if final_value is ...:
             final_value = value
 
         start_time = self._get_time()
 
-        animation_key = (id(obj), attribute)
-
         easing_function = EASING[easing] if isinstance(easing, str) else easing
 
         animation: Animation | None = None
@@ -342,11 +367,14 @@ class Animator:
         self._animations[animation_key] = animation
         self._timer.resume()
         self._idle_event.clear()
+        self._complete_event.clear()
 
     async def __call__(self) -> None:
         if not self._animations:
             self._timer.pause()
             self._idle_event.set()
+            if not self._scheduled:
+                self._complete_event.set()
         else:
             animation_time = self._get_time()
             animation_keys = list(self._animations.keys())
@@ -368,3 +396,7 @@ class Animator:
     async def wait_for_idle(self) -> None:
         """Wait for any animations to complete."""
         await self._idle_event.wait()
+
+    async def wait_until_complete(self) -> None:
+        """Wait for any current and scheduled animations to complete."""
+        await self._complete_event.wait()

src/textual/css/stylesheet.py

@@ -449,6 +449,8 @@ class Stylesheet:
         get_new_render_rule = new_render_rules.get
 
         if animate:
+            animator = node.app.animator
+            base = node.styles.base
             for key in modified_rule_keys:
                 # Get old and new render rules
                 old_render_value = get_current_render_rule(key)
@@ -456,13 +458,18 @@ class Stylesheet:
                 # Get new rule value (may be None)
                 new_value = rules.get(key)
 
-                # Check if this can / should be animated
+                # Check if this can / should be animated. It doesn't suffice to check
-                if is_animatable(key) and new_render_value != old_render_value:
+                # if the current and target values are different because a previous
+                # animation may have been scheduled but may have not started yet.
+                if is_animatable(key) and (
+                    new_render_value != old_render_value
+                    or animator.is_being_animated(base, key)
+                ):
                     transition = new_styles._get_transition(key)
                     if transition is not None:
                         duration, easing, delay = transition
-                        node.app.animator.animate(
+                        animator.animate(
-                            node.styles.base,
+                            base,
                             key,
                             new_render_value,
                             final_value=new_value,

src/textual/pilot.py

@@ -43,9 +43,13 @@ class Pilot(Generic[ReturnType]):
         await asyncio.sleep(delay)
 
     async def wait_for_animation(self) -> None:
-        """Wait for any animation to complete."""
+        """Wait for any current animation to complete."""
         await self._app.animator.wait_for_idle()
 
+    async def wait_for_scheduled_animations(self) -> None:
+        """Wait for any current and scheduled animations to complete."""
+        await self._app.animator.wait_until_complete()
+
     async def exit(self, result: ReturnType) -> None:
         """Exit the app with the given result.
 

tests/test_animation.py

@@ -9,7 +9,7 @@ class AnimApp(App):
     #foo {
         height: 1;
     }
-    
+
     """
 
     def compose(self) -> ComposeResult:
@@ -37,3 +37,124 @@ async def test_animate_height() -> None:
         assert elapsed >= 0.5
         # Check the height reached the maximum
         assert static.styles.height.value == 100
+
+
+async def test_scheduling_animation() -> None:
+    """Test that scheduling an animation works."""
+
+    app = AnimApp()
+    delay = 0.1
+
+    async with app.run_test() as pilot:
+        styles = app.query_one(Static).styles
+        styles.background = "black"
+
+        styles.animate("background", "white", delay=delay, duration=0)
+
+        await pilot.pause(0.9 * delay)
+        assert styles.background.rgb == (0, 0, 0)  # Still black
+
+        await pilot.wait_for_scheduled_animations()
+        assert styles.background.rgb == (255, 255, 255)
+
+
+async def test_wait_for_current_animations() -> None:
+    """Test that we can wait only for the current animations taking place."""
+
+    app = AnimApp()
+
+    delay = 10
+
+    async with app.run_test() as pilot:
+        styles = app.query_one(Static).styles
+        styles.animate("height", 100, duration=0.1)
+        start = perf_counter()
+        styles.animate("height", 200, duration=0.1, delay=delay)
+
+        # Wait for the first animation to finish
+        await pilot.wait_for_animation()
+        elapsed = perf_counter() - start
+        assert elapsed < (delay / 2)
+
+
+async def test_wait_for_current_and_scheduled_animations() -> None:
+    """Test that we can wait for current and scheduled animations."""
+
+    app = AnimApp()
+
+    async with app.run_test() as pilot:
+        styles = app.query_one(Static).styles
+
+        start = perf_counter()
+        styles.animate("height", 50, duration=0.01)
+        styles.animate("background", "black", duration=0.01, delay=0.05)
+
+        await pilot.wait_for_scheduled_animations()
+        elapsed = perf_counter() - start
+        assert elapsed >= 0.06
+        assert styles.background.rgb == (0, 0, 0)
+
+
+async def test_reverse_animations() -> None:
+    """Test that you can create reverse animations.
+
+    Regression test for #1372 https://github.com/Textualize/textual/issues/1372
+    """
+
+    app = AnimApp()
+
+    async with app.run_test() as pilot:
+        static = app.query_one(Static)
+        styles = static.styles
+
+        # Starting point.
+        styles.background = "black"
+        assert styles.background.rgb == (0, 0, 0)
+
+        # First, make sure we can go from black to white and back, step by step.
+        styles.animate("background", "white", duration=0.01)
+        await pilot.wait_for_animation()
+        assert styles.background.rgb == (255, 255, 255)
+
+        styles.animate("background", "black", duration=0.01)
+        await pilot.wait_for_animation()
+        assert styles.background.rgb == (0, 0, 0)
+
+        # Now, the actual test is to make sure we go back to black if creating both at once.
+        styles.animate("background", "white", duration=0.01)
+        styles.animate("background", "black", duration=0.01)
+        await pilot.wait_for_animation()
+        assert styles.background.rgb == (0, 0, 0)
+
+
+async def test_schedule_reverse_animations() -> None:
+    """Test that you can schedule reverse animations.
+
+    Regression test for #1372 https://github.com/Textualize/textual/issues/1372
+    """
+
+    app = AnimApp()
+
+    async with app.run_test() as pilot:
+        static = app.query_one(Static)
+        styles = static.styles
+
+        # Starting point.
+        styles.background = "black"
+        assert styles.background.rgb == (0, 0, 0)
+
+        # First, make sure we can go from black to white and back, step by step.
+        styles.animate("background", "white", delay=0.01, duration=0.01)
+        await pilot.wait_for_scheduled_animations()
+        assert styles.background.rgb == (255, 255, 255)
+
+        styles.animate("background", "black", delay=0.01, duration=0.01)
+        await pilot.wait_for_scheduled_animations()
+        assert styles.background.rgb == (0, 0, 0)
+
+        # Now, the actual test is to make sure we go back to black if scheduling both at once.
+        styles.animate("background", "white", delay=0.01, duration=0.01)
+        await pilot.pause(0.005)
+        styles.animate("background", "black", delay=0.01, duration=0.01)
+        await pilot.wait_for_scheduled_animations()
+        assert styles.background.rgb == (0, 0, 0)