From 66a644845b8376bb041b78d82756d7fa7b7b5d49 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rodrigo=20Gir=C3=A3o=20Serr=C3=A3o?= <5621605+rodrigogiraoserrao@users.noreply.github.com> Date: Tue, 18 Apr 2023 11:48:33 +0100 Subject: [PATCH] Prevent reactive-watcher loop in Tabs / TabbedContent. (#2305) * Add regression test for #2229. * Fix potential reactive-watch loop. * Simplify regression test. Labels are cheaper to use and the final visual result of the test won't depend on the directory it runs from. * Simplify solution. Turns out I didn't need a descriptor. :( * Fail on empty tab. --- CHANGELOG.md | 1 + src/textual/widgets/_tabbed_content.py | 27 +-- .../__snapshots__/test_snapshots.ambr | 159 ++++++++++++++++++ .../snapshot_apps/quickly_change_tabs.py | 24 +++ tests/snapshot_tests/test_snapshots.py | 5 + 5 files changed, 196 insertions(+), 20 deletions(-) create mode 100644 tests/snapshot_tests/snapshot_apps/quickly_change_tabs.py diff --git a/CHANGELOG.md b/CHANGELOG.md index adb5bfde9..6bb413d79 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -32,6 +32,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/). - Fix empty ListView preventing bindings from firing https://github.com/Textualize/textual/pull/2281 - Fix `get_component_styles` returning incorrect values on first call when combined with pseudoclasses https://github.com/Textualize/textual/pull/2304 - Fixed `active_message_pump.get` sometimes resulting in a `LookupError` https://github.com/Textualize/textual/issues/2301 +- Fixed issue arising when active tab was changed too quickly in succession https://github.com/Textualize/textual/pull/2305 ## [0.19.1] - 2023-04-10 diff --git a/src/textual/widgets/_tabbed_content.py b/src/textual/widgets/_tabbed_content.py index ffaa1112b..3792ed533 100644 --- a/src/textual/widgets/_tabbed_content.py +++ b/src/textual/widgets/_tabbed_content.py @@ -83,9 +83,6 @@ class TabbedContent(Widget): } """ - active: reactive[str] = reactive("", init=False) - """The ID of the active tab, or empty string if none are active.""" - class TabActivated(Message): """Posted when the active tab changes.""" @@ -116,21 +113,16 @@ class TabbedContent(Widget): self._initial = initial super().__init__() - def validate_active(self, active: str) -> str: - """It doesn't make sense for `active` to be an empty string. + @property + def active(self) -> str: + """The ID of the active tab, or empty string if none are active.""" + return self.get_child_by_type(Tabs).active - Args: - active: Attribute to be validated. - - Returns: - Value of `active`. - - Raises: - ValueError: If the active attribute is set to empty string. - """ + @active.setter + def active(self, active: str) -> None: if not active: raise ValueError("'active' tab must not be empty string.") - return active + self.get_child_by_type(Tabs).active = active def compose(self) -> ComposeResult: """Compose the tabbed content.""" @@ -186,7 +178,6 @@ class TabbedContent(Widget): 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( TabbedContent.TabActivated( tabbed_content=self, @@ -197,7 +188,3 @@ class TabbedContent(Widget): def _on_tabs_cleared(self, event: Tabs.Cleared) -> None: """All tabs were removed.""" event.stop() - - def watch_active(self, active: str) -> None: - """Switch tabs when the active attributes changes.""" - self.get_child_by_type(Tabs).active = active diff --git a/tests/snapshot_tests/__snapshots__/test_snapshots.ambr b/tests/snapshot_tests/__snapshots__/test_snapshots.ambr index 5cd124486..c80298cc6 100644 --- a/tests/snapshot_tests/__snapshots__/test_snapshots.ambr +++ b/tests/snapshot_tests/__snapshots__/test_snapshots.ambr @@ -19676,6 +19676,165 @@ ''' # --- +# name: test_quickly_change_tabs + ''' + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + QuicklyChangeTabsApp + + + + + + + + + + + onetwothree + ━━━━━━━━━━━━━╸━━━━━╺━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + three + + + + + + + + + + + + + + + + + + + + + + + + ''' +# --- # name: test_radio_button_example ''' diff --git a/tests/snapshot_tests/snapshot_apps/quickly_change_tabs.py b/tests/snapshot_tests/snapshot_apps/quickly_change_tabs.py new file mode 100644 index 000000000..d15433805 --- /dev/null +++ b/tests/snapshot_tests/snapshot_apps/quickly_change_tabs.py @@ -0,0 +1,24 @@ +"""Regression test for https://github.com/Textualize/textual/issues/2229.""" +from textual.app import App, ComposeResult +from textual.widgets import TabbedContent, TabPane, Tabs, Label + + +class QuicklyChangeTabsApp(App[None]): + def compose(self) -> ComposeResult: + with TabbedContent(): + with TabPane("one"): + yield Label("one") + with TabPane("two"): + yield Label("two") + with TabPane("three", id="three"): + yield Label("three") + + def key_p(self) -> None: + self.query_one(Tabs).action_next_tab() + self.query_one(Tabs).action_next_tab() + + +app = QuicklyChangeTabsApp() + +if __name__ == "__main__": + app.run() diff --git a/tests/snapshot_tests/test_snapshots.py b/tests/snapshot_tests/test_snapshots.py index 2ab2ba63c..16001300b 100644 --- a/tests/snapshot_tests/test_snapshots.py +++ b/tests/snapshot_tests/test_snapshots.py @@ -431,3 +431,8 @@ def test_scroll_to_center(snap_compare): # scrolled so that the red string >>bullseye<< is centered on the screen. # When this snapshot "breaks" because #2254 is fixed, this snapshot can be updated. assert snap_compare(SNAPSHOT_APPS_DIR / "scroll_to_center.py", press=["s"]) + + +def test_quickly_change_tabs(snap_compare): + # https://github.com/Textualize/textual/issues/2229 + assert snap_compare(SNAPSHOT_APPS_DIR / "quickly_change_tabs.py", press=["p"])