Skip to content

Commit

Permalink
Isolate the command palette from app-based queries
Browse files Browse the repository at this point in the history
This fixes Textualize#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.
  • Loading branch information
davep committed Nov 6, 2023
1 parent 3bb8c46 commit 51dc134
Show file tree
Hide file tree
Showing 11 changed files with 127 additions and 112 deletions.
2 changes: 1 addition & 1 deletion src/textual/app.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 ()

Expand Down
19 changes: 17 additions & 2 deletions src/textual/command.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions tests/command_palette/test_click_away.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
2 changes: 1 addition & 1 deletion tests/command_palette/test_command_source_environment.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
18 changes: 10 additions & 8 deletions tests/command_palette/test_declare_sources.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand All @@ -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]):
Expand All @@ -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):
Expand All @@ -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
)

Expand All @@ -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
)
18 changes: 9 additions & 9 deletions tests/command_palette/test_escaping.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
40 changes: 20 additions & 20 deletions tests/command_palette/test_interaction.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
2 changes: 1 addition & 1 deletion tests/command_palette/test_no_results.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 3 additions & 3 deletions tests/command_palette/test_run_on_select.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
119 changes: 60 additions & 59 deletions tests/snapshot_tests/__snapshots__/test_snapshots.ambr

Large diffs are not rendered by default.

9 changes: 3 additions & 6 deletions tests/snapshot_tests/test_snapshots.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down

0 comments on commit 51dc134

Please sign in to comment.