From 8aba7b7c71abc3ff401e4dfb896309cd131b14a7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rodrigo=20Gir=C3=A3o=20Serr=C3=A3o?= <5621605+rodrigogiraoserrao@users.noreply.github.com> Date: Thu, 14 Mar 2024 14:56:11 +0000 Subject: [PATCH 1/3] Active tab / tab pane can be unset. Although we can't quite see what would be a use case for a tabbed content where there are tabs and none of them are active (for example, it's weird to think of a browser where we have a bunch of tabs open and none of them are active...), we decided we'd let that possibility exist in code: the reactive `.active` can be set to the empty string both on `Tabs` and on `TabbedContent`. --- CHANGELOG.md | 1 + docs/widgets/tabbed_content.md | 1 + src/textual/widgets/_tabbed_content.py | 28 +++++++++----------------- src/textual/widgets/_tabs.py | 18 +++++++++++------ 4 files changed, 24 insertions(+), 24 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 497ae2bc23..f7172fcff9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -52,6 +52,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 ctrl+k 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 diff --git a/docs/widgets/tabbed_content.md b/docs/widgets/tabbed_content.md index 15164e7907..e6edb899cc 100644 --- a/docs/widgets/tabbed_content.md +++ b/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 diff --git a/src/textual/widgets/_tabbed_content.py b/src/textual/widgets/_tabbed_content.py index 61a2f29a4e..cc93b8c1fc 100644 --- a/src/textual/widgets/_tabbed_content.py +++ b/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. @@ -547,7 +535,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 +545,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: diff --git a/src/textual/widgets/_tabs.py b/src/textual/widgets/_tabs.py index e46c972362..df2076e5d8 100644 --- a/src/textual/widgets/_tabs.py +++ b/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 "" From bd5c9eecddc672e7d2996c1f9c35e5a3648047dd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rodrigo=20Gir=C3=A3o=20Serr=C3=A3o?= <5621605+rodrigogiraoserrao@users.noreply.github.com> Date: Thu, 14 Mar 2024 15:05:57 +0000 Subject: [PATCH 2/3] Remove redundant messages. Now that the watcher for `TabbedContent.active` accepts empty strings and emits the message `TabbedContent.Cleared` if `active` is the empty string, we don't need to explicit emit the messages in these two locations. --- src/textual/widgets/_tabbed_content.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/textual/widgets/_tabbed_content.py b/src/textual/widgets/_tabbed_content.py index cc93b8c1fc..54b0c38799 100644 --- a/src/textual/widgets/_tabbed_content.py +++ b/src/textual/widgets/_tabbed_content.py @@ -455,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()) @@ -474,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()) From edd5b60fddb95ff366c3b57d2a717c055cece251 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rodrigo=20Gir=C3=A3o=20Serr=C3=A3o?= <5621605+rodrigogiraoserrao@users.noreply.github.com> Date: Thu, 14 Mar 2024 15:06:50 +0000 Subject: [PATCH 3/3] Tweak tests to check unsetting works. --- tests/test_tabbed_content.py | 30 +++++++++++++++++++++++++++--- tests/test_tabs.py | 9 +-------- 2 files changed, 28 insertions(+), 11 deletions(-) diff --git a/tests/test_tabbed_content.py b/tests/test_tabbed_content.py index 92d525f698..57a8874ebf 100644 --- a/tests/test_tabbed_content.py +++ b/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(): diff --git a/tests/test_tabs.py b/tests/test_tabs.py index 32e626cefb..8d865552d2 100644 --- a/tests/test_tabs.py +++ b/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():