Merge pull request #4298 from Textualize/tabbed-content-issue-dave-didnt-fix

Enable unsetting active tab pane / tab

Author: willmcgugan

CHANGELOG.md

@@ -54,6 +54,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
   - When the terminal window loses focus, the currently-focused widget will also lose focus.
   - When the terminal window regains focus, the previously-focused widget will regain focus.
 - TextArea binding for <kbd>ctrl</kbd>+<kbd>k</kbd> will now delete the line if the line is empty https://github.com/Textualize/textual/issues/4277
+- The active tab (in `Tabs`) / tab pane (in `TabbedContent`) can now be unset https://github.com/Textualize/textual/issues/4241
 
 ## [0.52.1] - 2024-02-20
 

docs/widgets/tabbed_content.md

@@ -127,6 +127,7 @@ For example, to create a `TabbedContent` that has red and green labels:
 
 ## Messages
 
+- [TabbedContent.Cleared][textual.widgets.TabbedContent.Cleared]
 - [TabbedContent.TabActivated][textual.widgets.TabbedContent.TabActivated]
 
 ## Bindings

src/textual/widgets/_tabbed_content.py

@@ -276,7 +276,11 @@ def __rich_repr__(self) -> Result:
             yield self.pane
 
     class Cleared(Message):
-        """Posted when there are no more tab panes."""
+        """Posted when no tab pane is active.
+
+        This can happen if all tab panes are removed or if the currently active tab
+        pane is unset.
+        """
 
         def __init__(self, tabbed_content: TabbedContent) -> None:
             """Initialize message.
@@ -329,22 +333,6 @@ def active_pane(self) -> TabPane | None:
             return None
         return self.get_pane(self.active)
 
-    def validate_active(self, active: str) -> str:
-        """It doesn't make sense for `active` to be an empty string.
-
-        Args:
-            active: Attribute to be validated.
-
-        Returns:
-            Value of `active`.
-
-        Raises:
-            ValueError: If the active attribute is set to empty string when there are tabs available.
-        """
-        if not active and self.get_child_by_type(ContentSwitcher).current:
-            raise ValueError("'active' tab must not be empty string.")
-        return active
-
     @staticmethod
     def _set_id(content: TabPane, new_id: int) -> TabPane:
         """Set an id on the content, if not already present.
@@ -467,8 +455,6 @@ def remove_pane(self, pane_id: str) -> AwaitComplete:
 
         async def _remove_content() -> None:
             await gather(*removal_awaitables)
-            if self.tab_count == 0:
-                self.post_message(self.Cleared(self).set_sender(self))
 
         return AwaitComplete(_remove_content())
 
@@ -486,7 +472,6 @@ def clear_panes(self) -> AwaitComplete:
 
         async def _clear_content() -> None:
             await await_clear
-            self.post_message(self.Cleared(self).set_sender(self))
 
         return AwaitComplete(_clear_content())
 
@@ -547,7 +532,7 @@ def _is_associated_tabs(self, tabs: Tabs) -> bool:
 
     def _watch_active(self, active: str) -> None:
         """Switch tabs when the active attributes changes."""
-        with self.prevent(Tabs.TabActivated):
+        with self.prevent(Tabs.TabActivated, Tabs.Cleared):
             self.get_child_by_type(ContentTabs).active = ContentTab.add_prefix(active)
         self.get_child_by_type(ContentSwitcher).current = active
         if active:
@@ -557,6 +542,10 @@ def _watch_active(self, active: str) -> None:
                     tab=self.get_child_by_type(ContentTabs).get_content_tab(active),
                 )
             )
+        else:
+            self.post_message(
+                TabbedContent.Cleared(tabbed_content=self).set_sender(self)
+            )
 
     @property
     def tab_count(self) -> int:

src/textual/widgets/_tabs.py

@@ -285,7 +285,8 @@ class TabShown(TabMessage):
     class Cleared(Message):
         """Sent when there are no active tabs.
 
-        This can occur when Tabs are cleared, or if all tabs are hidden.
+        This can occur when Tabs are cleared, if all tabs are hidden, or if the
+        currently active tab is unset.
         """
 
         def __init__(self, tabs: Tabs) -> None:
