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

Fix for interaction between pseudoclasses and widget-level render caches #2155

Merged
merged 6 commits into from
Mar 28, 2023
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
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
- Issue with parsing action strings whose arguments contained quoted closing parenthesis https://github.com/Textualize/textual/pull/2112
- Issues with parsing action strings with tuple arguments https://github.com/Textualize/textual/pull/2112
- Fix for tabs not invalidating https://github.com/Textualize/textual/issues/2125
- Fix for interaction between pseudo-classes and widget-level render caches https://github.com/Textualize/textual/pull/2155

### Changed

Expand All @@ -35,6 +36,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).

- Added auto_scroll attribute to TextLog https://github.com/Textualize/textual/pull/2127
- Added scroll_end switch to TextLog.write https://github.com/Textualize/textual/pull/2127
- Added `Widget.get_pseudo_class_state` https://github.com/Textualize/textual/pull/2155
- Added Screen.ModalScreen which prevents App from handling bindings. https://github.com/Textualize/textual/pull/2139
- Added TEXTUAL_LOG env var which should be a path that Textual will write verbose logs to (textual devtools is generally preferred) https://github.com/Textualize/textual/pull/2148
- Added textual.logging.TextualHandler logging handler
Expand Down
30 changes: 30 additions & 0 deletions src/textual/widget.py
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,15 @@ class MountError(WidgetError):
"""Error raised when there was a problem with the mount request."""


class PseudoClasses(NamedTuple):
"""Used for render/render_line based widgets that use caching. This structure can be used as a
cache-key."""

enabled: bool
focus: bool
hover: bool


@rich.repr.auto
class Widget(DOMNode):
"""
Expand Down Expand Up @@ -2357,6 +2366,27 @@ def get_pseudo_classes(self) -> Iterable[str]:
break
node = node._parent

def get_pseudo_class_state(self) -> PseudoClasses:
"""Get an object describing whether each pseudo class is present on this object or not.

Returns:
A PseudoClasses object describing the pseudo classes that are present.
"""
node: MessagePump | None = self
disabled = False
while isinstance(node, Widget):
if node.disabled:
disabled = True
break
node = node._parent

pseudo_classes = PseudoClasses(
enabled=not disabled,
hover=self.mouse_over,
focus=self.has_focus,
)
return pseudo_classes

def post_render(self, renderable: RenderableType) -> ConsoleRenderable:
"""Applies style attributes to the default renderable.

Expand Down
35 changes: 28 additions & 7 deletions src/textual/widgets/_data_table.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,14 +29,13 @@
from ..render import measure
from ..scroll_view import ScrollView
from ..strip import Strip
from ..widget import PseudoClasses

