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

Update TabbedContent with methods to add, remove and clear tabs/panes #2751

Merged
merged 34 commits into from
Jun 16, 2023
Merged
Show file tree
Hide file tree
Changes from 30 commits
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
2d4183f
Clean up a type warning about tab ID in _on_tabs_tab_activated
davep Jun 6, 2023
69d86fb
Make the active tab watcher private
davep Jun 6, 2023
290351d
Add an add_pane method to TabbedContent
davep Jun 6, 2023
5cf50f7
Add a remove_pane method to TabbedContent
davep Jun 6, 2023
81edb86
Only error out when active is empty and there are tabs available
davep Jun 6, 2023
300401c
Set active to empty if there are no tabs left
davep Jun 6, 2023
8f4b40e
Add some tests for adding/removing tabs to TabbedContent
davep Jun 6, 2023
7802970
Add a tab_count property to TabbedContent
davep Jun 7, 2023
feb07db
Document the return value of TabbedContent.add_pane
davep Jun 7, 2023
d8aa49b
Deduplicate the create of tab IDs
davep Jun 7, 2023
0ac613d
Add tests that check the tab count when adding and removing tabs
davep Jun 7, 2023
fd243f4
Add a TabbedContent method to clear all panes
davep Jun 7, 2023
2bf8fd7
Add a TabbedContent.Cleared message
davep Jun 7, 2023
21d7049
Fix a copy/pasteo
davep Jun 7, 2023
a6e016d
Experiment with an actual wait time
davep Jun 7, 2023
83633ad
Try a slightly longer pause to wait for messages
davep Jun 7, 2023
ee59817
Merge branch 'main' into tabbed-content-redux
davep Jun 8, 2023
5f0f353
Merge branch 'main' into tabbed-content-redux
davep Jun 14, 2023
2c6d09e
Make attempting to move away from non-existing content a no-op
davep Jun 14, 2023
07c445c
Make the TabbedContent add/remove/clear methods optionally awaitable
davep Jun 14, 2023
617244d
Simplify add_pane
davep Jun 14, 2023
f216686
Be more forgiving when removing tabbed content
davep Jun 15, 2023
ffd6db6
Remove unused import
davep Jun 15, 2023
6f03566
Allow completely turning off the highlight
davep Jun 15, 2023
4af0f83
Turn on/off highlighting of the underline depending on tabs
davep Jun 15, 2023
f5e516d
Add support for adding a pane before/after another one
davep Jun 15, 2023
abc0e80
Make the rest of the TabbedContent tests lean on async/await
davep Jun 15, 2023
4dd934a
Add a couple of missing pauses to TabbedContent unit tests
davep Jun 15, 2023
146b1b8
Update the ChangeLog
davep Jun 15, 2023
06c2b97
Remove unnecessary f-string
davep Jun 15, 2023
282f2c6
Fix a typo
davep Jun 15, 2023
50d93b5
Swap to asyncio.gather
davep Jun 15, 2023
e4b4aad
Rename a couple of TabbedContent tests
davep Jun 15, 2023
832208b
Add unit testing for TabbedContent adding before/after
davep Jun 15, 2023
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
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,11 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
- Class variable `CSS` to screens https://github.com/Textualize/textual/issues/2137
- Class variable `CSS_PATH` to screens https://github.com/Textualize/textual/issues/2137
- Added `cursor_foreground_priority` and `cursor_background_priority` to `DataTable` https://github.com/Textualize/textual/pull/2736
- Added `TabbedContent.tab_count` https://github.com/Textualize/textual/pull/2751
- Added `TabbedContnet.add_pane` https://github.com/Textualize/textual/pull/2751
- Added `TabbedContent.remove_pane` https://github.com/Textualize/textual/pull/2751
- Added `TabbedContent.clear_panes` https://github.com/Textualize/textual/pull/2751
- Added `TabbedContent.Cleared` https://github.com/Textualize/textual/pull/2751

### Fixed

Expand All @@ -22,6 +27,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
- Fixed issue where internal data of `OptionList` could be invalid for short window after `clear_options` https://github.com/Textualize/textual/pull/2754
- Fixed `Tooltip` causing a `query_one` on a lone `Static` to fail https://github.com/Textualize/textual/issues/2723
- Nested widgets wouldn't lose focus when parent is disabled https://github.com/Textualize/textual/issues/2772
- Fixed the `Tabs` `Underline` highlight getting "lost" in some extreme situations https://github.com/Textualize/textual/pull/2751

### Changed

Expand Down
6 changes: 5 additions & 1 deletion src/textual/widgets/_content_switcher.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
from typing import Optional

