Merge pull request #1595 from Textualize/fix-1196-add-focus-selector

Fix 1196 add focus selector

CHANGELOG.md

@@ -16,6 +16,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 - Added a `Tree.NodeHighlighted` message, giving a `on_tree_node_highlighted` event handler https://github.com/Textualize/textual/issues/1400
 - Added a `inherit_component_classes` subclassing parameter to control whether or not component classes are inherited from base classes https://github.com/Textualize/textual/issues/1399
 - Added `diagnose` as a `textual` command https://github.com/Textualize/textual/issues/1542
+- Added an optional parameter `selector` to the methods `Screen.focus_next` and `Screen.focus_previous` that enable using a CSS selector to narrow down which widgets can get focus https://github.com/Textualize/textual/issues/1196
 
 ### Changed
 

src/textual/screen.py

@@ -9,6 +9,9 @@ from rich.style import Style
 from . import errors, events, messages
 from ._callback import invoke
 from ._compositor import Compositor, MapGeometry
+from .css.match import match
+from .css.parse import parse_selectors
+from .dom import DOMNode
 from .timer import Timer
 from ._types import CallbackType
 from .geometry import Offset, Region, Size
@@ -178,54 +181,105 @@ class Screen(Widget):
 
         return widgets
 
-    def _move_focus(self, direction: int = 0) -> Widget | None:
+    def _move_focus(
+        self, direction: int = 0, selector: str | type[DOMNode.ExpectType] = "*"
+    ) -> Widget | None:
         """Move the focus in the given direction.
 
+        If no widget is currently focused, this will focus the first focusable widget.
+        If no focusable widget matches the given CSS selector, focus is set to `None`.
+
         Args:
             direction (int, optional): 1 to move forward, -1 to move backward, or
                 0 to keep the current focus.
+            selector (str | type[DOMNode.ExpectType], optional): CSS selector to filter
+                what nodes can be focused.
 
         Returns:
-            Widget | None: Newly focused widget, or None for no focus.
+            Widget | None: Newly focused widget, or None for no focus. If the return
+                is not `None`, then it is guaranteed that the widget returned matches
+                the CSS selectors given in the argument.
         """
-        focusable_widgets = self.focus_chain
+        if not isinstance(selector, str):
+            selector = selector.__name__
+        selector_set = parse_selectors(selector)
+        focus_chain = self.focus_chain
+        filtered_focus_chain = (
+            node for node in focus_chain if match(selector_set, node)
+        )
 
-        if not focusable_widgets:
+        if not focus_chain:
             # Nothing focusable, so nothing to do
             return self.focused
         if self.focused is None:
-            # Nothing currently focused, so focus the first one
+            # Nothing currently focused, so focus the first one.
-            self.set_focus(focusable_widgets[0])
+            to_focus = next(filtered_focus_chain, None)
+            self.set_focus(to_focus)
+            return self.focused
+
+        # Ensure focus will be in a node that matches the selectors.
+        if not direction and not match(selector_set, self.focused):
+            direction = 1
+
+        try:
+            # Find the index of the currently focused widget
+            current_index = focus_chain.index(self.focused)
+        except ValueError:
+            # Focused widget was removed in the interim, start again
+            self.set_focus(next(filtered_focus_chain, None))
         else:
-            try:
+            # Only move the focus if we are currently showing the focus
-                # Find the index of the currently focused widget
+            if direction:
-                current_index = focusable_widgets.index(self.focused)
+                to_focus: Widget | None = None
-            except ValueError:
+                chain_length = len(focus_chain)
-                # Focused widget was removed in the interim, start again
+                for step in range(1, len(focus_chain) + 1):
-                self.set_focus(focusable_widgets[0])
+                    node = focus_chain[
-            else:
+                        (current_index + direction * step) % chain_length
-                # Only move the focus if we are currently showing the focus
+                    ]
-                if direction:
+                    if match(selector_set, node):
-                    current_index = (current_index + direction) % len(focusable_widgets)
+                        to_focus = node
-                    self.set_focus(focusable_widgets[current_index])
+                        break
+                self.set_focus(to_focus)
 
         return self.focused
 
