From 73f065bbbd5d02d2536e6a20421bc8a685fb5103 Mon Sep 17 00:00:00 2001 From: Will McGugan Date: Tue, 28 Mar 2023 11:50:28 +0100 Subject: [PATCH 1/5] Logging handler (#2151) * logging handler * changelog * remove logging experiment * handler * fix * docs for logging handler --- CHANGELOG.md | 3 ++- docs/api/logging.md | 1 + docs/guide/devtools.md | 34 +++++++++++++++++++++++++++++++++- mkdocs-nav.yml | 1 + src/textual/__init__.py | 5 +++++ src/textual/_log.py | 1 + src/textual/logging.py | 32 ++++++++++++++++++++++++++++++++ src/textual/message_pump.py | 13 +++++++++++-- 8 files changed, 86 insertions(+), 4 deletions(-) create mode 100644 docs/api/logging.md create mode 100644 src/textual/logging.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 7ba83ff7c..5e72bbe99 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -29,6 +29,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/). - `Tree`: `clear`, `reset` - Screens with alpha in their background color will now blend with the background. https://github.com/Textualize/textual/pull/2139 - Added "thick" border style. https://github.com/Textualize/textual/pull/2139 +- message_pump.app will now set the active app if it is not already set. ### Added @@ -36,7 +37,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/). - Added scroll_end switch to TextLog.write https://github.com/Textualize/textual/pull/2127 - 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 ## [0.16.0] - 2023-03-22 diff --git a/docs/api/logging.md b/docs/api/logging.md new file mode 100644 index 000000000..5fbf833e1 --- /dev/null +++ b/docs/api/logging.md @@ -0,0 +1 @@ +::: textual.logging diff --git a/docs/guide/devtools.md b/docs/guide/devtools.md index fccbce004..087ebf4ce 100644 --- a/docs/guide/devtools.md +++ b/docs/guide/devtools.md @@ -120,5 +120,37 @@ class LogApp(App): if __name__ == "__main__": LogApp().run() - +``` + +### Logging handler + +Textual has a [logging handler][textual.logging.TextualHandler] which will write anything logged via the builtin logging library to the devtools. +This may be useful if you have a third-party library that uses the logging module, and you want to see those logs with Textual logs. + +!!! note + + The logging library works with strings only, so you won't be able to log Rich renderables such as `self.tree` with the logging handler. + +Here's an example of configuring logging to use the `TextualHandler`. + +```python +import logging +from textual.app import App +from textual.logging import TextualHandler + +logging.basicConfig( + level="NOTSET", + handlers=[TextualHandler()], +) + + +class LogApp(App): + """Using logging with Textual.""" + + def on_mount(self) -> None: + logging.debug("Logged via TextualHandler") + + +if __name__ == "__main__": + LogApp().run() ``` diff --git a/mkdocs-nav.yml b/mkdocs-nav.yml index c508c3619..e785346eb 100644 --- a/mkdocs-nav.yml +++ b/mkdocs-nav.yml @@ -169,6 +169,7 @@ nav: - "api/list_item.md" - "api/list_view.md" - "api/loading_indicator.md" + - "api/logging.md" - "api/markdown_viewer.md" - "api/markdown.md" - "api/message_pump.md" diff --git a/src/textual/__init__.py b/src/textual/__init__.py index d2e59f0bf..935f4da97 100644 --- a/src/textual/__init__.py +++ b/src/textual/__init__.py @@ -142,6 +142,11 @@ class Logger: """Logs system information.""" return Logger(self._log, LogGroup.SYSTEM) + @property + def logging(self) -> Logger: + """Logs from stdlib logging module.""" + return Logger(self._log, LogGroup.LOGGING) + log = Logger(None) diff --git a/src/textual/_log.py b/src/textual/_log.py index e9adcef42..26125052d 100644 --- a/src/textual/_log.py +++ b/src/textual/_log.py @@ -12,6 +12,7 @@ class LogGroup(Enum): ERROR = 5 PRINT = 6 SYSTEM = 7 + LOGGING = 8 class LogVerbosity(Enum): diff --git a/src/textual/logging.py b/src/textual/logging.py new file mode 100644 index 000000000..1ceb1bd2a --- /dev/null +++ b/src/textual/logging.py @@ -0,0 +1,32 @@ +import sys +from logging import Handler, LogRecord + +from ._context import active_app + + +class TextualHandler(Handler): + """A Logging handler for Textual apps.""" + + def __init__(self, stderr: bool = True, stdout: bool = False) -> None: + """Initialize a Textual logging handler. + + Args: + stderr: Log to stderr when there is no active app. + stdout: Log to stdout when there is not active app. + """ + super().__init__() + self._stderr = stderr + self._stdout = stdout + + def emit(self, record: LogRecord) -> None: + """Invoked by logging.""" + message = self.format(record) + try: + app = active_app.get() + except LookupError: + if self._stderr: + print(message, file=sys.stderr) + elif self._stdout: + print(message, file=sys.stdout) + else: + app.log.logging(message) diff --git a/src/textual/message_pump.py b/src/textual/message_pump.py index 9163811d5..6599f7191 100644 --- a/src/textual/message_pump.py +++ b/src/textual/message_pump.py @@ -156,17 +156,26 @@ class MessagePump(metaclass=MessagePumpMeta): try: return active_app.get() except LookupError: - raise NoActiveAppError() + from .app import App + + node: MessagePump | None = self + while not isinstance(node, App): + if node is None: + raise NoActiveAppError() + node = node._parent + active_app.set(node) + return node @property def is_parent_active(self) -> bool: + """Is the parent active?""" return bool( self._parent and not self._parent._closed and not self._parent._closing ) @property def is_running(self) -> bool: - """bool: Is the message pump running (potentially processing messages).""" + """Is the message pump running (potentially processing messages).""" return self._running @property From 17c6f3fc2a362efad8dd2a5d4f51d07e930e8199 Mon Sep 17 00:00:00 2001 From: darrenburns Date: Tue, 28 Mar 2023 14:26:24 +0100 Subject: [PATCH 2/5] 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): From b005c13956be1e248cab3f79b8d9977947ecc5da Mon Sep 17 00:00:00 2001 From: Will McGugan Date: Tue, 28 Mar 2023 15:27:01 +0100 Subject: [PATCH 3/5] remove arrangement spacing (#2157) * remove arrangement spacing * snapshot test * comment * changelog --- CHANGELOG.md | 1 + src/textual/_arrange.py | 2 +- src/textual/_compositor.py | 5 +- src/textual/_layout.py | 6 +- .../__snapshots__/test_snapshots.ambr | 159 ++++++++++++++++++ .../snapshot_tests/snapshot_apps/layer_fix.py | 48 ++++++ tests/snapshot_tests/test_snapshots.py | 5 + tests/test_arrange.py | 5 - 8 files changed, 217 insertions(+), 14 deletions(-) create mode 100644 tests/snapshot_tests/snapshot_apps/layer_fix.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 10de68990..3aec5a144 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 +- Fixed scrollbar layers issue https://github.com/Textualize/textual/issues/1358 - Fix for interaction between pseudo-classes and widget-level render caches https://github.com/Textualize/textual/pull/2155 ### Changed diff --git a/src/textual/_arrange.py b/src/textual/_arrange.py index f1035688e..378d951eb 100644 --- a/src/textual/_arrange.py +++ b/src/textual/_arrange.py @@ -128,4 +128,4 @@ def arrange( placements.extend(layout_placements) - return DockArrangeResult(placements, arrange_widgets, scroll_spacing) + return DockArrangeResult(placements, arrange_widgets) diff --git a/src/textual/_compositor.py b/src/textual/_compositor.py index dd7816f9c..addec6ea8 100644 --- a/src/textual/_compositor.py +++ b/src/textual/_compositor.py @@ -484,7 +484,6 @@ class Compositor: # Arrange the layout arrange_result = widget._arrange(child_region.size) arranged_widgets = arrange_result.widgets - spacing = arrange_result.spacing widgets.update(arranged_widgets) if visible_only: @@ -513,9 +512,7 @@ class Compositor: if fixed: widget_region = sub_region + placement_offset else: - total_region = total_region.union( - sub_region.grow(spacing + margin) - ) + total_region = total_region.union(sub_region.grow(margin)) widget_region = sub_region + placement_scroll_offset widget_order = order + ( diff --git a/src/textual/_layout.py b/src/textual/_layout.py index 0cdfe5956..5c5dc2ff1 100644 --- a/src/textual/_layout.py +++ b/src/textual/_layout.py @@ -21,8 +21,6 @@ class DockArrangeResult: """A `WidgetPlacement` for every widget to describe it's location on screen.""" widgets: set[Widget] """A set of widgets in the arrangement.""" - spacing: Spacing - """Shared spacing around the widgets.""" _spatial_map: SpatialMap[WidgetPlacement] | None = None """A Spatial map to query widget placements.""" @@ -113,7 +111,7 @@ class Layout(ABC): else: # Use a size of 0, 0 to ignore relative sizes, since those are flexible anyway arrangement = widget._arrange(Size(0, 0)) - return arrangement.total_region.right + arrangement.spacing.right + return arrangement.total_region.right return width def get_content_height( @@ -135,5 +133,5 @@ class Layout(ABC): else: # Use a height of zero to ignore relative heights arrangement = widget._arrange(Size(width, 0)) - height = arrangement.total_region.bottom + arrangement.spacing.bottom + height = arrangement.total_region.bottom return height diff --git a/tests/snapshot_tests/__snapshots__/test_snapshots.ambr b/tests/snapshot_tests/__snapshots__/test_snapshots.ambr index 72fc4afec..0f3d41b3e 100644 --- a/tests/snapshot_tests/__snapshots__/test_snapshots.ambr +++ b/tests/snapshot_tests/__snapshots__/test_snapshots.ambr @@ -15508,6 +15508,165 @@ ''' # --- +# name: test_layer_fix + ''' + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + DialogIssueApp + + + + + + + + + + ⭘DialogIssueApp + + + + + + ╭───────────────────────────────────────╮ + ││ + ││ + ││ + ││ + ││ + │This should not cause a scrollbar to ap│ + ││ + ││ + ││ + ││ + ││ + ╰───────────────────────────────────────╯ + + + + +  D  Toggle the dialog  + + + + + ''' +# --- # name: test_layers ''' diff --git a/tests/snapshot_tests/snapshot_apps/layer_fix.py b/tests/snapshot_tests/snapshot_apps/layer_fix.py new file mode 100644 index 000000000..2cc8861a8 --- /dev/null +++ b/tests/snapshot_tests/snapshot_apps/layer_fix.py @@ -0,0 +1,48 @@ +from textual.app import App, ComposeResult +from textual.containers import Vertical +from textual.widgets import Header, Footer, Label +from textual.binding import Binding + + +class Dialog(Vertical): + def compose(self) -> ComposeResult: + """Compose the child widgets.""" + yield Label("This should not cause a scrollbar to appear") + + +class DialogIssueApp(App[None]): + CSS = """ + Screen { + layers: base dialog; + } + + .hidden { + display: none; + } + + Dialog { + align: center middle; + border: round red; + width: 50%; + height: 50%; + layer: dialog; + offset: 50% 50%; + } + """ + + BINDINGS = [ + Binding("d", "dialog", "Toggle the dialog"), + ] + + def compose(self) -> ComposeResult: + yield Header() + yield Vertical() + yield Dialog(classes="hidden") + yield Footer() + + def action_dialog(self) -> None: + self.query_one(Dialog).toggle_class("hidden") + + +if __name__ == "__main__": + DialogIssueApp().run() diff --git a/tests/snapshot_tests/test_snapshots.py b/tests/snapshot_tests/test_snapshots.py index 7c68f1405..ec268741b 100644 --- a/tests/snapshot_tests/test_snapshots.py +++ b/tests/snapshot_tests/test_snapshots.py @@ -340,3 +340,8 @@ def test_scrollbar_thumb_height(snap_compare): assert snap_compare( SNAPSHOT_APPS_DIR / "scrollbar_thumb_height.py", ) + + +def test_layer_fix(snap_compare): + # Check https://github.com/Textualize/textual/issues/1358 + assert snap_compare(SNAPSHOT_APPS_DIR / "layer_fix.py", press=["d"]) diff --git a/tests/test_arrange.py b/tests/test_arrange.py index 582f54bc1..38f5a191d 100644 --- a/tests/test_arrange.py +++ b/tests/test_arrange.py @@ -12,7 +12,6 @@ def test_arrange_empty(): result = arrange(container, [], Size(80, 24), Size(80, 24)) assert result.placements == [] assert result.widgets == set() - assert result.spacing == Spacing(0, 0, 0, 0) def test_arrange_dock_top(): @@ -31,7 +30,6 @@ def test_arrange_dock_top(): WidgetPlacement(Region(0, 1, 80, 23), Spacing(), child, order=0, fixed=False), ] assert result.widgets == {child, header} - assert result.spacing == Spacing(1, 0, 0, 0) def test_arrange_dock_left(): @@ -49,7 +47,6 @@ def test_arrange_dock_left(): WidgetPlacement(Region(10, 0, 70, 24), Spacing(), child, order=0, fixed=False), ] assert result.widgets == {child, header} - assert result.spacing == Spacing(0, 0, 0, 10) def test_arrange_dock_right(): @@ -67,7 +64,6 @@ def test_arrange_dock_right(): WidgetPlacement(Region(0, 0, 70, 24), Spacing(), child, order=0, fixed=False), ] assert result.widgets == {child, header} - assert result.spacing == Spacing(0, 10, 0, 0) def test_arrange_dock_bottom(): @@ -85,7 +81,6 @@ def test_arrange_dock_bottom(): WidgetPlacement(Region(0, 0, 80, 23), Spacing(), child, order=0, fixed=False), ] assert result.widgets == {child, header} - assert result.spacing == Spacing(0, 0, 1, 0) def test_arrange_dock_badly(): From 85a6f8343e32ffda4e211c1125b73c6a69814c17 Mon Sep 17 00:00:00 2001 From: Will McGugan Date: Tue, 28 Mar 2023 15:29:33 +0100 Subject: [PATCH 4/5] change Any import --- src/textual/actions.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/textual/actions.py b/src/textual/actions.py index a189b9a58..9365bb268 100644 --- a/src/textual/actions.py +++ b/src/textual/actions.py @@ -2,8 +2,9 @@ from __future__ import annotations import ast import re +from typing import Any -from typing_extensions import Any, TypeAlias +from typing_extensions import TypeAlias ActionParseResult: TypeAlias = "tuple[str, tuple[Any, ...]]" """An action is its name and the arbitrary tuple of its arguments.""" From eaca92bd379178d1f21ab900fdcb638c8870977d Mon Sep 17 00:00:00 2001 From: Will McGugan Date: Tue, 28 Mar 2023 15:45:40 +0100 Subject: [PATCH 5/5] Set classes (#2160) * Setting classes * changelog * docstrings * test bad identifiers * Update CHANGELOG.md Co-authored-by: Dave Pearson * Add return of self --------- Co-authored-by: Dave Pearson --- CHANGELOG.md | 1 + src/textual/css/query.py | 21 ++++++++++- src/textual/dom.py | 78 +++++++++++++++++++++++++++++++++------- tests/test_dom.py | 33 +++++++++++++++++ tests/test_query.py | 19 ++++++++++ 5 files changed, 138 insertions(+), 14 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3aec5a144..be188afcd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -41,6 +41,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/). - 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 +- Added Query.set_classes, DOMNode.set_classes, and `classes` setter for Widget https://github.com/Textualize/textual/issues/1081 ## [0.16.0] - 2023-03-22 diff --git a/src/textual/css/query.py b/src/textual/css/query.py index 73e09c61b..98e0063a0 100644 --- a/src/textual/css/query.py +++ b/src/textual/css/query.py @@ -16,7 +16,7 @@ a method which evaluates the query, such as first() and last(). from __future__ import annotations -from typing import TYPE_CHECKING, Generic, Iterator, TypeVar, cast, overload +from typing import TYPE_CHECKING, Generic, Iterable, Iterator, TypeVar, cast, overload import rich.repr @@ -330,6 +330,25 @@ class DOMQuery(Generic[QueryType]): node.set_class(add, *class_names) return self + def set_classes(self, classes: str | Iterable[str]) -> DOMQuery[QueryType]: + """Set the classes on nodes to exactly the given set. + + Args: + classes: A string of space separated classes, or an iterable of class names. + + Returns: + Self. + """ + + if isinstance(classes, str): + for node in self: + node.set_classes(classes) + else: + class_names = list(classes) + for node in self: + node.set_classes(class_names) + return self + def add_class(self, *class_names: str) -> DOMQuery[QueryType]: """Add the given class name(s) to nodes.""" for node in self: diff --git a/src/textual/dom.py b/src/textual/dom.py index cb9881f65..a5a8c0185 100644 --- a/src/textual/dom.py +++ b/src/textual/dom.py @@ -83,6 +83,26 @@ class NoScreen(DOMError): pass +class _ClassesDescriptor: + """A descriptor to manage the `classes` property.""" + + def __get__( + self, obj: DOMNode, objtype: type[DOMNode] | None = None + ) -> frozenset[str]: + """A frozenset of the current classes on the widget.""" + return frozenset(obj._classes) + + def __set__(self, obj: DOMNode, classes: str | Iterable[str]) -> None: + """Replaces classes entirely.""" + if isinstance(classes, str): + class_names = set(classes.split()) + else: + class_names = set(classes) + check_identifiers("class name", *class_names) + obj._classes = class_names + obj._update_styles() + + @rich.repr.auto class DOMNode(MessagePump): """The base class for object that can be in the Textual DOM (App and Widget)""" @@ -428,10 +448,7 @@ class DOMNode(MessagePump): tokens.append(f"[name={self.name}]", style="underline") return tokens - @property - def classes(self) -> frozenset[str]: - """A frozenset of the current classes set on the widget.""" - return frozenset(self._classes) + classes = _ClassesDescriptor() @property def pseudo_classes(self) -> frozenset[str]: @@ -851,8 +868,16 @@ class DOMNode(MessagePump): return query.only_one() if expect_type is None else query.only_one(expect_type) - def set_styles(self, css: str | None = None, **update_styles) -> None: - """Set custom styles on this object.""" + def set_styles(self, css: str | None = None, **update_styles) -> Self: + """Set custom styles on this object. + + Args: + css: Styles in CSS format. + **update_styles: Keyword arguments map style names on to style. + + Returns: + Self. + """ if css is not None: try: @@ -877,16 +902,32 @@ class DOMNode(MessagePump): """ return self._classes.issuperset(class_names) - def set_class(self, add: bool, *class_names: str) -> None: + def set_class(self, add: bool, *class_names: str) -> Self: """Add or remove class(es) based on a condition. Args: add: Add the classes if True, otherwise remove them. + + Returns: + Self. """ if add: self.add_class(*class_names) else: self.remove_class(*class_names) + return self + + def set_classes(self, classes: str | Iterable[str]) -> Self: + """Replace all classes. + + Args: + A string contain space separated classes, or an iterable of class names. + + Returns: + Self. + """ + self.classes = classes + return self def _update_styles(self) -> None: """Request an update of this node's styles. @@ -898,45 +939,56 @@ class DOMNode(MessagePump): except NoActiveAppError: pass - def add_class(self, *class_names: str) -> None: + def add_class(self, *class_names: str) -> Self: """Add class names to this Node. Args: *class_names: CSS class names to add. + Returns: + Self. """ check_identifiers("class name", *class_names) old_classes = self._classes.copy() self._classes.update(class_names) if old_classes == self._classes: - return + return self self._update_styles() + return self - def remove_class(self, *class_names: str) -> None: + def remove_class(self, *class_names: str) -> Self: """Remove class names from this Node. Args: *class_names: CSS class names to remove. + + Returns: + Self. """ check_identifiers("class name", *class_names) old_classes = self._classes.copy() self._classes.difference_update(class_names) if old_classes == self._classes: - return + return self self._update_styles() + return self - def toggle_class(self, *class_names: str) -> None: + def toggle_class(self, *class_names: str) -> Self: """Toggle class names on this Node. Args: *class_names: CSS class names to toggle. + + Returns: + Self. """ check_identifiers("class name", *class_names) old_classes = self._classes.copy() self._classes.symmetric_difference_update(class_names) if old_classes == self._classes: - return + return self self._update_styles() + return self def has_pseudo_class(self, *class_names: str) -> bool: """Check for pseudo classes (such as hover, focus etc) diff --git a/tests/test_dom.py b/tests/test_dom.py index b78a958c9..1b354067c 100644 --- a/tests/test_dom.py +++ b/tests/test_dom.py @@ -45,6 +45,39 @@ def test_validate(): node.toggle_class("1") +def test_classes_setter(): + node = DOMNode(classes="foo bar") + assert node.classes == frozenset({"foo", "bar"}) + node.classes = "baz egg" + assert node.classes == frozenset({"baz", "egg"}) + node.classes = "" + assert node.classes == frozenset({}) + + +def test_classes_setter_iterable(): + node = DOMNode(classes="foo bar") + assert node.classes == frozenset({"foo", "bar"}) + node.classes = "baz", "egg" + assert node.classes == frozenset({"baz", "egg"}) + node.classes = [] + assert node.classes == frozenset({}) + + +def test_classes_set_classes(): + node = DOMNode(classes="foo bar") + assert node.classes == frozenset({"foo", "bar"}) + node.set_classes("baz egg") + assert node.classes == frozenset({"baz", "egg"}) + node.set_classes([]) + assert node.classes == frozenset({}) + node.set_classes(["paul"]) + assert node.classes == frozenset({"paul"}) + with pytest.raises(BadIdentifier): + node.classes = "foo 25" + with pytest.raises(BadIdentifier): + node.classes = ["foo", "25"] + + def test_inherited_bindings(): """Test if binding merging is done correctly when (not) inheriting bindings.""" diff --git a/tests/test_query.py b/tests/test_query.py index c58ffd575..60dced971 100644 --- a/tests/test_query.py +++ b/tests/test_query.py @@ -152,6 +152,25 @@ def test_query_classes(): # Now, let's check there are *no* children with the test class. assert len(app.query(".test")) == 0 + # Add classes via set_classes + app.query(ClassTest).set_classes("foo bar") + assert (len(app.query(".foo"))) == CHILD_COUNT + assert (len(app.query(".bar"))) == CHILD_COUNT + + # Reset classes + app.query(ClassTest).set_classes("") + assert (len(app.query(".foo"))) == 0 + assert (len(app.query(".bar"))) == 0 + + # Repeat, to check setting empty iterable + app.query(ClassTest).set_classes("foo bar") + assert (len(app.query(".foo"))) == CHILD_COUNT + assert (len(app.query(".bar"))) == CHILD_COUNT + + app.query(ClassTest).set_classes([]) + assert (len(app.query(".foo"))) == 0 + assert (len(app.query(".bar"))) == 0 + # Add the test class to everything and then check again. app.query(ClassTest).add_class("test") assert len(app.query(".test")) == CHILD_COUNT