Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(tabbed content): handle pane containing tabs #3444

Conversation

TomJGooding
Copy link
Contributor

Fixes #3412. There might be a better way of discerning Tabs messages, so creating only as a draft for now.

I would still need to add regression tests, but this seems to work from some quick manual testing with the app below:

from textual.app import App, ComposeResult
from textual.widgets import TabbedContent, TabPane, Tabs


class TabsApp(App):
    BINDINGS = [("space", "clear_inner_tabs")]
    CSS = """
    TabPane > Tabs {
        height: auto;
    }
    """

    def compose(self) -> ComposeResult:
        with TabbedContent():
            with TabPane("outer tab #1"):
                yield Tabs(
                    "inner tab#1",
                    "inner tab#2",
                    "inner tab#3",
                    id="inner-tabs-one",
                )
            with TabPane("outer tab #2"):
                yield Tabs(
                    "inner tab#4",
                    "inner tab#5",
                    "inner tab#6",
                    id="inner-tabs-two",
                )

    def action_clear_inner_tabs(self) -> None:
        self.query_one("#inner-tabs-one", Tabs).clear()
        self.query_one("#inner-tabs-two", Tabs).clear()


if __name__ == "__main__":
    app = TabsApp()
    app.run()

Please review the following checklist.

  • Docstrings on all new or modified functions / classes
  • Updated documentation
  • Updated CHANGELOG.md (where appropriate)

@TomJGooding
Copy link
Contributor Author

There's obviously other events such as tabs_cleared that might be trickier to handle.

Sorry clearly I failed to heed my own advice before creating this PR... I'll have a a closer look at the code to understand the failing tests.

@TomJGooding
Copy link
Contributor Author

I think this docstring is misleading, as Tabs.Cleared is actually sent when when there are no active tabs.

def _on_tabs_cleared(self, event: Tabs.Cleared) -> None:
"""All tabs were removed."""

@TomJGooding
Copy link
Contributor Author

I must be still missing something obvious, as the same tests fail after changing to:

--- a/src/textual/widgets/_tabbed_content.py
+++ b/src/textual/widgets/_tabbed_content.py
@@ -395,10 +395,10 @@ class TabbedContent(Widget):
         )
 
     def _on_tabs_cleared(self, event: Tabs.Cleared) -> None:
-        """All tabs were removed."""
-        if self.get_child_by_type(Tabs).tab_count != 0:
+        """There are no active tabs."""
+        if self.get_child_by_type(Tabs).active_tab is not None:
             # A tab pane could contain its own Tabs widget, so just return
-            # early if the TabbedContent still contains tabs.
+            # early if the TabbedContent still contains active tabs.
             # https://github.com/Textualize/textual/issues/3412
             return
         event.stop()

@TomJGooding
Copy link
Contributor Author

TomJGooding commented Oct 25, 2023

I hoped this might be fixed with the changes to Tabs/TabbedContent done in #3498, but unfortunately I'm still seeing the same test failures.

I'm not sure this is really the best way to discern Tabs messages within TabbedContent anyway. Sorry @darrenburns to tag you, but before I throw in the towel and close this PR, do you have any ideas on this?

@TomJGooding
Copy link
Contributor Author

Closed in favour of #3602.

@TomJGooding TomJGooding deleted the fix-tabbed-content-handle-pane-containing-tabs branch November 8, 2023 23:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Assertion Error on Tabs in TabPane
1 participant