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

* Using pseudoclass state in DataTable cache keys

* Use full pseudo-class state on tree cache key

* Adding tests for Widget.get_pseudo_class_state

* Test hiding hover cursor when mouse cursor leaves DataTable

* Update CHANGELOG.md
This commit is contained in:
darrenburns
2023-03-28 14:26:24 +01:00
committed by GitHub
parent 73f065bbbd
commit 17c6f3fc2a
6 changed files with 116 additions and 9 deletions

View File

@@ -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
@@ -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

View File

@@ -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):
"""
@@ -2357,6 +2366,27 @@ class Widget(DOMNode):
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.

View File

@@ -29,14 +29,13 @@ from ..reactive import Reactive
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")
@@ -609,6 +608,11 @@ class DataTable(ScrollView, Generic[CellType], can_focus=True):
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()
@@ -1558,7 +1562,15 @@ class DataTable(ScrollView, Generic[CellType], can_focus=True):
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})
@@ -1614,6 +1626,7 @@ class DataTable(ScrollView, Generic[CellType], can_focus=True):
show_cursor,
self._show_hover_cursor,
self._update_count,
self._pseudo_class_state,
)
if cache_key in self._row_render_cache:
@@ -1786,6 +1799,7 @@ class DataTable(ScrollView, Generic[CellType], can_focus=True):
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]
@@ -1810,6 +1824,10 @@ class DataTable(ScrollView, Generic[CellType], can_focus=True):
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
@@ -1842,6 +1860,9 @@ class DataTable(ScrollView, Generic[CellType], can_focus=True):
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

View File

@@ -925,6 +925,10 @@ class Tree(Generic[TreeDataType], ScrollView, can_focus=True):
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
@@ -952,7 +956,7 @@ class Tree(Generic[TreeDataType], ScrollView, can_focus=True):
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:

View File

@@ -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."""

View File

@@ -7,7 +7,7 @@ from textual.containers import Container
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
@@ -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):