TabbedContent remove pane fix (actually Tabs fix) (#2808)

* Add a unit test for #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 <[email protected]>

Author: davep

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
 

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)
 

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)

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: