diff --git a/src/textual/_animator.py b/src/textual/_animator.py index 8a669b2c8a..5d6af31d8c 100644 --- a/src/textual/_animator.py +++ b/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) diff --git a/src/textual/widgets/_tabbed_content.py b/src/textual/widgets/_tabbed_content.py index b69e1f24c7..8809417917 100644 --- a/src/textual/widgets/_tabbed_content.py +++ b/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. diff --git a/src/textual/widgets/_tabs.py b/src/textual/widgets/_tabs.py index dce53e0e81..1cd0668dc0 100644 --- a/src/textual/widgets/_tabs.py +++ b/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.""" diff --git a/tests/animations/test_tabs_underline_animation.py b/tests/animations/test_tabs_underline_animation.py index 05e83e9e5d..c50409d59a 100644 --- a/tests/animations/test_tabs_underline_animation.py +++ b/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,19 +19,15 @@ 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: @@ -40,19 +35,15 @@ async def test_tabs_underline_animates_on_basic() -> None: 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: @@ -60,16 +51,12 @@ async def test_tabs_underline_does_not_animate_on_none() -> None: 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 diff --git a/tests/snapshot_tests/__snapshots__/test_snapshots/test_remove_tab_no_animation.svg b/tests/snapshot_tests/__snapshots__/test_snapshots/test_remove_tab_no_animation.svg new file mode 100644 index 0000000000..5a932b3641 --- /dev/null +++ b/tests/snapshot_tests/__snapshots__/test_snapshots/test_remove_tab_no_animation.svg @@ -0,0 +1,154 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + ReproApp + + + + + + + + + + +bar22baz333qux4444 +━╸━━━━━╺━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + +bar contents                                                                 + + + + + + + + + + + + + + + + + + + + + + diff --git a/tests/snapshot_tests/snapshot_apps/remove_tab.py b/tests/snapshot_tests/snapshot_apps/remove_tab.py new file mode 100644 index 0000000000..67a595f513 --- /dev/null +++ b/tests/snapshot_tests/snapshot_apps/remove_tab.py @@ -0,0 +1,30 @@ +from textual.app import App +from textual.binding import Binding +from textual.widgets import Label, TabbedContent, TabPane + + +class ReproApp(App[None]): + BINDINGS = [ + Binding("space", "close_pane"), + ] + + def __init__(self): + super().__init__() + self.animation_level = "none" + + def compose(self): + with TabbedContent(): + yield TabPane("foo1", Label("foo contents")) + yield TabPane("bar22", Label("bar contents")) + yield TabPane("baz333", Label("baz contents")) + yield TabPane("qux4444", Label("qux contents")) + + def action_close_pane(self): + tc = self.query_one(TabbedContent) + if tc.active: + tc.remove_pane(tc.active) + + +if __name__ == "__main__": + app = ReproApp() + app.run() diff --git a/tests/snapshot_tests/test_snapshots.py b/tests/snapshot_tests/test_snapshots.py index 96bb9a5e9b..ab6b1b249c 100644 --- a/tests/snapshot_tests/test_snapshots.py +++ b/tests/snapshot_tests/test_snapshots.py @@ -1395,3 +1395,8 @@ async def run_before(pilot: Pilot): await pilot.hover("#foo") assert snap_compare(SNAPSHOT_APPS_DIR / "enter_or_leave.py", run_before=run_before) + + +def test_remove_tab_no_animation(snap_compare): + """Regression test for https://github.com/Textualize/textual/issues/4814""" + assert snap_compare(SNAPSHOT_APPS_DIR / "remove_tab.py", press=["space"])