Merge pull request #4821 from Textualize/tab-remove-fix

remove tab fix

Author: willmcgugan

src/textual/_animator.py

@@ -316,6 +316,7 @@ def animate(
             on_complete: Callback to run after the animation completes.
             level: Minimum level required for the animation to take place (inclusive).
         """
+        self._record_animation(attribute)
         animate_callback = partial(
             self._animate,
             obj,
@@ -336,6 +337,13 @@ def animate(
         else:
             animate_callback()
 
+    def _record_animation(self, attribute: str) -> None:
+        """Called when an attribute is to be animated.
+
+        Args:
+            attribute: Attribute being animated.
+        """
+
     def _animate(
         self,
         obj: object,
@@ -438,6 +446,7 @@ def _animate(
                 ),
                 level=level,
             )
+
         assert animation is not None, "animation expected to be non-None"
 
         current_animation = self._animations.get(animation_key)

src/textual/widgets/_tabbed_content.py

@@ -462,10 +462,7 @@ def remove_pane(self, pane_id: str) -> AwaitComplete:
             # other means; so allow that to be a no-op.
             pass
 
-        async def _remove_content() -> None:
-            await gather(*removal_awaitables)
-
-        return AwaitComplete(_remove_content())
+        return AwaitComplete(*removal_awaitables)
 
     def clear_panes(self) -> AwaitComplete:
         """Remove all the panes in the tabbed content.

src/textual/widgets/_tabs.py

@@ -1,6 +1,5 @@
 from __future__ import annotations
 
-import asyncio
 from dataclasses import dataclass
 from typing import ClassVar
 
@@ -520,27 +519,20 @@ def remove_tab(self, tab_or_id: Tab | str | None) -> AwaitComplete:
             except NoMatches:
                 return AwaitComplete()
 
-        removing_active_tab = remove_tab.has_class("-active")
-        next_tab = self._next_active
-        remove_await = remove_tab.remove()
-
-        highlight_updated = asyncio.Event()
+        if remove_tab.has_class("-active"):
+            next_tab = self._next_active
+        else:
+            next_tab = None
 
         async def do_remove() -> None:
             """Perform the remove after refresh so the underline bar gets new positions."""
-            await remove_await
-            if next_tab is None or (removing_active_tab and next_tab.id is None):
-                self.active = ""
-            elif removing_active_tab:
+            await remove_tab.remove()
+            if next_tab is not None:
                 self.active = next_tab.id or ""
-                next_tab.add_class("-active")
-
-            highlight_updated.set()
-
-        async def wait_for_highlight_update() -> None:
-            await highlight_updated.wait()
+            if not self.query("#tabs-list > Tab"):
+                self.active = ""
 
-        return AwaitComplete(do_remove(), wait_for_highlight_update())
+        return AwaitComplete(do_remove())
 
     def validate_active(self, active: str) -> str:
         """Check id assigned to active attribute is a valid tab."""
@@ -584,7 +576,9 @@ def watch_active(self, previously_active: str, active: str) -> None:
             except NoMatches:
                 return
             active_tab.add_class("-active")
+
             self._highlight_active(animate=previously_active != "")
+
             self._scroll_active_tab()
             self.post_message(self.TabActivated(self, active_tab))
         else:
@@ -604,29 +598,30 @@ def _highlight_active(
         """
         underline = self.query_one(Underline)
         try:
-            active_tab = self.query_one(f"#tabs-list > Tab.-active")
+            _active_tab = self.query_one("#tabs-list > Tab.-active")
         except NoMatches:
             underline.show_highlight = False
             underline.highlight_start = 0
             underline.highlight_end = 0
         else:
             underline.show_highlight = True
-            tab_region = active_tab.virtual_region.shrink(active_tab.styles.gutter)
-            start, end = tab_region.column_span
-            # This is a basic animation, so we only disable it if we want no animations.
-            if animate and self.app.animation_level != "none":
 
-                def animate_underline() -> None:
-                    """Animate the underline."""
-                    try:
-                        active_tab = self.query_one(f"#tabs-list > Tab.-active")
-                    except NoMatches:
-                        pass
-                    else:
-                        tab_region = active_tab.virtual_region.shrink(
-                            active_tab.styles.gutter
-                        )
-                        start, end = tab_region.column_span
+            def move_underline(animate: bool) -> None:
+                """Move the tab underline.
+
+                Args:
+                    animate: animate the underline to its new position.
+                """
+                try:
+                    active_tab = self.query_one("#tabs-list > Tab.-active")
+                except NoMatches:
+                    pass
+                else:
+                    tab_region = active_tab.virtual_region.shrink(
+                        active_tab.styles.gutter
+                    )
+                    start, end = tab_region.column_span
+                    if animate:
                         underline.animate(
                             "highlight_start",
                             start,
@@ -639,11 +634,17 @@ def animate_underline() -> None:
                             duration=0.3,
                             level="basic",
                         )
+                    else:
+                        underline.highlight_start = start
+                        underline.highlight_end = end
 
-                self.set_timer(0.02, lambda: self.call_after_refresh(animate_underline))
+            if animate and self.app.animation_level != "none":
+                self.set_timer(
+                    0.02,
+                    lambda: self.call_after_refresh(move_underline, True),
+                )
             else:
-                underline.highlight_start = start
-                underline.highlight_end = end
+                self.call_after_refresh(move_underline, False)
 
     async def _on_tab_clicked(self, event: Tab.Clicked) -> None:
         """Activate a tab that was clicked."""

tests/animations/test_tabs_underline_animation.py

@@ -5,7 +5,6 @@
 
 from textual.app import App, ComposeResult
 from textual.widgets import Label, TabbedContent, Tabs
-from textual.widgets._tabs import Underline
 
 
 class TabbedContentApp(App[None]):
@@ -20,56 +19,44 @@ async def test_tabs_underline_animates_on_full() -> None:
     app = TabbedContentApp()
     app.animation_level = "full"
 
+    animations: list[str] = []
+
     async with app.run_test() as pilot:
-        underline = app.query_one(Underline)
         animator = app.animator
-        # Freeze time at 0 before triggering the animation.
-        animator._get_time = lambda *_: 0
+        animator._record_animation = animations.append
         app.query_one(Tabs).action_previous_tab()
         await pilot.pause()
-        # Freeze time after the animation start and before animation end.
-        animator._get_time = lambda *_: 0.01
-        # Move to the next frame.
-        animator()
-        assert animator.is_being_animated(underline, "highlight_start")
-        assert animator.is_being_animated(underline, "highlight_end")
+        assert "highlight_start" in animations
+        assert "highlight_end" in animations
 
 
 async def test_tabs_underline_animates_on_basic() -> None:
     """The underline takes some time to move when animated."""
     app = TabbedContentApp()
     app.animation_level = "basic"
 
+    animations: list[str] = []
+
     async with app.run_test() as pilot:
-        underline = app.query_one(Underline)
         animator = app.animator
-        # Freeze time at 0 before triggering the animation.
-        animator._get_time = lambda *_: 0
+        animator._record_animation = animations.append
         app.query_one(Tabs).action_previous_tab()
         await pilot.pause()
-        # Freeze time after the animation start and before animation end.
-        animator._get_time = lambda *_: 0.01
-        # Move to the next frame.
-        animator()
-        assert animator.is_being_animated(underline, "highlight_start")
-        assert animator.is_being_animated(underline, "highlight_end")
+        assert "highlight_start" in animations
+        assert "highlight_end" in animations
 
 
 async def test_tabs_underline_does_not_animate_on_none() -> None:
     """The underline jumps to its final position when not animated."""
     app = TabbedContentApp()
     app.animation_level = "none"
 
+    animations: list[str] = []
+
     async with app.run_test() as pilot:
-        underline = app.query_one(Underline)
         animator = app.animator
-        # Freeze time at 0 before triggering the animation.
-        animator._get_time = lambda *_: 0
+        animator._record_animation = animations.append
         app.query_one(Tabs).action_previous_tab()
         await pilot.pause()
-        # Freeze time after the animation start and before animation end.
-        animator._get_time = lambda *_: 0.01
-        # Move to the next frame.
-        animator()
-        assert not animator.is_being_animated(underline, "highlight_start")
-        assert not animator.is_being_animated(underline, "highlight_end")
+        assert "highlight_start" not in animations
+        assert "highlight_end" not in animations