-    def focus_next(self) -> Widget | None:
+    def focus_next(
-        """Focus the next widget.
+        self, selector: str | type[DOMNode.ExpectType] = "*"
+    ) -> Widget | None:
+        """Focus the next widget, optionally filtered by a CSS selector.
+
+        If no widget is currently focused, this will focus the first focusable widget.
+        If no focusable widget matches the given CSS selector, focus is set to `None`.
+
+        Args:
+            selector (str | type[DOMNode.ExpectType], optional): CSS selector to filter
+                what nodes can be focused.
 
         Returns:
-            Widget | None: Newly focused widget, or None for no focus.
+            Widget | None: Newly focused widget, or None for no focus. If the return
+                is not `None`, then it is guaranteed that the widget returned matches
+                the CSS selectors given in the argument.
         """
-        return self._move_focus(1)
+        return self._move_focus(1, selector)
 
-    def focus_previous(self) -> Widget | None:
+    def focus_previous(
-        """Focus the previous widget.
+        self, selector: str | type[DOMNode.ExpectType] = "*"
+    ) -> Widget | None:
+        """Focus the previous widget, optionally filtered by a CSS selector.
+
+        If no widget is currently focused, this will focus the first focusable widget.
+        If no focusable widget matches the given CSS selector, focus is set to `None`.
+
+        Args:
+            selector (str | type[DOMNode.ExpectType], optional): CSS selector to filter
+                what nodes can be focused.
 
         Returns:
-            Widget | None: Newly focused widget, or None for no focus.
+            Widget | None: Newly focused widget, or None for no focus. If the return
+                is not `None`, then it is guaranteed that the widget returned matches
+                the CSS selectors given in the argument.
         """
