diff --git a/src/textual/screen.py b/src/textual/screen.py index 0b9a35a558..b3de7b99ac 100644 --- a/src/textual/screen.py +++ b/src/textual/screen.py @@ -787,6 +787,11 @@ def _select_all_in_widget(self, widget: Widget) -> None: @property def focus_chain(self) -> list[Widget]: """A list of widgets that may receive focus, in focus order.""" + branches, focusable = self._focus_candidates() + return self._order_focus_candidates(branches, focusable) + + def _focus_candidates(self) -> tuple[dict[Widget, tuple[Widget, ...]], set[Widget]]: + """Acquire focus membership before demanding ordering geometry.""" root_node = self.screen if (focused := self.focused) is not None: @@ -836,6 +841,14 @@ def focus_chain(self) -> list[Widget]: if child in focusable or branches.get(child) ) + return branches, focusable + + def _order_focus_candidates( + self, branches: dict[Widget, tuple[Widget, ...]], focusable: set[Widget] + ) -> list[Widget]: + """Order the acquired eligible branches in their native coordinates.""" + root_node = next(iter(branches)) + self._compositor.acquire_geometry( child for children in branches.values() for child in children ) @@ -1064,8 +1077,18 @@ def _reset_focus( if self.focused is not widget: return - # Grab the list of widgets that we can set focus to. - focusable_widgets = self.focus_chain + # A disabled/hidden widget is absent from native membership. That reset + # uses sibling declaration order, so full focus geometry is irrelevant. + # An overridden public chain remains its own selection authority. + widget_index: int | None = None + if type(self).focus_chain is _NATIVE_FOCUS_CHAIN: + branches, focusable = self._focus_candidates() + focusable_widgets = ( + self._order_focus_candidates(branches, focusable) + if widget in focusable else list(focusable) + ) + else: + focusable_widgets = self.focus_chain if not focusable_widgets: # If there's nothing to focus... give up now. self.set_focus(None) @@ -1076,6 +1099,8 @@ def _reset_focus( # the focus chain. widget_index = focusable_widgets.index(widget) except ValueError: + pass + if widget_index is None: # widget is not in focusable widgets # It may have been made invisible # Move to a sibling if possible @@ -2372,6 +2397,11 @@ def validate_sub_title(self, sub_title: Any) -> str | None: return None if sub_title is None else str(sub_title) +# Retain the declaration, not the current public class attribute: replacing +# Screen.focus_chain itself must retain the replacement's selection authority. +_NATIVE_FOCUS_CHAIN = Screen.focus_chain + + @rich.repr.auto class ModalScreen(Screen[ScreenResultType]): """A screen with bindings that take precedence over the App's key bindings. diff --git a/tests/test_focus.py b/tests/test_focus.py index fc4b1ce4cf..deec54c1d6 100644 --- a/tests/test_focus.py +++ b/tests/test_focus.py @@ -1,5 +1,6 @@ from textual.app import App, ComposeResult from textual.containers import Container, ScrollableContainer, Vertical +from textual.screen import Screen from textual.widget import Widget from textual.widgets import Button, Label from textual.widgets._placeholder import Placeholder @@ -31,6 +32,34 @@ def compose(self) -> ComposeResult: yield ChildrenFocusableOnly(Focusable(id="child", classes="c")) +async def test_reset_focus_preserves_base_property_replacement(monkeypatch): + class ReplacementApp(App): + AUTO_FOCUS = None + + def compose(self): + yield Button("First", id="first") + yield Button("Second", id="second") + yield Button("Third", id="third") + + app = ReplacementApp() + async with app.run_test() as pilot: + await pilot.pause() + first, second, third = (app.query_one(f"#{name}") + for name in ("first", "second", "third")) + app.screen.set_focus(first, scroll_visible=False) + calls = [] + + def replacement(screen): + calls.append(screen) + return [third, first, second] + + with monkeypatch.context() as patch: + patch.setattr(Screen, "focus_chain", property(replacement)) + first.disabled = True + assert calls == [app.screen] + assert app.screen.focused is third + + async def test_focus_chain(): app = App() async with app.run_test(): diff --git a/tests/test_geometry_reader_acquisition.py b/tests/test_geometry_reader_acquisition.py index b0410496cd..bb867adc4e 100644 --- a/tests/test_geometry_reader_acquisition.py +++ b/tests/test_geometry_reader_acquisition.py @@ -78,11 +78,8 @@ async def drive(pilot): # This is the same original synchronous watcher/blur/reset path as a # send disabling its focused compose editor. No input is submitted. editor.disabled = True - assert len(compositor.arrangements) == 1 - _, visible, retained, _ = compositor.arrangements[0] - assert visible and compositor._full_map is full_map - assert all(screen.query_one(f"#button-{index}") in retained for index in range(40)) - assert screen.query_one("#source-39") not in retained + assert not compositor.arrangements + assert compositor._full_map is full_map assert editor.text == "original unsent draft" and screen.focused is not editor assert all(compositor._published_map[node] == geometry for node, geometry in committed.items()) @@ -92,6 +89,10 @@ async def drive(pilot): ] assert "editor" not in actual assert len(compositor.arrangements) == 1 + _, visible, retained, _ = compositor.arrangements[0] + assert visible and compositor._full_map is full_map + assert all(screen.query_one(f"#button-{index}") in retained for index in range(40)) + assert screen.query_one("#source-39") not in retained # Complete geometry gives the same ordering, rather than treating the # viewport as the full focus scope or manufacturing missing positions. compositor.reflow(screen, app.size)