From aff9bcdf9365d835e02e36a75b16f38b19e6c538 Mon Sep 17 00:00:00 2001 From: Dave Pearson Date: Mon, 15 May 2023 11:30:26 +0100 Subject: [PATCH 1/5] Fix clearing an OptionList See #2557, credit to Will: https://github.com/Textualize/textual/issues/2557#issuecomment-1546883815 --- src/textual/widgets/_option_list.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/textual/widgets/_option_list.py b/src/textual/widgets/_option_list.py index 3b94131816..7d9af219b7 100644 --- a/src/textual/widgets/_option_list.py +++ b/src/textual/widgets/_option_list.py @@ -631,7 +631,7 @@ def clear_options(self) -> Self: self.highlighted = None self._mouse_hovering_over = None self.virtual_size = Size(self.scrollable_content_region.width, 0) - self.refresh() + self._request_content_tracking_refresh() return self def _set_option_disabled(self, index: int, disabled: bool) -> Self: From f12aeb00d2b9e1835ccaf9b7e194818db25438a0 Mon Sep 17 00:00:00 2001 From: Dave Pearson Date: Tue, 16 May 2023 13:33:57 +0100 Subject: [PATCH 2/5] Remove forced content tracking refresh in clear_options While the fix for #2557 likely isn't *the* fix (see #2582 for some context around that), it is a fix that works for now. As such, with the change, there was a double attempt to refresh the content tracking in the clearing of options in the OptionList, which shouldn't be necessary. This removes that. --- src/textual/widgets/_option_list.py | 1 - 1 file changed, 1 deletion(-) diff --git a/src/textual/widgets/_option_list.py b/src/textual/widgets/_option_list.py index 7d9af219b7..5b4a39b488 100644 --- a/src/textual/widgets/_option_list.py +++ b/src/textual/widgets/_option_list.py @@ -627,7 +627,6 @@ def clear_options(self) -> Self: """ self._contents.clear() self._options.clear() - self._refresh_content_tracking(force=True) self.highlighted = None self._mouse_hovering_over = None self.virtual_size = Size(self.scrollable_content_region.width, 0) From 32fa259c94261dab70337314c2d55f406e919973 Mon Sep 17 00:00:00 2001 From: Dave Pearson Date: Tue, 16 May 2023 13:38:08 +0100 Subject: [PATCH 3/5] Add a TODO comment to the effect that this is a temp fix --- src/textual/widgets/_option_list.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/textual/widgets/_option_list.py b/src/textual/widgets/_option_list.py index 5b4a39b488..dfe530543d 100644 --- a/src/textual/widgets/_option_list.py +++ b/src/textual/widgets/_option_list.py @@ -630,6 +630,13 @@ def clear_options(self) -> Self: self.highlighted = None self._mouse_hovering_over = None self.virtual_size = Size(self.scrollable_content_region.width, 0) + # TODO: See https://github.com/Textualize/textual/issues/2582 -- it + # should not be necessary to do this like this here; ideally here in + # clear_options it would be a forced refresh, and also in a + # `on_show` it would be the same (which, I think, would actually + # solve the problem we're seeing). But, until such a time as we get + # to the bottom of 2582... this seems to delay the refresh enough + # that things fall into place. self._request_content_tracking_refresh() return self From 3d2e3d909277c02753c9fbb597c3395589e9e97c Mon Sep 17 00:00:00 2001 From: Dave Pearson Date: Tue, 16 May 2023 14:34:18 +0100 Subject: [PATCH 4/5] Add a snapshot test for a rebuilt Select This helps test the practical impact of the fix added for #2557. --- .../snapshot_apps/select_rebuild.py | 21 +++++++++++++++++++ tests/snapshot_tests/test_snapshots.py | 9 ++++++++ 2 files changed, 30 insertions(+) create mode 100644 tests/snapshot_tests/snapshot_apps/select_rebuild.py diff --git a/tests/snapshot_tests/snapshot_apps/select_rebuild.py b/tests/snapshot_tests/snapshot_apps/select_rebuild.py new file mode 100644 index 0000000000..190db3b4be --- /dev/null +++ b/tests/snapshot_tests/snapshot_apps/select_rebuild.py @@ -0,0 +1,21 @@ +"""Test https://github.com/Textualize/textual/issues/2557""" + +from textual.app import App, ComposeResult +from textual.widgets import Select, Button + + +class SelectRebuildApp(App[None]): + + def compose(self) -> ComposeResult: + yield Select[int]((("1", 1), ("2", 2))) + yield Button("Rebuild") + + def on_button_pressed(self): + self.query_one(Select).set_options(( + ("This", 0), ("Should", 1), ("Be", 2), + ("What", 3), ("Goes", 4), ("Into",5), + ("The", 6), ("Snapshit", 7) + )) + +if __name__ == "__main__": + SelectRebuildApp().run() diff --git a/tests/snapshot_tests/test_snapshots.py b/tests/snapshot_tests/test_snapshots.py index ea5e321538..9bb589f7a9 100644 --- a/tests/snapshot_tests/test_snapshots.py +++ b/tests/snapshot_tests/test_snapshots.py @@ -493,3 +493,12 @@ def test_quickly_change_tabs(snap_compare): def test_fr_unit_with_min(snap_compare): # https://github.com/Textualize/textual/issues/2378 assert snap_compare(SNAPSHOT_APPS_DIR / "fr_with_min.py") + + +def test_select_rebuild(snap_compare): + # https://github.com/Textualize/textual/issues/2557 + assert snap_compare( + SNAPSHOT_APPS_DIR / "select_rebuild.py", + press=["tab", "space", "escape", "tab", "enter", "tab", "space"] + ) + From 1ebfe2f418ab0cc58255114345eb912ca345f4be Mon Sep 17 00:00:00 2001 From: Dave Pearson Date: Tue, 16 May 2023 14:38:11 +0100 Subject: [PATCH 5/5] Update the snapshits --- .../__snapshots__/test_snapshots.ambr | 163 ++++++++++++++++++ 1 file changed, 163 insertions(+) diff --git a/tests/snapshot_tests/__snapshots__/test_snapshots.ambr b/tests/snapshot_tests/__snapshots__/test_snapshots.ambr index 8b03449d9e..0185582473 100644 --- a/tests/snapshot_tests/__snapshots__/test_snapshots.ambr +++ b/tests/snapshot_tests/__snapshots__/test_snapshots.ambr @@ -22706,6 +22706,169 @@ ''' # --- +# name: test_select_rebuild + ''' + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + SelectRebuildApp + + + + + + + + + + ▊▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▎ + ▊Select▲▎ + ▊▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▎ + ▊▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▎ + ▊Select▎ + ▊This▎ + ▊Should▎ + ▊Be▎ + ▊What▎ + ▊Goes▎ + ▊Into▎ + ▊The▎ + ▊Snapshit▎ + ▊▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▎ + + + + + + + + + + + + + + + ''' +# --- # name: test_switches '''