-        return self._move_focus(-1)
+        return self._move_focus(-1, selector)
 
     def _reset_focus(
         self, widget: Widget, avoiding: list[Widget] | None = None

tests/test_focus.py

@@ -1,3 +1,5 @@
+import pytest
+
 from textual.app import App
 from textual.screen import Screen
 from textual.widget import Widget
@@ -15,6 +17,28 @@ class ChildrenFocusableOnly(Widget, can_focus=False, can_focus_children=True):
     pass
 
 
+@pytest.fixture
+def screen() -> Screen:
+    app = App()
+    app._set_active()
+    app.push_screen(Screen())
+
+    screen = app.screen
+
+    # The classes even/odd alternate along the focus chain.
+    # The classes in/out identify nested widgets.
+    screen._add_children(
+        Focusable(id="foo", classes="a"),
+        NonFocusable(id="bar"),
+        Focusable(Focusable(id="Paul", classes="c"), id="container1", classes="b"),
+        NonFocusable(Focusable(id="Jessica", classes="a"), id="container2"),
+        Focusable(id="baz", classes="b"),
+        ChildrenFocusableOnly(Focusable(id="child", classes="c")),
+    )
+
+    return screen
+
+
 def test_focus_chain():
     app = App()
     app._set_active()
@@ -38,22 +62,7 @@ def test_focus_chain():
     assert focus_chain == ["foo", "container1", "Paul", "baz", "child"]
 
 
-def test_focus_next_and_previous():
+def test_focus_next_and_previous(screen: Screen):
-    app = App()
-    app._set_active()
-    app.push_screen(Screen())
-
-    screen = app.screen
-
-    screen._add_children(
-        Focusable(id="foo"),
-        NonFocusable(id="bar"),
-        Focusable(Focusable(id="Paul"), id="container1"),
-        NonFocusable(Focusable(id="Jessica"), id="container2"),
-        Focusable(id="baz"),
-        ChildrenFocusableOnly(Focusable(id="child")),
-    )
-
     assert screen.focus_next().id == "foo"
     assert screen.focus_next().id == "container1"
     assert screen.focus_next().id == "Paul"
@@ -64,3 +73,131 @@ def test_focus_next_and_previous():
     assert screen.focus_previous().id == "Paul"
     assert screen.focus_previous().id == "container1"
     assert screen.focus_previous().id == "foo"
+
+
+def test_focus_next_wrap_around(screen: Screen):
+    """Ensure focusing the next widget wraps around the focus chain."""
+    screen.set_focus(screen.query_one("#child"))
+    assert screen.focused.id == "child"
+
+    assert screen.focus_next().id == "foo"
+
+
+def test_focus_previous_wrap_around(screen: Screen):
+    """Ensure focusing the previous widget wraps around the focus chain."""
+    screen.set_focus(screen.query_one("#foo"))
+    assert screen.focused.id == "foo"
+
+    assert screen.focus_previous().id == "child"
+
+
+def test_wrap_around_selector(screen: Screen):
+    """Ensure moving focus in both directions wraps around the focus chain."""
+    screen.set_focus(screen.query_one("#foo"))
+    assert screen.focused.id == "foo"
+
+    assert screen.focus_previous("#Paul").id == "Paul"
+    assert screen.focus_next("#foo").id == "foo"
+
+
+def test_no_focus_empty_selector(screen: Screen):
+    """Ensure focus is cleared when selector matches nothing."""
+    assert screen.focus_next("#bananas") is None
+    assert screen.focus_previous("#bananas") is None
+
+    screen.set_focus(screen.query_one("#foo"))
+    assert screen.focused is not None
+    assert screen.focus_next("bananas") is None
+    assert screen.focused is None
+
+    screen.set_focus(screen.query_one("#foo"))
+    assert screen.focused is not None
+    assert screen.focus_previous("bananas") is None
+    assert screen.focused is None
+
+
+def test_focus_next_and_previous_with_type_selector(screen: Screen):
+    """Move focus with a selector that matches the currently focused node."""
+    screen.set_focus(screen.query_one("#Paul"))
+    assert screen.focused.id == "Paul"
+
+    assert screen.focus_next(Focusable).id == "baz"
+    assert screen.focus_next(Focusable).id == "child"
+
+    assert screen.focus_previous(Focusable).id == "baz"
+    assert screen.focus_previous(Focusable).id == "Paul"
+    assert screen.focus_previous(Focusable).id == "container1"
+    assert screen.focus_previous(Focusable).id == "foo"
+
+
+def test_focus_next_and_previous_with_str_selector(screen: Screen):
+    """Move focus with a selector that matches the currently focused node."""
+    screen.set_focus(screen.query_one("#foo"))
+    assert screen.focused.id == "foo"
+
+    assert screen.focus_next(".a").id == "foo"
+    assert screen.focus_next(".c").id == "Paul"
+    assert screen.focus_next(".c").id == "child"
+
+    assert screen.focus_previous(".c").id == "Paul"
+    assert screen.focus_previous(".a").id == "foo"
+
+
+def test_focus_next_and_previous_with_type_selector_without_self():
+    """Test moving the focus with a selector that does not match the currently focused node."""
+    app = App()
+    app._set_active()
+    app.push_screen(Screen())
+
+    screen = app.screen
+
+    from textual.containers import Horizontal, Vertical
+    from textual.widgets import Button, Checkbox, Input
+
+    screen._add_children(
+        Vertical(
+            Horizontal(
+                Input(id="w3"),
+                Checkbox(id="w4"),
+                Input(id="w5"),
+                Button(id="w6"),
+                Checkbox(id="w7"),
+                id="w2",
+            ),
+            Horizontal(
+                Button(id="w9"),
+                Checkbox(id="w10"),
+                Button(id="w11"),
+                Input(id="w12"),
+                Input(id="w13"),
+                id="w8",
+            ),
+            id="w1",
+        )
+    )
+
+    screen.set_focus(screen.query_one("#w3"))
+    assert screen.focused.id == "w3"
+
+    assert screen.focus_next(Button).id == "w6"
+    assert screen.focus_next(Checkbox).id == "w7"
+    assert screen.focus_next(Input).id == "w12"
+
+    assert screen.focus_previous(Button).id == "w11"
+    assert screen.focus_previous(Checkbox).id == "w10"
+    assert screen.focus_previous(Button).id == "w9"
+    assert screen.focus_previous(Input).id == "w5"
+
+
+def test_focus_next_and_previous_with_str_selector_without_self(screen: Screen):
+    """Test moving the focus with a selector that does not match the currently focused node."""
+    screen.set_focus(screen.query_one("#foo"))
+    assert screen.focused.id == "foo"
+
+    assert screen.focus_next(".c").id == "Paul"
+    assert screen.focus_next(".b").id == "baz"
+    assert screen.focus_next(".c").id == "child"
+
+    assert screen.focus_previous(".a").id == "foo"
+    assert screen.focus_previous(".a").id == "foo"
+    assert screen.focus_previous(".b").id == "baz"