Cancelling scrolling animations on new scroll_to calls (#4081)

* Ensure prior scrolling animations dont interfere with new scroll_to calls

* Adding test for animator force cancellation

* Updating changelog

* Different approach

* Running on_complete later

* Scheduling on_complete callback after animation completes rather than immediately invoking

* Reverting _scroll_to implementation

Author: darrenburns

CHANGELOG.md

@@ -18,6 +18,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 - Made `textual.cache` (formerly `textual._cache`) public https://github.com/Textualize/textual/pull/3976
 - `Tab.label` can now be used to change the label of a tab https://github.com/Textualize/textual/pull/3979
 - Changed the default notification timeout from 3 to 5 seconds https://github.com/Textualize/textual/pull/4059
+- Prior scroll animations are now cancelled on new scrolls https://github.com/Textualize/textual/pull/4081
 
 ### Added
 

src/textual/_animator.py

@@ -409,7 +409,11 @@ def _animate(
                 end_value=value,
                 final_value=final_value,
                 easing=easing_function,
-                on_complete=on_complete,
+                on_complete=(
+                    partial(self.app.call_later, on_complete)
+                    if on_complete is not None
+                    else None
+                ),
             )
         assert animation is not None, "animation expected to be non-None"
 
@@ -483,7 +487,34 @@ async def stop_animation(
         elif key in self._animations:
             await self._stop_running_animation(key, complete)
 
-    async def __call__(self) -> None:
+    def force_stop_animation(self, obj: object, attribute: str) -> None:
+        """Force stop an animation on an attribute. This will immediately stop the animation,
+        without running any associated callbacks, setting the attribute to its final value.
+
+        Args:
+            obj: The object containing the attribute.
+            attribute: The name of the attribute.
+
+        Note:
+            If there is no animation scheduled or running, this is a no-op.
+        """
+        from .css.scalar_animation import ScalarAnimation
+
+        animation_key = (id(obj), attribute)
+        try:
+            animation = self._animations.pop(animation_key)
+        except KeyError:
+            return
+
+        if isinstance(animation, SimpleAnimation):
+            setattr(obj, attribute, animation.end_value)
+        elif isinstance(animation, ScalarAnimation):
+            setattr(obj, attribute, animation.final_value)
+
+        if animation.on_complete is not None:
+            animation.on_complete()
+
+    def __call__(self) -> None:
         if not self._animations:
             self._timer.pause()
             self._idle_event.set()
@@ -497,7 +528,8 @@ async def __call__(self) -> None:
                 animation_complete = animation(animation_time)
                 if animation_complete:
                     del self._animations[animation_key]
-                    await animation.invoke_callback()
+                    if animation.on_complete is not None:
+                        animation.on_complete()
 
     def _get_time(self) -> float:
         """Get the current wall clock time, via the internal Timer.
@@ -506,7 +538,7 @@ def _get_time(self) -> float:
             The wall clock time.
         """
         # N.B. We could remove this method and always call `self._timer.get_time()` internally,
-        # but it's handy to have in mocking situations
+        # but it's handy to have in mocking situations.
         return _time.get_time()
 
     async def wait_for_idle(self) -> None:

src/textual/css/styles.py

@@ -2,7 +2,7 @@
 
 from abc import ABC, abstractmethod
 from dataclasses import dataclass, field
-from functools import lru_cache
+from functools import lru_cache, partial
 from operator import attrgetter
 from typing import TYPE_CHECKING, Any, Iterable, NamedTuple, cast
 
@@ -395,7 +395,11 @@ def __textual_animation__(
                 duration=duration,
                 speed=speed,
                 easing=easing,
-                on_complete=on_complete,
+                on_complete=(
+                    partial(self.node.app.call_later, on_complete)
+                    if on_complete is not None
+                    else None
+                ),
             )
         return None
 

src/textual/widget.py

@@ -1911,6 +1911,11 @@ def _scroll_to(
         maybe_scroll_x = x is not None and (self.allow_horizontal_scroll or force)
         maybe_scroll_y = y is not None and (self.allow_vertical_scroll or force)
         scrolled_x = scrolled_y = False
+
+        animator = self.app.animator
+        animator.force_stop_animation(self, "scroll_x")
+        animator.force_stop_animation(self, "scroll_y")
+
         if animate:
             # TODO: configure animation speed
             if duration is None and speed is None:

tests/test_animator.py

@@ -203,19 +203,19 @@ async def test_animator():
     assert animator._animations[(id(animate_test), "foo")] == expected
     assert not animator._on_animation_frame_called
 
-    await animator()
+    animator()
     assert animate_test.foo == 0
 
     animator._time = 5
-    await animator()
+    animator()
     assert animate_test.foo == 50
 
     # New animation in the middle of an existing one
     animator.animate(animate_test, "foo", 200, duration=1)
     assert animate_test.foo == 50
 
     animator._time = 6
-    await animator()
+    animator()
     assert animate_test.foo == 200
 
 
@@ -251,19 +251,42 @@ async def test_animator_on_complete_callback_not_fired_before_duration_ends():
     animator.animate(animate_test, "foo", 200, duration=10, on_complete=callback)
 
     animator._time = 9
-    await animator()
+    animator()
 
     assert not callback.called
 
 
 async def test_animator_on_complete_callback_fired_at_duration():
     callback = Mock()
     animate_test = AnimateTest()
-    animator = MockAnimator(Mock())
+    mock_app = Mock()
+    animator = MockAnimator(mock_app)
 
     animator.animate(animate_test, "foo", 200, duration=10, on_complete=callback)
 
     animator._time = 10
-    await animator()
+    animator()
+
+    # Ensure that the callback is scheduled to run after the duration is up.
+    mock_app.call_later.assert_called_once_with(callback)
+
+
+def test_force_stop_animation():
+    callback = Mock()
+    animate_test = AnimateTest()
+    mock_app = Mock()
+    animator = MockAnimator(mock_app)
+
+    animator.animate(animate_test, "foo", 200, duration=10, on_complete=callback)
+
+    assert animator.is_being_animated(animate_test, "foo")
+    assert animate_test.foo != 200
+
+    animator.force_stop_animation(animate_test, "foo")
+
+    # The animation of the attribute was force cancelled.
+    assert not animator.is_being_animated(animate_test, "foo")
+    assert animate_test.foo == 200
 
-    callback.assert_called_once_with()
+    # The on_complete callback was scheduled.
+    mock_app.call_later.assert_called_once_with(callback)