from ..containers import Container
from ..css.query import NoMatches
from ..events import Mount
from ..reactive import reactive
from ..widget import Widget
Expand Down Expand Up @@ -84,6 +85,9 @@ def watch_current(self, old: str | None, new: str | None) -> None:
"""
with self.app.batch_update():
if old:
self.get_child_by_id(old).display = False
try:
self.get_child_by_id(old).display = False
except NoMatches:
pass
if new:
self.get_child_by_id(new).display = True
192 changes: 169 additions & 23 deletions src/textual/widgets/_tabbed_content.py
Original file line number Diff line number Diff line change
@@ -1,14 +1,17 @@
from __future__ import annotations

from itertools import zip_longest
from typing import Generator

from rich.repr import Result
from rich.text import Text, TextType

from ..app import ComposeResult
from ..await_remove import AwaitRemove
from ..css.query import NoMatches
from ..message import Message
from ..reactive import reactive
from ..widget import Widget
from ..widget import AwaitMount, Widget
from ._content_switcher import ContentSwitcher
from ._tabs import Tab, Tabs

Expand Down Expand Up @@ -70,6 +73,26 @@ def __init__(
)


class AwaitTabbedContent:
"""An awaitable return by [`TabbedContent`][textual.widgets.TabbedContent] methods that modify the tabs."""
davep marked this conversation as resolved.
Show resolved Hide resolved

def __init__(self, *awaitables: AwaitMount | AwaitRemove) -> None:
"""Initialise the awaitable.

Args:
*awaitables: The collection of awaitables to await.
"""
super().__init__()
self._awaitables = awaitables

def __await__(self) -> Generator[None, None, None]:
async def await_tabbed_content() -> None:
for awaitable in self._awaitables:
davep marked this conversation as resolved.
Show resolved Hide resolved
await awaitable

return await_tabbed_content().__await__()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I need to learn more about __await__.
Is it a "standard" approach to implement __await__ with a wrapper around what we care about and then call __await__ on the wrapper?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Calling @willmcgugan -- I'm mostly just following the pattern laid down by AwaitMount and AwaitRemove; doubtless Will will have the detail.



class TabbedContent(Widget):
"""A container with associated tabs to toggle content visibility."""

Expand Down Expand Up @@ -117,6 +140,28 @@ def __rich_repr__(self) -> Result:
yield self.tabbed_content
yield self.tab

class Cleared(Message):
"""Posted when there are no more tab panes."""

def __init__(self, tabbed_content: TabbedContent) -> None:
"""Initialize message.

Args:
tabbed_content: The TabbedContent widget.
"""
self.tabbed_content = tabbed_content
"""The `TabbedContent` widget that contains the tab activated."""
super().__init__()

@property
def control(self) -> TabbedContent:
"""The `TabbedContent` widget that was cleared of all tab panes.

This is an alias for [`Cleared.tabbed_content`][textual.widgets.TabbedContent.Cleared.tabbed_content]
and is used by the [`on`][textual.on] decorator.
"""
return self.tabbed_content

def __init__(
self,
*titles: TextType,
Expand Down Expand Up @@ -151,37 +196,37 @@ def validate_active(self, active: str) -> str:
Value of `active`.

Raises:
ValueError: If the active attribute is set to empty string.
ValueError: If the active attribute is set to empty string when there are tabs available.
"""
if not active:
if not active and self.get_child_by_type(ContentSwitcher).current:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there any chance the dev might attempt to set active earlier, and there won't be a ContentSwitcer yet?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the only way that could arise is if they do that before the widget is composed, but even without this change that would end up with a similar issue due to the triggering of _watch_active, right? Attempting to set the active tab to a pane that doesn't even exist yet is never going to go well.

raise ValueError("'active' tab must not be empty string.")
return active

def compose(self) -> ComposeResult:
"""Compose the tabbed content."""
@staticmethod
def _set_id(content: TabPane, new_id: int) -> TabPane:
"""Set an id on the content, if not already present.

def set_id(content: TabPane, new_id: str) -> TabPane:
"""Set an id on the content, if not already present.
Args:
content: a TabPane.
new_id: Numeric ID to make the pane ID from.

Args:
content: a TabPane.
new_id: New `is` attribute, if it is not already set.
Returns:
The same TabPane.
"""
if content.id is None:
content.id = f"tab-{new_id}"
return content

Returns:
The same TabPane.
"""
if content.id is None:
content.id = new_id
return content
def compose(self) -> ComposeResult:
"""Compose the tabbed content."""

# Wrap content in a `TabPane` if required.
pane_content = [
(
set_id(content, f"tab-{index}")
self._set_id(
content
if isinstance(content, TabPane)
else TabPane(
title or self.render_str(f"Tab {index}"), content, id=f"tab-{index}"
)
else TabPane(title or self.render_str(f"Tab {index}"), content),
index,
)
for index, (title, content) in enumerate(
zip_longest(self.titles, self._tab_content), 1
Expand All @@ -197,6 +242,99 @@ def set_id(content: TabPane, new_id: str) -> TabPane:
with ContentSwitcher(initial=self._initial or None):
yield from pane_content

def add_pane(
self,
pane: TabPane,
*,
before: TabPane | str | None = None,
after: TabPane | str | None = None,
) -> AwaitTabbedContent:
"""Add a new pane to the tabbed content.

Args:
pane: The pane to add.
before: Optional pane or pane ID to add the pane before.
after: Optional pane or pane ID to add the pane after.

Returns:
An awaitable object that waits for the pane to be added.

Raises:
Tabs.TabError: If there is a problem with the addition request.

Note:
Only one of `before` or `after` can be provided. If both are
provided a `Tabs.TabError` will be raised.
"""
if isinstance(before, TabPane):
before = before.id
if isinstance(after, TabPane):
after = after.id
tabs = self.get_child_by_type(Tabs)
pane = self._set_id(pane, tabs.tab_count + 1)
assert pane.id is not None
pane.display = False
return AwaitTabbedContent(
tabs.add_tab(ContentTab(pane._title, pane.id), before=before, after=after),
self.get_child_by_type(ContentSwitcher).mount(pane),
)

def remove_pane(self, pane_id: str) -> AwaitTabbedContent:
"""Remove a given pane from the tabbed content.

Args:
pane_id: The ID of the pane to remove.

Returns:
An awaitable object that waits for the pane to be removed.
"""
removals = [self.get_child_by_type(Tabs).remove_tab(pane_id)]
try:
removals.append(
self.get_child_by_type(ContentSwitcher)
.get_child_by_id(pane_id)
.remove()
)
except NoMatches:
# It's possible that the content itself may have gone away via
# other means; so allow that to be a no-op.
pass
await_remove = AwaitTabbedContent(*removals)

async def _remove_content(cleared_message: TabbedContent.Cleared) -> None:
await await_remove
if self.tab_count == 0:
self.post_message(cleared_message)

# Note that I create the message out here, rather than in
# _remove_content, to ensure that the message's internal
# understanding of who the sender is is correct.
#
# https://github.com/Textualize/textual/issues/2750
self.call_after_refresh(_remove_content, self.Cleared(self))

return await_remove

def clear_panes(self) -> AwaitTabbedContent:
"""Remove all the panes in the tabbed content."""
await_clear = AwaitTabbedContent(
self.get_child_by_type(Tabs).clear(),
self.get_child_by_type(ContentSwitcher).remove_children(),
)

async def _clear_content(cleared_message: TabbedContent.Cleared) -> None:
await await_clear
self.post_message(cleared_message)

# Note that I create the message out here, rather than in
# _clear_content, to ensure that the message's internal
# understanding of who the sender is is correct.
#
# https://github.com/Textualize/textual/issues/2750
self.call_after_refresh(_clear_content, self.Cleared(self))

return await_clear

def compose_add_child(self, widget: Widget) -> None:
"""When using the context manager compose syntax, we want to attach nodes to the switcher.

Expand All @@ -207,9 +345,10 @@ 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)
assert isinstance(event.tab, ContentTab)
switcher.current = event.tab.id
self.active = event.tab.id
self.post_message(
Expand All @@ -222,9 +361,16 @@ def _on_tabs_tab_activated(self, event: Tabs.TabActivated) -> None:
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 = ""

def watch_active(self, active: str) -> None:
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(ContentSwitcher).current = active

@property
def tab_count(self) -> int:
"""Total number of tabs."""
return self.get_child_by_type(Tabs).tab_count
12 changes: 10 additions & 2 deletions src/textual/widgets/_tabs.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
from __future__ import annotations

from typing import TYPE_CHECKING, ClassVar
from typing import ClassVar

import rich.repr
from rich.style import Style
Expand Down Expand Up @@ -46,6 +46,8 @@ class Underline(Widget):
"""First cell in highlight."""
highlight_end = reactive(0)
"""Last cell (inclusive) in highlight."""
show_highlight: reactive[bool] = reactive(True)
"""Flag to indicate if a highlight should be shown at all."""

class Clicked(Message):
"""Inform ancestors the underline was clicked."""
Expand All @@ -60,7 +62,11 @@ def __init__(self, offset: Offset) -> None:
@property
def _highlight_range(self) -> tuple[int, int]:
"""Highlighted range for underline bar."""
return (self.highlight_start, self.highlight_end)
return (
(self.highlight_start, self.highlight_end)
if self.show_highlight
else (0, 0)
)

def render(self) -> RenderResult:
"""Render the bar."""
Expand Down Expand Up @@ -504,9 +510,11 @@ def _highlight_active(self, animate: bool = True) -> None:
try:
active_tab = self.query_one(f"#tabs-list > Tab.-active")
except NoMatches:
underline.show_highlight = False
underline.highlight_start = 0
underline.highlight_end = 0
else:
underline.show_highlight = True
tab_region = active_tab.virtual_region.shrink(active_tab.styles.gutter)
start, end = tab_region.column_span
if animate:
Expand Down
Loading