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,7 +1,6 @@
 from __future__ import annotations
 
 import asyncio
-from typing import Any
 
 try:
     import httpx

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(
-        Binding(key="o,f,w", action="widget_fade_out", description="opacity fade out"),
+            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")
 

sandbox/darren/screens_focus.css

@@ -0,0 +1,9 @@
+Focusable {
+    padding: 1 2;
+    background: $panel;
+    margin-bottom: 1;
+}
+
+Focusable:focus {
+    outline: solid dodgerblue;
+}

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()

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]:
+    def focused(self) -> Widget | None:
-        """Get widgets that may receive focus, in focus order.
+        """Get the widget that is focused on the currently active screen."""
-
+        return self.screen.focused
-        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
 
     @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:
+        self.screen.set_focus(widget, scroll_visible)
-            # 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
 
     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)

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

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:

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,

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:
+                    if self.focused is not widget:
-                        self.app.set_focus(widget)
+                        self.set_focus(widget)
                         event.stop()
                         return
                 event.style = self.get_style_at(event.screen_x, event.screen_y)

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.

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),

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:

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 screen.focus_next().id == "foo"
-    assert app.focus_next().id == "Paul"
+    assert screen.focus_next().id == "Paul"
-    assert app.focus_next().id == "baz"
+    assert screen.focus_next().id == "baz"
 
-    assert app.focus_previous().id == "Paul"
+    assert screen.focus_previous().id == "Paul"
-    assert app.focus_previous().id == "foo"
+    assert screen.focus_previous().id == "foo"