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): prevent duplicate ids error #5216

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
### Fixed

- Fixed duplicated key displays in the help panel https://github.com/Textualize/textual/issues/5037
- Fixed crash when removing then adding panes to `TabbedContent` https://github.com/Textualize/textual/issues/5215

## [0.85.2] - 2024-11-02

Expand Down
7 changes: 6 additions & 1 deletion src/textual/widgets/_tabbed_content.py
Original file line number Diff line number Diff line change
Expand Up @@ -332,6 +332,8 @@ 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
"""Tracks the total number of tabs added to ensure unique IDs if tabs are removed"""
super().__init__(name=name, id=id, classes=classes, disabled=disabled)

@property
Expand Down Expand Up @@ -384,6 +386,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 +423,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")))
Loading