Skip to content

Commit

Permalink
fix(tabbed content): prevent duplicate ids error
Browse files Browse the repository at this point in the history
Fixes #5215 where removing
then adding a pane could crash with a `DuplicateIds` exception.

Rather than assigning the ID based on the *current* tab count, this adds
a `_cumulative_tab_count` to ensure added panes have a unique ID.
  • Loading branch information
TomJGooding committed Nov 7, 2024
1 parent a3fee68 commit c9c56bb
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 1 deletion.
6 changes: 5 additions & 1 deletion src/textual/widgets/_tabbed_content.py
Original file line number Diff line number Diff line change
Expand Up @@ -332,6 +332,7 @@ def __init__(
self.titles = [self.render_str(title) for title in titles]
self._tab_content: list[Widget] = []
self._initial = initial
self._cumulative_tab_count = 0
super().__init__(name=name, id=id, classes=classes, disabled=disabled)

@property
Expand Down Expand Up @@ -384,6 +385,8 @@ def compose(self) -> ComposeResult:
for content in pane_content
]

self._cumulative_tab_count = len(tabs)

# Yield the tabs, and ensure they're linked to this TabbedContent.
# It's important to associate the Tabs with the TabbedContent, so that this
# TabbedContent can determine whether a message received from a Tabs instance
Expand Down Expand Up @@ -419,12 +422,13 @@ def add_pane(
Only one of `before` or `after` can be provided. If both are
provided an exception is raised.
"""
self._cumulative_tab_count += 1
if isinstance(before, TabPane):
before = before.id
if isinstance(after, TabPane):
after = after.id
tabs = self.get_child_by_type(ContentTabs)
pane = self._set_id(pane, tabs.tab_count + 1)
pane = self._set_id(pane, self._cumulative_tab_count)
assert pane.id is not None
pane.display = False
return AwaitComplete(
Expand Down
20 changes: 20 additions & 0 deletions tests/test_tabbed_content.py
Original file line number Diff line number Diff line change
Expand Up @@ -914,3 +914,23 @@ def compose(self) -> ComposeResult:
await pilot.pause()
assert app.query_one("Tab#duplicate").disabled is True
assert app.query_one("TabPane#duplicate").disabled is False


async def test_remove_and_add_pane_no_duplicate_id_error():
"""Ensure that removing then adding panes does not raise a `DuplicateIds` exception.
Regression test for https://github.com/Textualize/textual/issues/5215
"""

class TabbedApp(App[None]):
def compose(self) -> ComposeResult:
with TabbedContent():
yield Label("tab-1")
yield Label("tab-2")

app = TabbedApp()
async with app.run_test() as pilot:
# If no exception is raised, the test will pass
tabbed_content = pilot.app.query_one(TabbedContent)
await tabbed_content.remove_pane("tab-1")
await tabbed_content.add_pane(TabPane("Tab 3", Label("tab-3")))

0 comments on commit c9c56bb

Please sign in to comment.