TabbedContent.remove_pane unexpected behaviour

[davep]

Stemming from a question on Discord, there is something not quite working right with remove_pane, which unfortunately didn't get noticed with the current set of unit tests (or the test rig I built for visually/manually testing and experimenting). There are two issues at play, which I think are related.

First, if you run up this code:

from textual.app import App, ComposeResult
from textual.widgets import TabbedContent, TabPane, Label, Footer

class TabbedContentRemoveBug(App[None]):

    BINDINGS = [
        ("delete", "delete"),
    ]

    def compose(self) -> ComposeResult:
        with TabbedContent():
            with TabPane("Keep me", id="keep-me"):
                yield Label("Keep me")
            with TabPane("Keep me too", id="keep-me-too"):
                yield Label("Keep me too")
            with TabPane("Remove me", id="remove-me"):
                yield Label("Remove me")
        yield Footer()

    def action_delete(self) -> None:
        self.query_one(TabbedContent).remove_pane("remove-me")

if __name__ == "__main__":
    TabbedContentRemoveBug().run()

first notice that the first tab is highlighted. Now hit Delete. At this point we'd expect the third tab to be removed and the highlight to remain in place. Instead it slides over to the second tab!

And then on to the problem reported on Discord. If you take a variation on the above, like this:

from textual.app import App, ComposeResult
from textual.widgets import TabbedContent, TabPane, Label, Footer

class TabbedContentRemoveBug(App[None]):

    BINDINGS = [
        ("delete", "delete"),
    ]

    def compose(self) -> ComposeResult:
        with TabbedContent():
            with TabPane("Keep me", id="keep-me"):
                yield Label("Keep me")
            with TabPane("Remove me", id="remove-me"):
                yield Label("Remove me")
        yield Footer()

    def action_delete(self) -> None:
        self.query_one(TabbedContent).remove_pane("remove-me")

if __name__ == "__main__":
    TabbedContentRemoveBug().run()

so just the two tabs, and do the same, that results in an exception.

╭────────────────────────────────────────────────────────── Traceback (most recent call last) ───────────────────────────────────────────────────────────╮
│ /Users/davep/develop/python/textual-sandbox/.venv/lib/python3.10/site-packages/textual/widgets/_tabbed_content.py:352 in _on_tabs_tab_activated        │
│                                                                                                                                                        │
│   349 │   │   assert isinstance(event.tab.id, str)                                                                                                     │
│   350 │   │   event.stop()                                                                                                                             │
│   351 │   │   switcher = self.get_child_by_type(ContentSwitcher)                                                                                       │
│ ❱ 352 │   │   switcher.current = event.tab.id                                                                                                          │
│   353 │   │   self.active = event.tab.id                                                                                                               │
│   354 │   │   self.post_message(                                                                                                                       │
│   355 │   │   │   TabbedContent.TabActivated(                                                                                                          │
│                                                                                                                                                        │
│ ╭────────────────────────── locals ───────────────────────────╮                                                                                        │
│ │    event = TabActivated(Tabs(), ContentTab(id='remove-me')) │                                                                                        │
│ │     self = TabbedContent()                                  │                                                                                        │
│ │ switcher = ContentSwitcher()                                │                                                                                        │
│ ╰─────────────────────────────────────────────────────────────╯                                                                                        │
│                                                                                                                                                        │
│ /Users/davep/develop/python/textual-sandbox/.venv/lib/python3.10/site-packages/textual/widgets/_content_switcher.py:93 in watch_current                │
│                                                                                                                                                        │
│   90 │   │   │   │   except NoMatches:                                                        ╭───────── locals ─────────╮                             │
│   91 │   │   │   │   │   pass                                                                 │  new = 'remove-me'       │                             │
│   92 │   │   │   if new:                                                                      │  old = 'keep-me'         │                             │
│ ❱ 93 │   │   │   │   self.get_child_by_id(new).display = True                                 │ self = ContentSwitcher() │                             │
│   94                                                                                          ╰──────────────────────────╯                             │
│                                                                                                                                                        │
│ /Users/davep/develop/python/textual-sandbox/.venv/lib/python3.10/site-packages/textual/widget.py:507 in get_child_by_id                                │
│                                                                                                                                                        │
│    504 │   │   """                                                                              ╭──────────── locals ─────────────╮                    │
│    505 │   │   child = self._nodes._get_by_id(id)                                               │       child = None              │                    │
│    506 │   │   if child is None:                                                                │ expect_type = None              │                    │
│ ❱  507 │   │   │   raise NoMatches(f"No child found with id={id!r}")                            │          id = 'remove-me'       │                    │
│    508 │   │   if expect_type is None:                                                          │        self = ContentSwitcher() │                    │
│    509 │   │   │   return child                                                                 ╰─────────────────────────────────╯                    │
│    510 │   │   if not isinstance(child, expect_type):                                                                                                  │
╰────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────╯
NoMatches: No child found with id='remove-me'

The priority here is to start out with a version of this test, but remove the panes in reverse. This should fail.

[davep]

By the looks of things the bug is actually in Tabs.remove_tab, where it's sending a Changed event when the tab hasn't actually changed.

[github-actions]

Don't forget to star the repository!

Follow @textualizeio for Textual updates.