From 17c6f3fc2a362efad8dd2a5d4f51d07e930e8199 Mon Sep 17 00:00:00 2001 From: darrenburns Date: Tue, 28 Mar 2023 14:26:24 +0100 Subject: [PATCH] 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 --- CHANGELOG.md | 2 ++ src/textual/widget.py | 30 +++++++++++++++++++++++++ src/textual/widgets/_data_table.py | 35 ++++++++++++++++++++++++------ src/textual/widgets/_tree.py | 6 ++++- tests/test_data_table.py | 17 +++++++++++++++ tests/test_widget.py | 35 +++++++++++++++++++++++++++++- 6 files changed, 116 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5e72bbe99..10de68990 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 diff --git a/src/textual/widget.py b/src/textual/widget.py index 1818cac89..09eda4f54 100644 --- a/src/textual/widget.py +++ b/src/textual/widget.py @@ -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. diff --git a/src/textual/widgets/_data_table.py b/src/textual/widgets/_data_table.py index e76b092b1..0ec6a23a6 100644 --- a/src/textual/widgets/_data_table.py +++ b/src/textual/widgets/_data_table.py @@ -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 diff --git a/src/textual/widgets/_tree.py b/src/textual/widgets/_tree.py index db0d77cf7..0b5034364 100644 --- a/src/textual/widgets/_tree.py +++ b/src/textual/widgets/_tree.py @@ -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: diff --git a/tests/test_data_table.py b/tests/test_data_table.py index 74c809dd6..faea0436f 100644 --- a/tests/test_data_table.py +++ b/tests/test_data_table.py @@ -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.""" diff --git a/tests/test_widget.py b/tests/test_widget.py index a51040a6c..11297682c 100644 --- a/tests/test_widget.py +++ b/tests/test_widget.py @@ -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):