From 4b02d37e8cc298945ea15f01bb7bf134eb164c91 Mon Sep 17 00:00:00 2001 From: Dave Pearson Date: Tue, 20 Jun 2023 09:15:16 +0100 Subject: [PATCH] `TabbedContent` remove pane fix (actually `Tabs` fix) (#2808) * Add a unit test for https://github.com/Textualize/textual/issues/2807 * Add a test for removing tabs in reverse * Add a test for the messages sent when removing tabs in reverse Marked as xfail for the moment, I suspect the root cause of #2807. * Don't sent Changed when tab removal doesn't result in change * Update the CHANGELOG --------- Co-authored-by: Will McGugan --- CHANGELOG.md | 6 ++--- src/textual/widgets/_tabs.py | 13 +++++----- tests/test_tabbed_content.py | 35 +++++++++++++++++++++++++ tests/test_tabs.py | 50 ++++++++++++++++++++++++++++++++++++ 4 files changed, 95 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 286efa3c34..3d982778b7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,9 @@ and this project adheres to [Semantic Versioning](http://semver.org/). - Fixed indented code blocks not showing up in `Markdown` https://github.com/Textualize/textual/issues/2781 - Fixed inline code blocks in lists showing out of order in `Markdown` https://github.com/Textualize/textual/issues/2676 - Fixed list items in a `Markdown` being added to the focus chain https://github.com/Textualize/textual/issues/2380 +- Fixed `Tabs` posting unnecessary messages when removing non-active tabs https://github.com/Textualize/textual/issues/2807 +- call_after_refresh will preserve the sender within the callback https://github.com/Textualize/textual/pull/2806 + ### Added @@ -22,9 +25,6 @@ and this project adheres to [Semantic Versioning](http://semver.org/). - Tooltips are now inherited, so will work with compound widgets -### Fixed - -- call_after_refresh will preserve the sender within the callback https://github.com/Textualize/textual/pull/2806 ## [0.28.0] - 2023-06-19 diff --git a/src/textual/widgets/_tabs.py b/src/textual/widgets/_tabs.py index b4f486e4a2..be0767fd70 100644 --- a/src/textual/widgets/_tabs.py +++ b/src/textual/widgets/_tabs.py @@ -429,11 +429,11 @@ def remove_tab(self, tab_or_id: Tab | str | None) -> AwaitRemove: removing_active_tab = remove_tab.has_class("-active") next_tab = self._next_active - result_message: Tabs.Cleared | Tabs.TabActivated = ( - self.Cleared(self) - if next_tab is None - else self.TabActivated(self, next_tab) - ) + result_message: Tabs.Cleared | Tabs.TabActivated | None = None + if removing_active_tab and next_tab is not None: + result_message = self.TabActivated(self, next_tab) + elif self.tab_count == 1: + result_message = self.Cleared(self) remove_await = remove_tab.remove() @@ -444,7 +444,8 @@ async def do_remove() -> None: if next_tab is not None: next_tab.add_class("-active") self.call_after_refresh(self._highlight_active, animate=True) - self.post_message(result_message) + if result_message is not None: + self.post_message(result_message) self.call_after_refresh(do_remove) diff --git a/tests/test_tabbed_content.py b/tests/test_tabbed_content.py index 6e84f3addd..882dd11523 100644 --- a/tests/test_tabbed_content.py +++ b/tests/test_tabbed_content.py @@ -369,6 +369,41 @@ def on_tabbed_content_cleared(self) -> None: assert tabbed_content.active == "" +async def test_tabbed_content_reversed_removal(): + class TabbedApp(App[None]): + cleared: var[int] = var(0) + + def compose(self) -> ComposeResult: + with TabbedContent(): + yield TabPane("Test 1", id="initial-1") + yield TabPane("Test 2", id="initial-2") + yield TabPane("Test 3", id="initial-3") + + def on_tabbed_content_cleared(self) -> None: + self.cleared += 1 + + async with TabbedApp().run_test() as pilot: + tabbed_content = pilot.app.query_one(TabbedContent) + assert tabbed_content.tab_count == 3 + assert pilot.app.cleared == 0 + assert tabbed_content.active == "initial-1" + await tabbed_content.remove_pane("initial-3") + await pilot.pause() + assert tabbed_content.tab_count == 2 + assert pilot.app.cleared == 0 + assert tabbed_content.active == "initial-1" + await tabbed_content.remove_pane("initial-2") + await pilot.pause() + assert tabbed_content.tab_count == 1 + assert pilot.app.cleared == 0 + assert tabbed_content.active == "initial-1" + await tabbed_content.remove_pane("initial-1") + await pilot.pause() + assert tabbed_content.tab_count == 0 + assert pilot.app.cleared == 1 + assert tabbed_content.active == "" + + async def test_tabbed_content_clear(): class TabbedApp(App[None]): cleared: var[int] = var(0) diff --git a/tests/test_tabs.py b/tests/test_tabs.py index 34ef9b377a..383a262d8d 100644 --- a/tests/test_tabs.py +++ b/tests/test_tabs.py @@ -243,6 +243,43 @@ def compose(self) -> ComposeResult: assert tabs.active_tab is None +async def test_remove_tabs_reversed(): + """It should be possible to remove tabs.""" + + class TabsApp(App[None]): + def compose(self) -> ComposeResult: + yield Tabs("John", "Aeryn", "Moya", "Pilot") + + async with TabsApp().run_test() as pilot: + tabs = pilot.app.query_one(Tabs) + assert tabs.tab_count == 4 + assert tabs.active_tab is not None + assert tabs.active_tab.id == "tab-1" + + await tabs.remove_tab("tab-4") + await pilot.pause() + assert tabs.tab_count == 3 + assert tabs.active_tab is not None + assert tabs.active_tab.id == "tab-1" + + await tabs.remove_tab("tab-3") + await pilot.pause() + assert tabs.tab_count == 2 + assert tabs.active_tab is not None + assert tabs.active_tab.id == "tab-1" + + await tabs.remove_tab("tab-2") + await pilot.pause() + assert tabs.tab_count == 1 + assert tabs.active_tab is not None + assert tabs.active_tab.id == "tab-1" + + await tabs.remove_tab("tab-1") + await pilot.pause() + assert tabs.tab_count == 0 + assert tabs.active_tab is None + + async def test_clear_tabs(): """It should be possible to clear all tabs.""" @@ -420,6 +457,19 @@ async def test_remove_tabs_messages(): ] +async def test_reverse_remove_tabs_messages(): + """Removing tabs should result in various messages.""" + async with TabsMessageCatchApp().run_test() as pilot: + tabs = pilot.app.query_one(Tabs) + for n in reversed(range(4)): + await tabs.remove_tab(f"tab-{n+1}") + await pilot.pause() + assert pilot.app.intended_handlers == [ + "on_tabs_tab_activated", + "on_tabs_cleared", + ] + + async def test_keyboard_navigation_messages(): """Keyboard navigation should result in the expected messages.""" async with TabsMessageCatchApp().run_test() as pilot: