Update TabbedContent with methods to add, remove and clear tabs/panes
#2751
Conversation
If, on the other hand, we set active to empty when there is no content to be tabbed, then we let it slide.
Mimics (and actually simply returns) Tabs.tab_count. The idea being that if people can now add and remove tabs from TabbedContent, they may want to be able to keep track of how many tabs there are.
Much like Tabs.Cleared, this indicates that all available tabs/panes have been removed and the widget is now empty. This is especially important here as the way we remove tabs is such that we can't await their removal and then make the remove methods async (because Tabs doesn't allow for that). So the approach taken here is to send a message from TabbedContent, and delay it as much as possible, ideally once the action that's taking place *has* taken place. The reasoning is: a user may clear down all panes, then want to add some back, possibly with IDs they've used before. The clear down might not have fully happened, but we can't await it all, so the approach for the user would be to wait until the Cleared message turns up *then* repopulate.
The tests touched in this commit are working fine in CI for GNU/Linux and macOS; but fail on Windows as the message we need to come through doesn't seem to be coming through. Testing on Windows (11, in Parallels, on macOS) it seems that setting an actual time for the pauses does the trick. I'm not sure why, I thought a pause with no time ensured that all message queues were emptied before coming out of the pause. Apparently not. So this is an experiment to see if it'll pass in CI too.
davep
changed the title
TabbedContent with methods to add, remove and clear tabs/panesTabbedContent with methods to add, remove and clear tabs/panes
davep
force-pushed
the
tabbed-content-redux
branch
from
c38463d
to
ee59817
Compare
It's possible that we might be being asked to switch from an item of content that has actually been removed; there's no harm in making not finding the old thing a no-op.
It's possible, unlikely but possible, that the content could get removed via some other route, or out of sync, so allow for that. Don't get upset of the content has gone away when we're removing the tab that was in charge of it.
Ideally this would use something like what Textualize#2786 intends to add, but meanwhile this solves the problem of ghost highlights in extreme situations of adding/removing tabs.
These are needed to give messages time to flow.
| """ | ||
| if not active: | ||
| raise ValueError("'active' tab must not be empty string.") | ||
| if not active and self.get_child_by_type(ContentSwitcher).current: |
There was a problem hiding this comment.
Is there any chance the dev might attempt to set active earlier, and there won't be a ContentSwitcer yet?
There was a problem hiding this comment.
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.
| def __await__(self) -> Generator[None, None, None]: | ||
| async def await_tabbed_content() -> None: | ||
| for awaitable in self._awaitables: | ||
| await awaitable | ||
|
|
||
| return await_tabbed_content().__await__() |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
Calling @willmcgugan -- I'm mostly just following the pattern laid down by AwaitMount and AwaitRemove; doubtless Will will have the detail.
There was a problem hiding this comment.
Would you say it's worth adding a test for add_pane that uses after, another with before, and another that checks we get an error when providing both after and before?
There was a problem hiding this comment.
Actually, good point, TabbedContent could do with the same tests the underlying Tabs has; forgot to add those when I added before and after support up here.
Co-authored-by: Rodrigo Girão Serrão <5621605+rodrigogiraoserrao@users.noreply.github.com>
So they don't get confused with actual "add after".
|
This PR can also close #2427 which does something similar, although as a subset of what this PR does. |
This PR extends
TabbedContentwith some new methods, and also adds a new message. The new methods are:add_paneto add a newTabPaneto the widgetremove_paneto remove an existingTabPanefrom the widgetclear_panesto remove all existing panes from the widgettab_count(okay, actually a property) to give easy access to the underlyingTabs.tab_countThe new message is
TabbedContent.Cleared, which, like withTabs, is posted when the number of tabs in the widget falls to zero.In the process of working on this, a small change has also been made to
UnderlineandTabsto counter the case where an animation of the underline is still taking place when the target tab has been removed.Work on this was the cause for #2750 being noted.