Skip to content

Commit

Permalink
Handle other Tabs widgets as DOM descendants of a TabbedContent (#3602)
Browse files Browse the repository at this point in the history
* Handle other Tabs widgets as DOM descendants of a TabbedContent

* Update CHANGELOG.md

* Ensure TabbedContent can identify messages from the associated Tabs so that it can ignore messages from other, irrelevant Tabs instances that may exist as descendants deeper in the DOM. Also adds some tests to ensure corresponding issues are fixed.

* Improve docstrings for Tabs.Cleared and corresponding handler inside TabbedContent - it now includes a note that the Cleared message is sent when all tabs are hidden.
  • Loading branch information
darrenburns authored Oct 30, 2023
1 parent 3f33cd1 commit efbb655
Show file tree
Hide file tree
Showing 4 changed files with 127 additions and 30 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
- Fix issue with chunky highlights on buttons https://github.com/Textualize/textual/pull/3571
- Fixed `OptionList` event leakage from `CommandPalette` to `App`.
- Fixed crash in `LoadingIndicator` https://github.com/Textualize/textual/pull/3498
- Fixed crash when `Tabs` appeared as a descendant of `TabbedContent` in the DOM https://github.com/Textualize/textual/pull/3602

### Added

Expand Down
105 changes: 76 additions & 29 deletions src/textual/widgets/_tabbed_content.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,26 @@ def __init__(self, label: Text, content_id: str, disabled: bool = False):
super().__init__(label, id=content_id, disabled=disabled)


class ContentTabs(Tabs):
"""A Tabs which is associated with a TabbedContent."""

def __init__(
self,
*tabs: Tab | TextType,
active: str | None = None,
tabbed_content: TabbedContent,
):
"""Initialize a ContentTabs.
Args:
*tabs: The child tabs.
active: ID of the tab which should be active on start.
tabbed_content: The associated TabbedContent instance.
"""
super().__init__(*tabs, active=active)
self.tabbed_content = tabbed_content


class TabPane(Widget):
"""A container for switchable content, with additional title.
Expand Down Expand Up @@ -245,11 +265,21 @@ def compose(self) -> ComposeResult:
]
# Get a tab for each pane
tabs = [
ContentTab(content._title, content.id or "", disabled=content.disabled)
ContentTab(
content._title,
content.id or "",
disabled=content.disabled,
)
for content in pane_content
]
# Yield the tabs
yield Tabs(*tabs, active=self._initial or None)

# 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
# has been sent from this Tabs, or from a Tabs that may exist as a descendant
# deeper in the DOM.
yield ContentTabs(*tabs, active=self._initial or None, tabbed_content=self)

# Yield the content switcher and panes
with ContentSwitcher(initial=self._initial or None):
yield from pane_content
Expand Down Expand Up @@ -282,7 +312,7 @@ def add_pane(
before = before.id
if isinstance(after, TabPane):
after = after.id
tabs = self.get_child_by_type(Tabs)
tabs = self.get_child_by_type(ContentTabs)
pane = self._set_id(pane, tabs.tab_count + 1)
assert pane.id is not None
pane.display = False
Expand All @@ -301,7 +331,7 @@ def remove_pane(self, pane_id: str) -> AwaitComplete:
An optionally awaitable object that waits for the pane to be removed
and the Cleared message to be posted.
"""
removal_awaitables = [self.get_child_by_type(Tabs).remove_tab(pane_id)]
removal_awaitables = [self.get_child_by_type(ContentTabs).remove_tab(pane_id)]
try:
removal_awaitables.append(
self.get_child_by_type(ContentSwitcher)
Expand Down Expand Up @@ -332,7 +362,7 @@ def clear_panes(self) -> AwaitComplete:
and the Cleared message to be posted.
"""
await_clear = gather(
self.get_child_by_type(Tabs).clear(),
self.get_child_by_type(ContentTabs).clear(),
self.get_child_by_type(ContentSwitcher).remove_children(),
)

Expand All @@ -356,35 +386,52 @@ def compose_add_child(self, widget: Widget) -> None:

def _on_tabs_tab_activated(self, event: Tabs.TabActivated) -> None:
"""User clicked a tab."""
assert isinstance(event.tab, ContentTab)
assert isinstance(event.tab.id, str)
event.stop()
switcher = self.get_child_by_type(ContentSwitcher)
switcher.current = event.tab.id
self.active = event.tab.id
self.post_message(
TabbedContent.TabActivated(
tabbed_content=self,
tab=event.tab,
if self._is_associated_tabs(event.tabs):
# The message is relevant, so consume it and update state accordingly.
event.stop()
switcher = self.get_child_by_type(ContentSwitcher)
switcher.current = event.tab.id
self.active = event.tab.id
self.post_message(
TabbedContent.TabActivated(
tabbed_content=self,
tab=event.tab,
)
)
)

def _on_tabs_cleared(self, event: Tabs.Cleared) -> None:
"""All tabs were removed."""
event.stop()
self.get_child_by_type(ContentSwitcher).current = None
self.active = ""
"""Called when there are no active tabs. The tabs may have been cleared,
or they may all be hidden."""
if self._is_associated_tabs(event.tabs):
event.stop()
self.get_child_by_type(ContentSwitcher).current = None
self.active = ""

def _is_associated_tabs(self, tabs: Tabs) -> bool:
"""Determine whether a tab is associated with this TabbedContent or not.
A tab is "associated" with a `TabbedContent`, if it's one of the tabs that can
be used to control it. These have a special type: `ContentTab`, and are linked
back to this `TabbedContent` instance via a `tabbed_content` attribute.
Args:
tabs: The Tabs instance to check.
Returns:
True if the tab is associated with this `TabbedContent`.
"""
return isinstance(tabs, ContentTabs) and tabs.tabbed_content is self

def _watch_active(self, active: str) -> None:
"""Switch tabs when the active attributes changes."""
with self.prevent(Tabs.TabActivated):
self.get_child_by_type(Tabs).active = active
self.get_child_by_type(ContentTabs).active = active
self.get_child_by_type(ContentSwitcher).current = active

@property
def tab_count(self) -> int:
"""Total number of tabs."""
return self.get_child_by_type(Tabs).tab_count
return self.get_child_by_type(ContentTabs).tab_count

def _on_tabs_tab_disabled(self, event: Tabs.TabDisabled) -> None:
"""Disable the corresponding tab pane."""
Expand All @@ -404,7 +451,7 @@ def _on_tab_pane_disabled(self, event: TabPane.Disabled) -> None:
tab_pane_id = event.tab_pane.id or ""
try:
with self.prevent(Tab.Disabled):
self.get_child_by_type(Tabs).query_one(
self.get_child_by_type(ContentTabs).query_one(
f"Tab#{tab_pane_id}"
).disabled = True
except NoMatches:
Expand All @@ -428,7 +475,7 @@ def _on_tab_pane_enabled(self, event: TabPane.Enabled) -> None:
tab_pane_id = event.tab_pane.id or ""
try:
with self.prevent(Tab.Enabled):
self.get_child_by_type(Tabs).query_one(
self.get_child_by_type(ContentTabs).query_one(
f"Tab#{tab_pane_id}"
).disabled = False
except NoMatches:
Expand All @@ -444,7 +491,7 @@ def disable_tab(self, tab_id: str) -> None:
Tabs.TabError: If there are any issues with the request.
"""

self.get_child_by_type(Tabs).disable(tab_id)
self.get_child_by_type(ContentTabs).disable(tab_id)

def enable_tab(self, tab_id: str) -> None:
"""Enables the tab with the given ID.
Expand All @@ -456,7 +503,7 @@ def enable_tab(self, tab_id: str) -> None:
Tabs.TabError: If there are any issues with the request.
"""

self.get_child_by_type(Tabs).enable(tab_id)
self.get_child_by_type(ContentTabs).enable(tab_id)

def hide_tab(self, tab_id: str) -> None:
"""Hides the tab with the given ID.
Expand All @@ -468,7 +515,7 @@ def hide_tab(self, tab_id: str) -> None:
Tabs.TabError: If there are any issues with the request.
"""

self.get_child_by_type(Tabs).hide(tab_id)
self.get_child_by_type(ContentTabs).hide(tab_id)

def show_tab(self, tab_id: str) -> None:
"""Shows the tab with the given ID.
Expand All @@ -480,4 +527,4 @@ def show_tab(self, tab_id: str) -> None:
Tabs.TabError: If there are any issues with the request.
"""

self.get_child_by_type(Tabs).show(tab_id)
self.get_child_by_type(ContentTabs).show(tab_id)
5 changes: 4 additions & 1 deletion src/textual/widgets/_tabs.py
Original file line number Diff line number Diff line change
Expand Up @@ -266,7 +266,10 @@ class TabShown(TabMessage):
"""Sent when a tab is shown."""

class Cleared(Message):
"""Sent when there are no active tabs."""
"""Sent when there are no active tabs.
This can occur when Tabs are cleared, or if all tabs are hidden.
"""

def __init__(self, tabs: Tabs) -> None:
"""Initialize the event.
Expand Down
46 changes: 46 additions & 0 deletions tests/test_tabbed_content.py
Original file line number Diff line number Diff line change
Expand Up @@ -796,3 +796,49 @@ def compose(self) -> ComposeResult:
await pilot.pause()
tabber.show_tab("tab-1")
await pilot.pause()


async def test_tabs_nested_in_tabbed_content_doesnt_crash():
"""Regression test for https://github.com/Textualize/textual/issues/3412"""

class TabsNestedInTabbedContent(App):
def compose(self) -> ComposeResult:
with TabbedContent():
with TabPane("Outer TabPane"):
yield Tabs("Inner Tab")

app = TabsNestedInTabbedContent()
async with app.run_test() as pilot:
await pilot.pause()


async def test_tabs_nested_doesnt_interfere_with_ancestor_tabbed_content():
"""When a Tabs is nested as a descendant in the DOM of a TabbedContent,
the messages posted from that Tabs should not interfere with the TabbedContent.
A TabbedContent should only handle messages from Tabs which are direct children.
Relates to https://github.com/Textualize/textual/issues/3412
"""

class TabsNestedInTabbedContent(App):
def compose(self) -> ComposeResult:
with TabbedContent():
with TabPane("OuterTab", id="outer1"):
yield Tabs(
Tab("Tab1", id="tab1"),
Tab("Tab2", id="tab2"),
id="inner-tabs",
)

app = TabsNestedInTabbedContent()
async with app.run_test():
inner_tabs = app.query_one("#inner-tabs", Tabs)
tabbed_content = app.query_one(TabbedContent)

assert inner_tabs.active_tab.id == "tab1"
assert tabbed_content.active == "outer1"

await inner_tabs.clear()

assert inner_tabs.active_tab is None
assert tabbed_content.active == "outer1"

0 comments on commit efbb655

Please sign in to comment.