Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

check parent focus #4236

Merged
merged 6 commits into from
Feb 29, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
### Added

- Mapping of ANSI colors to hex codes configurable via `App.ansi_theme_dark` and `App.ansi_theme_light` https://github.com/Textualize/textual/pull/4192
- `Pilot.resize_terminal` to resize the terminal in testing https://github.com/Textualize/textual/issues/4212

### Fixed

Expand All @@ -26,9 +27,9 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
- Rename `CollapsibleTitle.action_toggle` to `action_toggle_collapsible` to fix clash with `DOMNode.action_toggle` https://github.com/Textualize/textual/pull/4221
- Markdown component classes weren't refreshed when watching for CSS https://github.com/Textualize/textual/issues/3464

### Added
### Changed

- `Pilot.resize_terminal` to resize the terminal in testing https://github.com/Textualize/textual/issues/4212
- Clicking a non focusable widget focus ancestors https://github.com/Textualize/textual/pull/4236

## [0.52.1] - 2024-02-20

Expand Down
29 changes: 27 additions & 2 deletions src/textual/screen.py
Original file line number Diff line number Diff line change
Expand Up @@ -307,6 +307,29 @@ def get_widgets_at(self, x: int, y: int) -> Iterable[tuple[Widget, Region]]:
"""
return self._compositor.get_widgets_at(x, y)

def get_focusable_widget_at(self, x: int, y: int) -> Widget | None:
"""Get the focusable widget under a given coordinate.

If the widget directly under the given coordinate is not focusable, then this method will check
if any of the ancestors are focusable. If no ancestors are focusable, then `None` will be returned.

Args:
x: X coordinate.
y: Y coordinate.

Returns:
A `Widget`, or `None` if there is no focusable widget underneath the coordinate.
"""
try:
widget, _region = self.get_widget_at(x, y)
except NoWidget:
return None

for node in widget.ancestors_with_self:
if isinstance(node, Widget) and node.focusable:
return node
Comment on lines +329 to +330
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to guard against a pathological case where the node that was clicked isn't focusable, one of its DOM ancestors is focusable, but the position clicked doesn't fall within said ancestor?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That rarely occurs, since widgets are clipped within the boundary of their parents. The one exception is the overlay rule, and we decided that if you click on the overlay it should focus the DOM parent, even if it isn't directly under the mouse.

return None

def get_style_at(self, x: int, y: int) -> Style:
"""Get the style under a given coordinate.

Expand Down Expand Up @@ -1015,8 +1038,10 @@ def _forward_event(self, event: events.Event) -> None:
except errors.NoWidget:
self.set_focus(None)
else:
if isinstance(event, events.MouseDown) and widget.focusable:
self.set_focus(widget, scroll_visible=False)
if isinstance(event, events.MouseDown):
focusable_widget = self.get_focusable_widget_at(event.x, event.y)
if focusable_widget:
self.set_focus(focusable_widget, scroll_visible=False)
event.style = self.get_style_at(event.screen_x, event.screen_y)
if widget is self:
event._set_forwarded()
Expand Down
43 changes: 41 additions & 2 deletions tests/test_focus.py
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
import pytest

from textual.app import App, ComposeResult
from textual.containers import Container
from textual.containers import Container, ScrollableContainer
from textual.screen import Screen
from textual.widget import Widget
from textual.widgets import Button
from textual.widgets import Button, Label


class Focusable(Widget, can_focus=True):
Expand Down Expand Up @@ -409,3 +409,42 @@ def compose(self) -> ComposeResult:
classes = list(button.get_pseudo_classes())
assert "blur" not in classes
assert "focus" in classes


async def test_get_focusable_widget_at() -> None:
"""Check that clicking a non-focusable widget will focus any (focusable) ancestors."""

class FocusApp(App):
AUTO_FOCUS = None

def compose(self) -> ComposeResult:
with ScrollableContainer(id="focusable"):
with Container():
yield Label("Foo", id="foo")
yield Label("Bar", id="bar")
yield Label("Egg", id="egg")

app = FocusApp()
async with app.run_test() as pilot:
# Nothing focused
assert app.screen.focused is None
# Click foo
await pilot.click("#foo")
# Confirm container is focused
assert app.screen.focused is not None
assert app.screen.focused.id == "focusable"
# Reset focus
app.screen.set_focus(None)
assert app.screen.focused is None
# Click bar
await pilot.click("#bar")
# Confirm container is focused
assert app.screen.focused is not None
assert app.screen.focused.id == "focusable"
# Reset focus
app.screen.set_focus(None)
assert app.screen.focused is None
# Click egg (outside of focusable widget)
await pilot.click("#egg")
# Confirm nothing focused
assert app.screen.focused is None
Loading