CellCacheKey: TypeAlias = "tuple[RowKey, ColumnKey, Style, bool, bool, int]"
LineCacheKey: TypeAlias = (
"tuple[int, int, int, int, Coordinate, Coordinate, Style, CursorType, bool, int]"
)
RowCacheKey: TypeAlias = (
"tuple[RowKey, int, Style, Coordinate, Coordinate, CursorType, bool, bool, int]"
CellCacheKey: TypeAlias = (
"tuple[RowKey, ColumnKey, Style, bool, bool, int, PseudoClasses]"
)
LineCacheKey: TypeAlias = "tuple[int, int, int, int, Coordinate, Coordinate, Style, CursorType, bool, int, PseudoClasses]"
RowCacheKey: TypeAlias = "tuple[RowKey, int, Style, Coordinate, Coordinate, CursorType, bool, bool, int, PseudoClasses]"
CursorType = Literal["cell", "row", "column", "none"]
CellType = TypeVar("CellType")

Expand Down Expand Up @@ -609,6 +608,11 @@ def __init__(
self._ordered_row_cache: LRUCache[tuple[int, int], list[Row]] = LRUCache(1)
"""Caches row ordering - key is (num_rows, update_count)."""

self._pseudo_class_state = PseudoClasses(False, False, False)
"""The pseudo-class state is used as part of cache keys to ensure that, for example,
when we lose focus on the DataTable, rules which apply to :focus are invalidated
and we prevent lingering styles."""

self._require_update_dimensions: bool = False
"""Set to re-calculate dimensions on idle."""
self._new_rows: set[RowKey] = set()
Expand Down Expand Up @@ -1558,7 +1562,15 @@ def _render_cell(
row_key = self._row_locations.get_key(row_index)

column_key = self._column_locations.get_key(column_index)
cell_cache_key = (row_key, column_key, style, cursor, hover, self._update_count)
cell_cache_key = (
row_key,
column_key,
style,
cursor,
hover,
self._update_count,
self._pseudo_class_state,
)

if cell_cache_key not in self._cell_render_cache:
style += Style.from_meta({"row": row_index, "column": column_index})
Expand Down Expand Up @@ -1614,6 +1626,7 @@ def _render_line_in_row(
show_cursor,
self._show_hover_cursor,
self._update_count,
self._pseudo_class_state,
)

if cache_key in self._row_render_cache:
Expand Down Expand Up @@ -1786,6 +1799,7 @@ def _render_line(self, y: int, x1: int, x2: int, base_style: Style) -> Strip:
self.cursor_type,
self._show_hover_cursor,
self._update_count,
self._pseudo_class_state,
)
if cache_key in self._line_cache:
return self._line_cache[cache_key]
Expand All @@ -1810,6 +1824,10 @@ def _render_line(self, y: int, x1: int, x2: int, base_style: Style) -> Strip:
self._line_cache[cache_key] = strip
return strip

def render_lines(self, crop: Region) -> list[Strip]:
self._pseudo_class_state = self.get_pseudo_class_state()
return super().render_lines(crop)

def render_line(self, y: int) -> Strip:
width, height = self.size
scroll_x, scroll_y = self.scroll_offset
Expand Down Expand Up @@ -1842,6 +1860,9 @@ def on_mouse_move(self, event: events.MouseMove):
except KeyError:
pass

def on_leave(self, event: events.Leave) -> None:
self._set_hover_cursor(False)

def _get_fixed_offset(self) -> Spacing:
"""Calculate the "fixed offset", that is the space to the top and left
that is occupied by fixed rows and columns respectively. Fixed rows and columns
Expand Down
6 changes: 5 additions & 1 deletion src/textual/widgets/_tree.py
Original file line number Diff line number Diff line change
Expand Up @@ -925,6 +925,10 @@ def get_line_width(line: _TreeLine) -> int:
self.cursor_line = -1
self.refresh()

def render_lines(self, crop: Region) -> list[Strip]:
self._pseudo_class_state = self.get_pseudo_class_state()
return super().render_lines(crop)

def render_line(self, y: int) -> Strip:
width = self.size.width
scroll_x, scroll_y = self.scroll_offset
Expand Down Expand Up @@ -952,7 +956,7 @@ def _render_line(self, y: int, x1: int, x2: int, base_style: Style) -> Strip:
is_hover,
width,
self._updates,
self.has_focus,
self._pseudo_class_state,
tuple(node._updates for node in line.path),
)
if cache_key in self._line_cache:
Expand Down
17 changes: 17 additions & 0 deletions tests/test_data_table.py
Original file line number Diff line number Diff line change
Expand Up @@ -652,6 +652,23 @@ async def test_hover_coordinate():
assert table.hover_coordinate == Coordinate(1, 0)


async def test_hover_mouse_leave():
"""When the mouse cursor leaves the DataTable, there should be no hover highlighting."""
app = DataTableApp()
async with app.run_test() as pilot:
table = app.query_one(DataTable)
table.add_column("ABC")
table.add_row("123")
await pilot.pause()
assert table.hover_coordinate == Coordinate(0, 0)
# Hover over a cell, and the hover cursor is visible
await pilot.hover(DataTable, offset=Offset(1, 1))
assert table._show_hover_cursor
# Move our cursor away from the DataTable, and the hover cursor is hidden
await pilot.hover(DataTable, offset=Offset(-1, -1))
assert not table._show_hover_cursor


async def test_header_selected():
"""Ensure that a HeaderSelected event gets posted when we click
on the header in the DataTable."""
Expand Down
35 changes: 34 additions & 1 deletion tests/test_widget.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
from textual.css.errors import StyleValueError
from textual.css.query import NoMatches
from textual.geometry import Size
from textual.widget import MountError, Widget
from textual.widget import MountError, PseudoClasses, Widget
from textual.widgets import Label


Expand Down Expand Up @@ -178,6 +178,39 @@ def test_widget_mount_ids_must_be_unique_mounting_multiple_calls(parent):
parent.mount(widget2)


def test_get_pseudo_class_state():
widget = Widget()
pseudo_classes = widget.get_pseudo_class_state()
assert pseudo_classes == PseudoClasses(enabled=True, focus=False, hover=False)


def test_get_pseudo_class_state_disabled():
widget = Widget(disabled=True)
pseudo_classes = widget.get_pseudo_class_state()
assert pseudo_classes == PseudoClasses(enabled=False, focus=False, hover=False)


def test_get_pseudo_class_state_parent_disabled():
child = Widget()
_parent = Widget(child, disabled=True)
pseudo_classes = child.get_pseudo_class_state()
assert pseudo_classes == PseudoClasses(enabled=False, focus=False, hover=False)


def test_get_pseudo_class_state_hover():
widget = Widget()
widget.mouse_over = True
pseudo_classes = widget.get_pseudo_class_state()
assert pseudo_classes == PseudoClasses(enabled=True, focus=False, hover=True)


def test_get_pseudo_class_state_focus():
widget = Widget()
widget.has_focus = True
pseudo_classes = widget.get_pseudo_class_state()
assert pseudo_classes == PseudoClasses(enabled=True, focus=True, hover=False)


# Regression test for https://github.com/Textualize/textual/issues/1634
async def test_remove():
class RemoveMeLabel(Label):
Expand Down