From 51dc134a135771f02f5cb462671c988d8ac665ca Mon Sep 17 00:00:00 2001 From: Dave Pearson Date: Mon, 6 Nov 2023 13:57:44 +0000 Subject: [PATCH] Isolate the command palette from app-based queries This fixes #3633 by ensuring that if a query is made against the app while the command palette is active, the query trickles down to the previously-active screen rather than into the command palette modal screen. This also updates the command palette unit tests to take this change into account. --- src/textual/app.py | 2 +- src/textual/command.py | 19 ++- tests/command_palette/test_click_away.py | 4 +- .../test_command_source_environment.py | 2 +- tests/command_palette/test_declare_sources.py | 18 +-- tests/command_palette/test_escaping.py | 18 +-- tests/command_palette/test_interaction.py | 40 +++--- tests/command_palette/test_no_results.py | 2 +- tests/command_palette/test_run_on_select.py | 6 +- .../__snapshots__/test_snapshots.ambr | 119 +++++++++--------- tests/snapshot_tests/test_snapshots.py | 9 +- 11 files changed, 127 insertions(+), 112 deletions(-) diff --git a/src/textual/app.py b/src/textual/app.py index f7296edb6f..e88b52e5f9 100644 --- a/src/textual/app.py +++ b/src/textual/app.py @@ -600,7 +600,7 @@ def children(self) -> Sequence["Widget"]: A sequence of widgets. """ try: - return (self.screen,) + return (CommandPalette.current_screen(self),) except ScreenError: return () diff --git a/src/textual/command.py b/src/textual/command.py index 1922c296ea..13accb74cb 100644 --- a/src/textual/command.py +++ b/src/textual/command.py @@ -475,6 +475,21 @@ def is_open(app: App) -> bool: """ return app.screen.id == CommandPalette._PALETTE_ID + @staticmethod + def current_screen(app: App) -> Screen: + """Get the current screen. + + This method gets the current screen for the app, ignoring the + command palette. + + Args: + app: The app to get the screen for. + + Returns: + The current screen for the application, excluding the command palette. + """ + return app.screen_stack[-2] if CommandPalette.is_open(app) else app.screen + @property def _provider_classes(self) -> set[type[Provider]]: """The currently available command providers. @@ -519,8 +534,8 @@ def _on_click(self, event: Click) -> None: self.dismiss() def on_mount(self, _: Mount) -> None: - """Capture the calling screen.""" - self._calling_screen = self.app.screen_stack[-2] + """Configure the command palette once the DOM is ready.""" + self._calling_screen = self.current_screen(self.app) match_style = self.get_component_rich_style( "command-palette--highlight", partial=True diff --git a/tests/command_palette/test_click_away.py b/tests/command_palette/test_click_away.py index 383f39cdb2..ad8f989113 100644 --- a/tests/command_palette/test_click_away.py +++ b/tests/command_palette/test_click_away.py @@ -20,6 +20,6 @@ def on_mount(self) -> None: async def test_clicking_outside_command_palette_closes_it() -> None: """Clicking 'outside' the command palette should make it go away.""" async with CommandPaletteApp().run_test() as pilot: - assert len(pilot.app.query(CommandPalette)) == 1 + assert isinstance(pilot.app.screen, CommandPalette) await pilot.click() - assert len(pilot.app.query(CommandPalette)) == 0 + assert not isinstance(pilot.app.screen, CommandPalette) diff --git a/tests/command_palette/test_command_source_environment.py b/tests/command_palette/test_command_source_environment.py index af9b691d70..b34d9fff04 100644 --- a/tests/command_palette/test_command_source_environment.py +++ b/tests/command_palette/test_command_source_environment.py @@ -31,7 +31,7 @@ def on_mount(self) -> None: async def test_command_source_environment() -> None: """The command source should see the app and default screen.""" async with CommandPaletteApp().run_test() as pilot: - base_screen = pilot.app.query_one(CommandPalette)._calling_screen + base_screen = CommandPalette.current_screen(pilot.app) assert base_screen is not None await pilot.press(*"test") assert len(SimpleSource.environment) == 1 diff --git a/tests/command_palette/test_declare_sources.py b/tests/command_palette/test_declare_sources.py index c5bae17904..fd411d4c5e 100644 --- a/tests/command_palette/test_declare_sources.py +++ b/tests/command_palette/test_declare_sources.py @@ -28,7 +28,8 @@ class AppWithNoSources(AppWithActiveCommandPalette): async def test_no_app_command_sources() -> None: """An app with no sources declared should work fine.""" async with AppWithNoSources().run_test() as pilot: - assert pilot.app.query_one(CommandPalette)._provider_classes == App.COMMANDS + assert isinstance(pilot.app.screen, CommandPalette) + assert pilot.app.screen._provider_classes == App.COMMANDS class AppWithSources(AppWithActiveCommandPalette): @@ -38,10 +39,8 @@ class AppWithSources(AppWithActiveCommandPalette): async def test_app_command_sources() -> None: """Command sources declared on an app should be in the command palette.""" async with AppWithSources().run_test() as pilot: - assert ( - pilot.app.query_one(CommandPalette)._provider_classes - == AppWithSources.COMMANDS - ) + assert isinstance(pilot.app.screen, CommandPalette) + assert pilot.app.screen._provider_classes == AppWithSources.COMMANDS class AppWithInitialScreen(App[None]): @@ -61,7 +60,8 @@ def on_mount(self) -> None: async def test_no_screen_command_sources() -> None: """An app with a screen with no sources declared should work fine.""" async with AppWithInitialScreen(ScreenWithNoSources()).run_test() as pilot: - assert pilot.app.query_one(CommandPalette)._provider_classes == App.COMMANDS + assert isinstance(pilot.app.screen, CommandPalette) + assert pilot.app.screen._provider_classes == App.COMMANDS class ScreenWithSources(ScreenWithNoSources): @@ -71,8 +71,9 @@ class ScreenWithSources(ScreenWithNoSources): async def test_screen_command_sources() -> None: """Command sources declared on a screen should be in the command palette.""" async with AppWithInitialScreen(ScreenWithSources()).run_test() as pilot: + assert isinstance(pilot.app.screen, CommandPalette) assert ( - pilot.app.query_one(CommandPalette)._provider_classes + pilot.app.screen._provider_classes == App.COMMANDS | ScreenWithSources.COMMANDS ) @@ -91,7 +92,8 @@ def on_mount(self) -> None: async def test_app_and_screen_command_sources_combine() -> None: """If an app and the screen have command sources they should combine.""" async with CombinedSourceApp().run_test() as pilot: + assert isinstance(pilot.app.screen, CommandPalette) assert ( - pilot.app.query_one(CommandPalette)._provider_classes + pilot.app.screen._provider_classes == CombinedSourceApp.COMMANDS | ScreenWithSources.COMMANDS ) diff --git a/tests/command_palette/test_escaping.py b/tests/command_palette/test_escaping.py index 2ac2013b6c..83bf36ad15 100644 --- a/tests/command_palette/test_escaping.py +++ b/tests/command_palette/test_escaping.py @@ -20,31 +20,31 @@ def on_mount(self) -> None: async def test_escape_closes_when_no_list_visible() -> None: """Pressing escape when no list is visible should close the command palette.""" async with CommandPaletteApp().run_test() as pilot: - assert len(pilot.app.query(CommandPalette)) == 1 + assert CommandPalette.is_open(pilot.app) await pilot.press("escape") - assert len(pilot.app.query(CommandPalette)) == 0 + assert not CommandPalette.is_open(pilot.app) async def test_escape_does_not_close_when_list_visible() -> None: """Pressing escape when a hit list is visible should not close the command palette.""" async with CommandPaletteApp().run_test() as pilot: - assert len(pilot.app.query(CommandPalette)) == 1 + assert CommandPalette.is_open(pilot.app) await pilot.press("a") await pilot.press("escape") - assert len(pilot.app.query(CommandPalette)) == 1 + assert CommandPalette.is_open(pilot.app) await pilot.press("escape") - assert len(pilot.app.query(CommandPalette)) == 0 + assert not CommandPalette.is_open(pilot.app) async def test_down_arrow_should_undo_closing_of_list_via_escape() -> None: """Down arrow should reopen the hit list if escape closed it before.""" async with CommandPaletteApp().run_test() as pilot: - assert len(pilot.app.query(CommandPalette)) == 1 + assert CommandPalette.is_open(pilot.app) await pilot.press("a") await pilot.press("escape") - assert len(pilot.app.query(CommandPalette)) == 1 + assert CommandPalette.is_open(pilot.app) await pilot.press("down") await pilot.press("escape") - assert len(pilot.app.query(CommandPalette)) == 1 + assert CommandPalette.is_open(pilot.app) await pilot.press("escape") - assert len(pilot.app.query(CommandPalette)) == 0 + assert not CommandPalette.is_open(pilot.app) diff --git a/tests/command_palette/test_interaction.py b/tests/command_palette/test_interaction.py index d243a35565..f5d64093b4 100644 --- a/tests/command_palette/test_interaction.py +++ b/tests/command_palette/test_interaction.py @@ -21,46 +21,46 @@ def on_mount(self) -> None: async def test_initial_list_no_highlight() -> None: """When the list initially appears, nothing will be highlighted.""" async with CommandPaletteApp().run_test() as pilot: - assert len(pilot.app.query(CommandPalette)) == 1 - assert pilot.app.query_one(CommandList).visible is False + assert CommandPalette.is_open(pilot.app) + assert pilot.app.screen.query_one(CommandList).visible is False await pilot.press("a") - assert pilot.app.query_one(CommandList).visible is True - assert pilot.app.query_one(CommandList).highlighted is None + assert pilot.app.screen.query_one(CommandList).visible is True + assert pilot.app.screen.query_one(CommandList).highlighted is None async def test_down_arrow_selects_an_item() -> None: """Typing in a search value then pressing down should select a command.""" async with CommandPaletteApp().run_test() as pilot: - assert len(pilot.app.query(CommandPalette)) == 1 - assert pilot.app.query_one(CommandList).visible is False + assert CommandPalette.is_open(pilot.app) + assert pilot.app.screen.query_one(CommandList).visible is False await pilot.press("a") - assert pilot.app.query_one(CommandList).visible is True - assert pilot.app.query_one(CommandList).highlighted is None + assert pilot.app.screen.query_one(CommandList).visible is True + assert pilot.app.screen.query_one(CommandList).highlighted is None await pilot.press("down") - assert pilot.app.query_one(CommandList).highlighted is not None + assert pilot.app.screen.query_one(CommandList).highlighted is not None async def test_enter_selects_an_item() -> None: """Typing in a search value then pressing enter should select a command.""" async with CommandPaletteApp().run_test() as pilot: - assert len(pilot.app.query(CommandPalette)) == 1 - assert pilot.app.query_one(CommandList).visible is False + assert CommandPalette.is_open(pilot.app) + assert pilot.app.screen.query_one(CommandList).visible is False await pilot.press("a") - assert pilot.app.query_one(CommandList).visible is True - assert pilot.app.query_one(CommandList).highlighted is None + assert pilot.app.screen.query_one(CommandList).visible is True + assert pilot.app.screen.query_one(CommandList).highlighted is None await pilot.press("enter") - assert pilot.app.query_one(CommandList).highlighted is not None + assert pilot.app.screen.query_one(CommandList).highlighted is not None async def test_selection_of_command_closes_command_palette() -> None: """Selecting a command from the list should close the list.""" async with CommandPaletteApp().run_test() as pilot: - assert len(pilot.app.query(CommandPalette)) == 1 - assert pilot.app.query_one(CommandList).visible is False + assert CommandPalette.is_open(pilot.app) + assert pilot.app.screen.query_one(CommandList).visible is False await pilot.press("a") - assert pilot.app.query_one(CommandList).visible is True - assert pilot.app.query_one(CommandList).highlighted is None + assert pilot.app.screen.query_one(CommandList).visible is True + assert pilot.app.screen.query_one(CommandList).highlighted is None await pilot.press("enter") - assert pilot.app.query_one(CommandList).highlighted is not None + assert pilot.app.screen.query_one(CommandList).highlighted is not None await pilot.press("enter") - assert len(pilot.app.query(CommandPalette)) == 0 + assert not CommandPalette.is_open(pilot.app) diff --git a/tests/command_palette/test_no_results.py b/tests/command_palette/test_no_results.py index 400e69c28c..de4956262f 100644 --- a/tests/command_palette/test_no_results.py +++ b/tests/command_palette/test_no_results.py @@ -13,7 +13,7 @@ def on_mount(self) -> None: async def test_no_results() -> None: """Receiving no results from a search for a command should not be a problem.""" async with CommandPaletteApp().run_test() as pilot: - assert len(pilot.app.query(CommandPalette)) == 1 + assert CommandPalette.is_open(pilot.app) results = pilot.app.screen.query_one(OptionList) assert results.visible is False assert results.option_count == 0 diff --git a/tests/command_palette/test_run_on_select.py b/tests/command_palette/test_run_on_select.py index a652096b56..31946a127b 100644 --- a/tests/command_palette/test_run_on_select.py +++ b/tests/command_palette/test_run_on_select.py @@ -37,7 +37,7 @@ async def test_with_run_on_select_on() -> None: assert isinstance(pilot.app, CommandPaletteRunOnSelectApp) pilot.app.action_command_palette() await pilot.press("0") - await pilot.app.query_one(CommandPalette).workers.wait_for_complete() + await pilot.app.screen.workers.wait_for_complete() await pilot.press("down") await pilot.press("enter") assert pilot.app.selection is not None @@ -57,11 +57,11 @@ async def test_with_run_on_select_off() -> None: assert isinstance(pilot.app, CommandPaletteDoNotRunOnSelectApp) pilot.app.action_command_palette() await pilot.press("0") - await pilot.app.query_one(CommandPalette).workers.wait_for_complete() + await pilot.app.screen.workers.wait_for_complete() await pilot.press("down") await pilot.press("enter") assert pilot.app.selection is None - assert pilot.app.query_one(Input).value != "" + assert pilot.app.screen.query_one(Input).value != "" await pilot.press("enter") assert pilot.app.selection is not None CommandPalette.run_on_select = save diff --git a/tests/snapshot_tests/__snapshots__/test_snapshots.ambr b/tests/snapshot_tests/__snapshots__/test_snapshots.ambr index 65901f8f26..5d649d2575 100644 --- a/tests/snapshot_tests/__snapshots__/test_snapshots.ambr +++ b/tests/snapshot_tests/__snapshots__/test_snapshots.ambr @@ -2831,136 +2831,137 @@ font-weight: 700; } - .terminal-3973201778-matrix { + .terminal-3683073661-matrix { font-family: Fira Code, monospace; font-size: 20px; line-height: 24.4px; font-variant-east-asian: full-width; } - .terminal-3973201778-title { + .terminal-3683073661-title { font-size: 18px; font-weight: bold; font-family: arial; } - .terminal-3973201778-r1 { fill: #a2a2a2 } - .terminal-3973201778-r2 { fill: #c5c8c6 } - .terminal-3973201778-r3 { fill: #004578 } - .terminal-3973201778-r4 { fill: #00ff00 } - .terminal-3973201778-r5 { fill: #e2e3e3 } - .terminal-3973201778-r6 { fill: #1e1e1e } - .terminal-3973201778-r7 { fill: #fea62b;font-weight: bold } + .terminal-3683073661-r1 { fill: #a2a2a2 } + .terminal-3683073661-r2 { fill: #c5c8c6 } + .terminal-3683073661-r3 { fill: #004578 } + .terminal-3683073661-r4 { fill: #00ff00 } + .terminal-3683073661-r5 { fill: #24292f } + .terminal-3683073661-r6 { fill: #e2e3e3 } + .terminal-3683073661-r7 { fill: #1e1e1e } + .terminal-3683073661-r8 { fill: #fea62b;font-weight: bold } - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - CommandPaletteApp + CommandPaletteApp - + - - - - - ▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔ - - 🔎A - - - This is a test of this code 9 - This is a test of this code 8 - This is a test of this code 7 - This is a test of this code 6 - This is a test of this code 5 - This is a test of this code 4 - This is a test of this code 3 - This is a test of this code 2 - This is a test of this code 1 - This is a test of this code 0 - ▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁ - - - - + + + + + ▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔ + + 🔎A + + + This is a test of this code 9 + This is a test of this code 8 + This is a test of this code 7 + This is a test of this code 6 + This is a test of this code 5 + This is a test of this code 4 + This is a test of this code 3 + This is a test of this code 2 + This is a test of this code 1 + This is a test of this code 0 + ▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁ + + + + diff --git a/tests/snapshot_tests/test_snapshots.py b/tests/snapshot_tests/test_snapshots.py index 6856c89d74..d39e47b8f5 100644 --- a/tests/snapshot_tests/test_snapshots.py +++ b/tests/snapshot_tests/test_snapshots.py @@ -669,15 +669,12 @@ async def run_before(pilot) -> None: def test_command_palette(snap_compare) -> None: - from textual.command import CommandPalette async def run_before(pilot) -> None: - palette = pilot.app.query_one(CommandPalette) - palette_input = palette.query_one(Input) - palette_input.cursor_blink = False - await pilot.press("ctrl+backslash") + #await pilot.press("ctrl+backslash") + pilot.app.screen.query_one(Input).cursor_blink = False await pilot.press("A") - await pilot.app.query_one(CommandPalette).workers.wait_for_complete() + await pilot.app.screen.workers.wait_for_complete() assert snap_compare(SNAPSHOT_APPS_DIR / "command_palette.py", run_before=run_before)