@@ -527,9 +528,10 @@ def remove_tab(self, tab_or_id: Tab | str | None) -> AwaitComplete:
         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:
+            if next_tab is None or (removing_active_tab and next_tab.id is None):
                 self.active = ""
             elif removing_active_tab:
+                assert next_tab.id is not None
                 self.active = next_tab.id
                 next_tab.add_class("-active")
 
@@ -575,12 +577,12 @@ def compose(self) -> ComposeResult:
 
     def watch_active(self, previously_active: str, active: str) -> None:
         """Handle a change to the active tab."""
+        self.query("#tabs-list > Tab.-active").remove_class("-active")
         if active:
             try:
                 active_tab = self.query_one(f"#tabs-list > #{active}", Tab)
             except NoMatches:
                 return
-            self.query("#tabs-list > Tab.-active").remove_class("-active")
             active_tab.add_class("-active")
             self._highlight_active(animate=previously_active != "")
             self._scroll_active_tab()
@@ -699,17 +701,21 @@ def action_previous_tab(self) -> None:
         self._move_tab(-1)
 
     def _move_tab(self, direction: int) -> None:
-        """Activate the next tab.
+        """Activate the next enabled tab in the given direction.
+
+        Tab selection wraps around. If no tab is currently active, the "next"
+        tab is set to be the first and the "previous" tab is the last one.
 
         Args:
             direction: +1 for the next tab, -1 for the previous.
         """
         active_tab = self.active_tab
-        if active_tab is None:
-            return
         tabs = self._potentially_active_tabs
         if not tabs:
             return
+        if not active_tab:
+            self.active = tabs[0 if direction == 1 else -1].id or ""
+            return
         tab_count = len(tabs)
         new_tab_index = (tabs.index(active_tab) + direction) % tab_count
         self.active = tabs[new_tab_index].id or ""

tests/test_tabbed_content.py

@@ -108,9 +108,33 @@ def compose(self) -> ComposeResult:
         with pytest.raises(ValueError):
             tabbed_content.active = "X"
 
-        # Check fail with empty tab
-        with pytest.raises(ValueError):
-            tabbed_content.active = ""
+
+async def test_unsetting_tabbed_content_active():
+    """Check that setting `TabbedContent.active = ""` unsets active tab."""
+
+    messages = []
+
+    class TabbedApp(App[None]):
+        def compose(self) -> ComposeResult:
+            with TabbedContent(initial="bar"):
+                with TabPane("foo", id="foo"):
+                    yield Label("Foo", id="foo-label")
+                with TabPane("bar", id="bar"):
+                    yield Label("Bar", id="bar-label")
+                with TabPane("baz", id="baz"):
+                    yield Label("Baz", id="baz-label")
+
+        def on_tabbed_content_cleared(self, event: TabbedContent.Cleared) -> None:
+            messages.append(event)
+
+    app = TabbedApp()
+    async with app.run_test() as pilot:
+        tabbed_content = app.query_one(TabbedContent)
+        assert bool(tabbed_content.active)
+        tabbed_content.active = ""
+        await pilot.pause()
+        assert len(messages) == 1
+        assert isinstance(messages[0], TabbedContent.Cleared)
 
 
 async def test_tabbed_content_initial():

tests/test_tabs.py

@@ -316,15 +316,8 @@ def compose(self) -> ComposeResult:
         assert tabs.active_tab.id == "tab-2"
         assert tabs.active == tabs.active_tab.id
 
-        # TODO: This one is questionable. It seems Tabs has been designed so
-        # that you can set the active tab to an empty string, and it remains
-        # so, and just removes the underline; no other changes. So active
-        # will be an empty string while active_tab will be a tab. This feels
-        # like an oversight. Need to investigate and possibly modify this
-        # behaviour unless there's a good reason for this.
         tabs.active = ""
-        assert tabs.active_tab is not None
-        assert tabs.active_tab.id == "tab-2"
+        assert tabs.active_tab is None
 
 
 async def test_navigate_tabs_with_keyboard():