From 36ac94734ff9ec7a834444820352bf73c98d1528 Mon Sep 17 00:00:00 2001 From: darrenburns Date: Thu, 13 Oct 2022 10:43:16 +0100 Subject: [PATCH] Move focus logic to screen, add more key replacements, collapse bindings in footer (#880) * Move focusing logic to the Screen level * Update tests to support per-screen focus management * Some additional key name replacements * Improve rendering of bindings in footer when multiple items have same action * Clean up footer, allow key_displays csv * Prevent exception when widget is not in screen --- examples/dictionary.py | 1 - sandbox/darren/just_a_box.py | 13 ++- sandbox/darren/screens_focus.css | 9 ++ sandbox/darren/screens_focus.py | 47 ++++++++ src/textual/app.py | 142 +++--------------------- src/textual/binding.py | 16 ++- src/textual/devtools/redirect_output.py | 2 +- src/textual/keys.py | 3 + src/textual/screen.py | 132 +++++++++++++++++++++- src/textual/widget.py | 2 +- src/textual/widgets/_footer.py | 20 +++- src/textual/widgets/tabs.py | 2 +- tests/test_focus.py | 23 ++-- 13 files changed, 257 insertions(+), 155 deletions(-) create mode 100644 sandbox/darren/screens_focus.css create mode 100644 sandbox/darren/screens_focus.py diff --git a/examples/dictionary.py b/examples/dictionary.py index 80936ac01..56c986371 100644 --- a/examples/dictionary.py +++ b/examples/dictionary.py @@ -1,7 +1,6 @@ from __future__ import annotations import asyncio -from typing import Any try: import httpx diff --git a/sandbox/darren/just_a_box.py b/sandbox/darren/just_a_box.py index 51157227c..d4e53d178 100644 --- a/sandbox/darren/just_a_box.py +++ b/sandbox/darren/just_a_box.py @@ -7,8 +7,14 @@ from textual.widgets import Static, Footer, Header class JustABox(App): BINDINGS = [ - Binding(key="t", action="text_fade_out", description="text-opacity fade out"), - Binding(key="o,f,w", action="widget_fade_out", description="opacity fade out"), + Binding( + key="ctrl+t", action="text_fade_out", description="text-opacity fade out" + ), + Binding( + key="o,f,w", + action="widget_fade_out", + description="opacity fade out", + ), ] def compose(self) -> ComposeResult: @@ -28,6 +34,9 @@ class JustABox(App): print(self.screen.styles.get_rules()) print(self.screen.styles.css) + def key_plus(self): + print("plus!") + app = JustABox(watch_css=True, css_path="../darren/just_a_box.css") diff --git a/sandbox/darren/screens_focus.css b/sandbox/darren/screens_focus.css new file mode 100644 index 000000000..8affd01fa --- /dev/null +++ b/sandbox/darren/screens_focus.css @@ -0,0 +1,9 @@ +Focusable { + padding: 1 2; + background: $panel; + margin-bottom: 1; +} + +Focusable:focus { + outline: solid dodgerblue; +} diff --git a/sandbox/darren/screens_focus.py b/sandbox/darren/screens_focus.py new file mode 100644 index 000000000..ba67bcef8 --- /dev/null +++ b/sandbox/darren/screens_focus.py @@ -0,0 +1,47 @@ +from textual.app import App, ComposeResult, ScreenStackError +from textual.binding import Binding +from textual.screen import Screen +from textual.widgets import Static, Footer, Input + + +class Focusable(Static, can_focus=True): + pass + + +class CustomScreen(Screen): + def compose(self) -> ComposeResult: + yield Focusable(f"Screen {id(self)} - two") + yield Focusable(f"Screen {id(self)} - three") + yield Focusable(f"Screen {id(self)} - four") + yield Input(placeholder="Text input") + yield Footer() + + +class ScreensFocusApp(App): + BINDINGS = [ + Binding("plus", "push_new_screen", "Push"), + Binding("minus", "pop_top_screen", "Pop"), + ] + + def compose(self) -> ComposeResult: + yield Focusable("App - one") + yield Input(placeholder="Text input") + yield Input(placeholder="Text input") + yield Focusable("App - two") + yield Focusable("App - three") + yield Focusable("App - four") + yield Footer() + + def action_push_new_screen(self): + self.push_screen(CustomScreen()) + + def action_pop_top_screen(self): + try: + self.pop_screen() + except ScreenStackError: + pass + + +app = ScreensFocusApp(css_path="screens_focus.css") +if __name__ == "__main__": + app.run() diff --git a/src/textual/app.py b/src/textual/app.py index c4a7f058e..643ffe2d3 100644 --- a/src/textual/app.py +++ b/src/textual/app.py @@ -150,8 +150,6 @@ class App(Generic[ReturnType], DOMNode): _BASE_PATH: str | None = None CSS_PATH: CSSPathType = None - focused: Reactive[Widget | None] = Reactive(None) - def __init__( self, driver_class: Type[Driver] | None = None, @@ -326,36 +324,15 @@ class App(Generic[ReturnType], DOMNode): self._close_messages_no_wait() @property - def focus_chain(self) -> list[Widget]: - """Get widgets that may receive focus, in focus order. - - Returns: - list[Widget]: List of Widgets in focus order. - - """ - widgets: list[Widget] = [] - add_widget = widgets.append - root = self.screen - stack: list[Iterator[Widget]] = [iter(root.focusable_children)] - pop = stack.pop - push = stack.append - - while stack: - node = next(stack[-1], None) - if node is None: - pop() - else: - if node.is_container and node.can_focus_children: - push(iter(node.focusable_children)) - else: - if node.can_focus: - add_widget(node) - - return widgets + def focused(self) -> Widget | None: + """Get the widget that is focused on the currently active screen.""" + return self.screen.focused @property def bindings(self) -> Bindings: - """Get current bindings.""" + """Get current bindings. If no widget is focused, then the app-level bindings + are returned. If a widget is focused, then any bindings present between that widget + and the App in the DOM are merged and returned.""" if self.focused is None: return self._bindings else: @@ -367,63 +344,6 @@ class App(Generic[ReturnType], DOMNode): """Set this app to be the currently active app.""" active_app.set(self) - def _move_focus(self, direction: int = 0) -> Widget | None: - """Move the focus in the given direction. - - Args: - direction (int, optional): 1 to move forward, -1 to move backward, or - 0 to highlight the current focus. - - Returns: - Widget | None: Newly focused widget, or None for no focus. - """ - focusable_widgets = self.focus_chain - - if not focusable_widgets: - # Nothing focusable, so nothing to do - return self.focused - if self.focused is None: - # Nothing currently focused, so focus the first one - self.set_focus(focusable_widgets[0]) - else: - try: - # Find the index of the currently focused widget - current_index = focusable_widgets.index(self.focused) - except ValueError: - # Focused widget was removed in the interim, start again - self.set_focus(focusable_widgets[0]) - else: - # Only move the focus if we are currently showing the focus - if direction: - current_index = (current_index + direction) % len(focusable_widgets) - self.set_focus(focusable_widgets[current_index]) - - return self.focused - - def show_focus(self) -> Widget | None: - """Highlight the currently focused widget. - - Returns: - Widget | None: Focused widget, or None for no focus. - """ - return self._move_focus(0) - - def focus_next(self) -> Widget | None: - """Focus the next widget. - - Returns: - Widget | None: Newly focused widget, or None for no focus. - """ - return self._move_focus(1) - - def focus_previous(self) -> Widget | None: - """Focus the previous widget. - - Returns: - Widget | None: Newly focused widget, or None for no focus. - """ - return self._move_focus(-1) - def compose(self) -> ComposeResult: """Yield child widgets for a container.""" return @@ -872,7 +792,7 @@ class App(Generic[ReturnType], DOMNode): self.log.system(f"{self.screen} is current (PUSHED)") def switch_screen(self, screen: Screen | str) -> None: - """Switch to a another screen by replacing the top of the screen stack with a new screen. + """Switch to another screen by replacing the top of the screen stack with a new screen. Args: screen (Screen | str): Either a Screen object or screen name (the `name` argument when installed). @@ -965,43 +885,7 @@ class App(Generic[ReturnType], DOMNode): widget (Widget): Widget to focus. scroll_visible (bool, optional): Scroll widget in to view. """ - if widget is self.focused: - # Widget is already focused - return - - if widget is None: - # No focus, so blur currently focused widget if it exists - if self.focused is not None: - self.focused.post_message_no_wait(events.Blur(self)) - self.focused.emit_no_wait(events.DescendantBlur(self)) - self.focused = None - elif widget.can_focus: - if self.focused != widget: - if self.focused is not None: - # Blur currently focused widget - self.focused.post_message_no_wait(events.Blur(self)) - self.focused.emit_no_wait(events.DescendantBlur(self)) - # Change focus - self.focused = widget - # Send focus event - if scroll_visible: - self.screen.scroll_to_widget(widget) - widget.post_message_no_wait(events.Focus(self)) - widget.emit_no_wait(events.DescendantFocus(self)) - - def _reset_focus(self, widget: Widget) -> None: - """Reset the focus when a widget is removed - - Args: - widget (Widget): A widget that is removed. - """ - if self.focused is widget: - for sibling in widget.siblings: - if sibling.can_focus: - sibling.focus() - break - else: - self.focused = None + self.screen.set_focus(widget, scroll_visible) async def _set_mouse_over(self, widget: Widget | None) -> None: """Called when the mouse is over another widget. @@ -1269,7 +1153,7 @@ class App(Generic[ReturnType], DOMNode): Args: widget (Widget): A Widget to unregister """ - self._reset_focus(widget) + widget.screen._reset_focus(widget) if isinstance(widget._parent, Widget): widget._parent.children._remove(widget) @@ -1391,14 +1275,14 @@ class App(Generic[ReturnType], DOMNode): # Record current mouse position on App self.mouse_position = Offset(event.x, event.y) if isinstance(event, events.Key) and self.focused is not None: - # Key events are sent direct to focused widget + # Key events are sent direct to focused widget of the currently active screen if self.bindings.allow_forward(event.key): await self.focused._forward_event(event) else: # Key has allow_forward=False which disallows forward to focused widget await super().on_event(event) else: - # Forward the event to the view + # Forward the event to the currently active Screen await self.screen._forward_event(event) elif isinstance(event, events.Paste): if self.focused is not None: @@ -1499,9 +1383,9 @@ class App(Generic[ReturnType], DOMNode): async def _on_key(self, event: events.Key) -> None: if event.key == "tab": - self.focus_next() + self.screen.focus_next() elif event.key == "shift+tab": - self.focus_previous() + self.screen.focus_previous() else: if not (await self.press(event.key)): await self.dispatch_key(event) diff --git a/src/textual/binding.py b/src/textual/binding.py index 498201c76..1f52a5a10 100644 --- a/src/textual/binding.py +++ b/src/textual/binding.py @@ -47,14 +47,26 @@ class Bindings: for binding in bindings: if isinstance(binding, Binding): binding_keys = binding.key.split(",") + + # If there's a key display, split it and associate it with the keys + key_displays = ( + binding.key_display.split(",") if binding.key_display else [] + ) + if len(binding_keys) == len(key_displays): + keys_and_displays = zip(binding_keys, key_displays) + else: + keys_and_displays = [ + (key, binding.key_display) for key in binding_keys + ] + if len(binding_keys) > 1: - for key in binding_keys: + for key, display in keys_and_displays: new_binding = Binding( key=key, action=binding.action, description=binding.description, show=binding.show, - key_display=binding.key_display, + key_display=display, allow_forward=binding.allow_forward, ) yield new_binding diff --git a/src/textual/devtools/redirect_output.py b/src/textual/devtools/redirect_output.py index 46a830ab0..bed976274 100644 --- a/src/textual/devtools/redirect_output.py +++ b/src/textual/devtools/redirect_output.py @@ -7,7 +7,7 @@ from .client import DevtoolsLog from .._log import LogGroup, LogVerbosity if TYPE_CHECKING: - from .devtools.client import DevtoolsClient + from .client import DevtoolsClient class StdoutRedirector: diff --git a/src/textual/keys.py b/src/textual/keys.py index fbc4ca292..7f9fed165 100644 --- a/src/textual/keys.py +++ b/src/textual/keys.py @@ -202,6 +202,9 @@ KEY_NAME_REPLACEMENTS = { "solidus": "slash", "reverse_solidus": "backslash", "commercial_at": "at", + "hyphen_minus": "minus", + "plus_sign": "plus", + "low_line": "underscore", } # Some keys have aliases. For example, if you press `ctrl+m` on your keyboard, diff --git a/src/textual/screen.py b/src/textual/screen.py index dd8c78cfd..c3a06d6c0 100644 --- a/src/textual/screen.py +++ b/src/textual/screen.py @@ -1,7 +1,7 @@ from __future__ import annotations import sys -from typing import Iterable +from typing import Iterable, Iterator import rich.repr from rich.console import RenderableType @@ -39,6 +39,7 @@ class Screen(Widget): """ dark: Reactive[bool] = Reactive(False) + focused: Reactive[Widget | None] = Reactive(None) def __init__( self, @@ -142,9 +143,132 @@ class Screen(Widget): Returns: Region: Region relative to screen. + + Raises: + NoWidget: If the widget could not be found in this screen. """ return self._compositor.find_widget(widget) + @property + def focus_chain(self) -> list[Widget]: + """Get widgets that may receive focus, in focus order. + + Returns: + list[Widget]: List of Widgets in focus order. + """ + widgets: list[Widget] = [] + add_widget = widgets.append + stack: list[Iterator[Widget]] = [iter(self.focusable_children)] + pop = stack.pop + push = stack.append + + while stack: + node = next(stack[-1], None) + if node is None: + pop() + else: + if node.is_container and node.can_focus_children: + push(iter(node.focusable_children)) + else: + if node.can_focus: + add_widget(node) + + return widgets + + def _move_focus(self, direction: int = 0) -> Widget | None: + """Move the focus in the given direction. + + Args: + direction (int, optional): 1 to move forward, -1 to move backward, or + 0 to keep the current focus. + + Returns: + Widget | None: Newly focused widget, or None for no focus. + """ + focusable_widgets = self.focus_chain + + if not focusable_widgets: + # Nothing focusable, so nothing to do + return self.focused + if self.focused is None: + # Nothing currently focused, so focus the first one + self.set_focus(focusable_widgets[0]) + else: + try: + # Find the index of the currently focused widget + current_index = focusable_widgets.index(self.focused) + except ValueError: + # Focused widget was removed in the interim, start again + self.set_focus(focusable_widgets[0]) + else: + # Only move the focus if we are currently showing the focus + if direction: + current_index = (current_index + direction) % len(focusable_widgets) + self.set_focus(focusable_widgets[current_index]) + + return self.focused + + def focus_next(self) -> Widget | None: + """Focus the next widget. + + Returns: + Widget | None: Newly focused widget, or None for no focus. + """ + return self._move_focus(1) + + def focus_previous(self) -> Widget | None: + """Focus the previous widget. + + Returns: + Widget | None: Newly focused widget, or None for no focus. + """ + return self._move_focus(-1) + + def _reset_focus(self, widget: Widget) -> None: + """Reset the focus when a widget is removed + + Args: + widget (Widget): A widget that is removed. + """ + if self.focused is widget: + for sibling in widget.siblings: + if sibling.can_focus: + sibling.focus() + break + else: + self.focused = None + + def set_focus(self, widget: Widget | None, scroll_visible: bool = True) -> None: + """Focus (or un-focus) a widget. A focused widget will receive key events first. + + Args: + widget (Widget | None): Widget to focus, or None to un-focus. + scroll_visible (bool, optional): Scroll widget in to view. + """ + if widget is self.focused: + # Widget is already focused + return + + if widget is None: + # No focus, so blur currently focused widget if it exists + if self.focused is not None: + self.focused.post_message_no_wait(events.Blur(self)) + self.focused.emit_no_wait(events.DescendantBlur(self)) + self.focused = None + elif widget.can_focus: + if self.focused != widget: + if self.focused is not None: + # Blur currently focused widget + self.focused.post_message_no_wait(events.Blur(self)) + self.focused.emit_no_wait(events.DescendantBlur(self)) + # Change focus + self.focused = widget + # Send focus event + if scroll_visible: + self.screen.scroll_to_widget(widget) + widget.post_message_no_wait(events.Focus(self)) + widget.emit_no_wait(events.DescendantFocus(self)) + async def _on_idle(self, event: events.Idle) -> None: # Check for any widgets marked as 'dirty' (needs a repaint) event.prevent_default() @@ -318,11 +442,11 @@ class Screen(Widget): else: widget, region = self.get_widget_at(event.x, event.y) except errors.NoWidget: - self.app.set_focus(None) + self.set_focus(None) else: if isinstance(event, events.MouseUp) and widget.can_focus: - if self.app.focused is not widget: - self.app.set_focus(widget) + if self.focused is not widget: + self.set_focus(widget) event.stop() return event.style = self.get_style_at(event.screen_x, event.screen_y) diff --git a/src/textual/widget.py b/src/textual/widget.py index 525c3db00..2052f9f6e 100644 --- a/src/textual/widget.py +++ b/src/textual/widget.py @@ -1812,7 +1812,7 @@ class Widget(DOMNode): scroll_visible (bool, optional): Scroll parent to make this widget visible. Defaults to True. """ - self.app.set_focus(self, scroll_visible=scroll_visible) + self.screen.set_focus(self, scroll_visible=scroll_visible) def capture_mouse(self, capture: bool = True) -> None: """Capture (or release) the mouse. diff --git a/src/textual/widgets/_footer.py b/src/textual/widgets/_footer.py index e012b96fa..cd9808713 100644 --- a/src/textual/widgets/_footer.py +++ b/src/textual/widgets/_footer.py @@ -1,5 +1,7 @@ from __future__ import annotations +from collections import defaultdict + from rich.console import RenderableType from rich.text import Text @@ -55,7 +57,7 @@ class Footer(Widget): self._key_text = None def on_mount(self) -> None: - watch(self.app, "focused", self._focus_changed) + watch(self.screen, "focused", self._focus_changed) def _focus_changed(self, focused: Widget | None) -> None: self._key_text = None @@ -85,12 +87,22 @@ class Footer(Widget): highlight_style = self.get_component_rich_style("footer--highlight") highlight_key_style = self.get_component_rich_style("footer--highlight-key") key_style = self.get_component_rich_style("footer--key") - for binding in self.app.bindings.shown_keys: - key_display = ( + + bindings = self.app.bindings.shown_keys + + action_to_bindings = defaultdict(list) + for binding in bindings: + action_to_bindings[binding.action].append(binding) + + for action, bindings in action_to_bindings.items(): + key_displays = [ binding.key.upper() if binding.key_display is None else binding.key_display - ) + for binding in bindings + ] + key_display = "ยท".join(key_displays) + binding = bindings[0] hovered = self.highlight_key == binding.key key_text = Text.assemble( (f" {key_display} ", highlight_key_style if hovered else key_style), diff --git a/src/textual/widgets/tabs.py b/src/textual/widgets/tabs.py index 48381586a..2b814d6a8 100644 --- a/src/textual/widgets/tabs.py +++ b/src/textual/widgets/tabs.py @@ -243,7 +243,7 @@ class Tabs(Widget): return if event.key == Keys.Escape: - self.app.set_focus(None) + self.screen.set_focus(None) elif event.key == Keys.Right: self.activate_next_tab() elif event.key == Keys.Left: diff --git a/tests/test_focus.py b/tests/test_focus.py index 78542aed9..811817b92 100644 --- a/tests/test_focus.py +++ b/tests/test_focus.py @@ -12,13 +12,14 @@ class NonFocusable(Widget, can_focus=False, can_focus_children=False): async def test_focus_chain(): - app = App() app._set_active() app.push_screen(Screen()) + screen = app.screen + # Check empty focus chain - assert not app.focus_chain + assert not screen.focus_chain app.screen._add_children( Focusable(id="foo"), @@ -28,16 +29,18 @@ async def test_focus_chain(): Focusable(id="baz"), ) - focused = [widget.id for widget in app.focus_chain] + focused = [widget.id for widget in screen.focus_chain] assert focused == ["foo", "Paul", "baz"] async def test_focus_next_and_previous(): - app = App() app._set_active() app.push_screen(Screen()) - app.screen._add_children( + + screen = app.screen + + screen._add_children( Focusable(id="foo"), NonFocusable(id="bar"), Focusable(Focusable(id="Paul"), id="container1"), @@ -45,9 +48,9 @@ async def test_focus_next_and_previous(): Focusable(id="baz"), ) - assert app.focus_next().id == "foo" - assert app.focus_next().id == "Paul" - assert app.focus_next().id == "baz" + assert screen.focus_next().id == "foo" + assert screen.focus_next().id == "Paul" + assert screen.focus_next().id == "baz" - assert app.focus_previous().id == "Paul" - assert app.focus_previous().id == "foo" + assert screen.focus_previous().id == "Paul" + assert screen.focus_previous().id == "foo"