Merge branch 'main' into cli-keys

CHANGELOG.md

@@ -5,6 +5,20 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](http://keepachangelog.com/)
 and this project adheres to [Semantic Versioning](http://semver.org/).
 
+## [0.8.0] - Unreleased
+
+### Fixed 
+
+- Fixed issues with nested auto dimensions https://github.com/Textualize/textual/issues/1402
+
+### Added
+
+- Added `textual.actions.SkipAction` exception which can be raised from an action to allow parents to process bindings.
+
+### Fixed
+
+- Fixed watch method incorrectly running on first set when value hasnt changed and init=False https://github.com/Textualize/textual/pull/1367
+
 ## [0.7.0] - 2022-12-17
 
 ### Added
@@ -24,6 +38,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 - Fixed validator not running on first reactive set https://github.com/Textualize/textual/pull/1359
 - Ensure only printable characters are used as key_display https://github.com/Textualize/textual/pull/1361
 
+
 ## [0.6.0] - 2022-12-11
 
 ### Added

docs/guide/actions.md

@@ -75,7 +75,7 @@ When you click any of the links, Textual runs the `"set_background"` action to c
 
 ## Bindings
 
-Textual will also run actions bound to keys. The following example adds key [bindings](./input.md#bindings) for the ++r++, ++g++, and ++b++ keys which call the `"set_background"` action.
+Textual will run actions bound to keys. The following example adds key [bindings](./input.md#bindings) for the ++r++, ++g++, and ++b++ keys which call the `"set_background"` action.
 
 === "actions04.py"
 
@@ -92,7 +92,7 @@ If you run this example, you can change the background by pressing keys in addit
 
 ## Namespaces
 
-Textual will look for action methods on the widget or app where they are used. If we were to create a [custom widget](./widgets.md#custom-widgets) it can have its own set of actions.
+Textual will look for action methods in the class where they are defined (App, Screen, or Widget). If we were to create a [custom widget](./widgets.md#custom-widgets) it can have its own set of actions.
 
 The following example defines a custom widget with its own `set_background` action.
 

src/textual/actions.py

@@ -4,6 +4,10 @@ import ast
 import re
 
 
+class SkipAction(Exception):
+    """Raise in an action to skip the action (and allow any parent bindings to run)."""
+
+
 class ActionError(Exception):
     pass
 

src/textual/app.py

@@ -38,7 +38,7 @@ from rich.protocol import is_renderable
 from rich.segment import Segment, Segments
 from rich.traceback import Traceback
 
-from . import Logger, LogGroup, LogVerbosity, actions, events, log, messages
+from . import actions, Logger, LogGroup, LogVerbosity, events, log, messages
 from ._animator import DEFAULT_EASING, Animatable, Animator, EasingFunction
 from ._ansi_sequences import SYNC_END, SYNC_START
 from ._callback import invoke
@@ -47,6 +47,7 @@ from ._event_broker import NoHandler, extract_handler_actions
 from ._filter import LineFilter, Monochrome
 from ._path import _make_path_object_relative
 from ._typing import Final, TypeAlias
+from .actions import SkipAction
 from .await_remove import AwaitRemove
 from .binding import Binding, Bindings
 from .css.query import NoMatches
@@ -1393,11 +1394,10 @@ class App(Generic[ReturnType], DOMNode):
                 raise
 
             finally:
+                self._running = True
                 await self._ready()
                 await invoke_ready_callback()
 
-            self._running = True
-
             try:
                 await self._process_messages_loop()
             except asyncio.CancelledError:
@@ -1760,8 +1760,8 @@ class App(Generic[ReturnType], DOMNode):
         ):
             binding = bindings.keys.get(key)
             if binding is not None and binding.priority == priority:
-                await self.action(binding.action, default_namespace=namespace)
+                if await self.action(binding.action, namespace):
-                return True
+                    return True
         return False
 
     async def on_event(self, event: events.Event) -> None:
@@ -1830,32 +1830,41 @@ class App(Generic[ReturnType], DOMNode):
     async def _dispatch_action(
         self, namespace: object, action_name: str, params: Any
     ) -> bool:
+        """Dispatch an action to an action method.
+
+        Args:
+            namespace (object): Namespace (object) of action.
+            action_name (str): Name of the action.
+            params (Any): Action parameters.
+
+        Returns:
+            bool: True if handled, otherwise False.
+        """
+        _rich_traceback_guard = True
+
         log(
             "<action>",
             namespace=namespace,
             action_name=action_name,
             params=params,
         )
-        _rich_traceback_guard = True
 
-        public_method_name = f"action_{action_name}"
+        try:
-        private_method_name = f"_{public_method_name}"
+            private_method = getattr(namespace, f"_action_{action_name}", None)
-
+            if callable(private_method):
-        private_method = getattr(namespace, private_method_name, None)
+                await invoke(private_method, *params)
-        public_method = getattr(namespace, public_method_name, None)
+                return True
-
+            public_method = getattr(namespace, f"action_{action_name}", None)
-        if private_method is None and public_method is None:
+            if callable(public_method):
+                await invoke(public_method, *params)
+                return True
             log(
-                f"<action> {action_name!r} has no target. Couldn't find methods {public_method_name!r} or {private_method_name!r}"
+                f"<action> {action_name!r} has no target."
+                f" Could not find methods '_action_{action_name}' or 'action_{action_name}'"
             )
-
+        except SkipAction:
-        if callable(private_method):
+            # The action method raised this to explicitly not handle the action
-            await invoke(private_method, *params)
+            log("<action> {action_name!r} skipped.")
-            return True
-        elif callable(public_method):
-            await invoke(public_method, *params)
-            return True
-
         return False
 
     async def _broker_event(
@@ -1866,7 +1875,7 @@ class App(Generic[ReturnType], DOMNode):
         Args:
             event_name (str): _description_
             event (events.Event): An event object.
-            default_namespace (object | None): TODO: _description_
+            default_namespace (object | None): The default namespace, where one isn't supplied.
 
         Returns:
             bool: True if an action was processed.

src/textual/css/_style_properties.py

@@ -794,9 +794,8 @@ class NameListProperty:
 class ColorProperty:
     """Descriptor for getting and setting color properties."""
 
-    def __init__(self, default_color: Color | str, background: bool = False) -> None:
+    def __init__(self, default_color: Color | str) -> None:
         self._default_color = Color.parse(default_color)
-        self._is_background = background
 
     def __set_name__(self, owner: StylesBase, name: str) -> None:
         self.name = name
@@ -830,11 +829,10 @@ class ColorProperty:
         _rich_traceback_omit = True
         if color is None:
             if obj.clear_rule(self.name):
-                obj.refresh(children=self._is_background)
+                obj.refresh(children=True)
         elif isinstance(color, Color):
             if obj.set_rule(self.name, color):
-                obj.refresh(children=self._is_background)
+                obj.refresh(children=True)
-
         elif isinstance(color, str):
             alpha = 1.0
             parsed_color = Color(255, 255, 255)
@@ -855,8 +853,9 @@ class ColorProperty:
                         ),
                     )
             parsed_color = parsed_color.with_alpha(alpha)
+
             if obj.set_rule(self.name, parsed_color):
-                obj.refresh(children=self._is_background)
+                obj.refresh(children=True)
         else:
             raise StyleValueError(f"Invalid color value {color}")
 

src/textual/css/styles.py

@@ -214,7 +214,7 @@ class StylesBase(ABC):
 
     auto_color = BooleanProperty(default=False)
     color = ColorProperty(Color(255, 255, 255))
-    background = ColorProperty(Color(0, 0, 0, 0), background=True)
+    background = ColorProperty(Color(0, 0, 0, 0))
     text_style = StyleFlagsProperty()
 
     opacity = FractionalProperty()
@@ -421,7 +421,7 @@ class StylesBase(ABC):
 
         Args:
             layout (bool, optional): Also require a layout. Defaults to False.
-            children (bool, opional): Also refresh children. Defaults to False.
+            children (bool, optional): Also refresh children. Defaults to False.
         """
 
     @abstractmethod

src/textual/reactive.py

@@ -175,15 +175,11 @@ class Reactive(Generic[ReactiveType]):
         current_value = getattr(obj, name)
         # Check for validate function
         validate_function = getattr(obj, f"validate_{name}", None)
-        # Check if this is the first time setting the value
-        first_set = getattr(obj, f"__first_set_{self.internal_name}", True)
         # Call validate
         if callable(validate_function):
             value = validate_function(value)
         # If the value has changed, or this is the first time setting the value
-        if current_value != value or first_set or self._always_update:
+        if current_value != value or self._always_update:
-            # Set the first set flag to False
-            setattr(obj, f"__first_set_{self.internal_name}", False)
             # Store the internal value
             setattr(obj, self.internal_name, value)
             # Check all watchers
@@ -200,7 +196,6 @@ class Reactive(Generic[ReactiveType]):
             obj (Reactable): The reactable object.
             name (str): Attribute name.
             old_value (Any): The old (previous) value of the attribute.
-            first_set (bool, optional): True if this is the first time setting the value. Defaults to False.
         """
         _rich_traceback_omit = True
         # Get the current value.

src/textual/screen.py

@@ -295,7 +295,6 @@ class Screen(Widget):
             # No focus, so blur currently focused widget if it exists
             if self.focused is not None:
                 self.focused.post_message_no_wait(events.Blur(self))
-                self.focused.emit_no_wait(events.DescendantBlur(self))
                 self.focused = None
             self.log.debug("focus was removed")
         elif widget.can_focus:
@@ -303,14 +302,12 @@ class Screen(Widget):
                 if self.focused is not None:
                     # Blur currently focused widget
                     self.focused.post_message_no_wait(events.Blur(self))
-                    self.focused.emit_no_wait(events.DescendantBlur(self))
                 # Change focus
                 self.focused = widget
                 # Send focus event
                 if scroll_visible:
                     self.screen.scroll_to_widget(widget)
                 widget.post_message_no_wait(events.Focus(self))
-                widget.emit_no_wait(events.DescendantFocus(self))
                 self.log.debug(widget, "was focused")
 
     async def _on_idle(self, event: events.Idle) -> None:

src/textual/widget.py

@@ -44,6 +44,7 @@ from ._layout import Layout
 from ._segment_tools import align_lines
 from ._styles_cache import StylesCache
 from ._types import Lines
+from .actions import SkipAction
 from .await_remove import AwaitRemove
 from .binding import Binding
 from .box_model import BoxModel, get_box_model
@@ -2175,8 +2176,14 @@ class Widget(DOMNode):
 
         if layout:
             self._layout_required = True
-            if isinstance(self._parent, Widget):
+            for ancestor in self.ancestors:
-                self._parent._clear_arrangement_cache()
+                if not isinstance(ancestor, Widget):
+                    break
+                if ancestor.styles.auto_dimensions:
+                    for ancestor in self.ancestors_with_self:
+                        if isinstance(ancestor, Widget):
+                            ancestor._clear_arrangement_cache()
+                    break
 
         if repaint:
             self._set_dirty(*regions)
@@ -2344,17 +2351,22 @@ class Widget(DOMNode):
         self.mouse_over = True
 
     def _on_focus(self, event: events.Focus) -> None:
-        for node in self.ancestors_with_self:
-            if node._has_focus_within:
-                self.app.update_styles(node)
         self.has_focus = True
         self.refresh()
+        self.emit_no_wait(events.DescendantFocus(self))
 
     def _on_blur(self, event: events.Blur) -> None:
-        if any(node._has_focus_within for node in self.ancestors_with_self):
-            self.app.update_styles(self)
         self.has_focus = False
         self.refresh()
+        self.emit_no_wait(events.DescendantBlur(self))
+
+    def _on_descendant_blur(self, event: events.DescendantBlur) -> None:
+        if self._has_focus_within:
+            self.app.update_styles(self)
+
+    def _on_descendant_focus(self, event: events.DescendantBlur) -> None:
+        if self._has_focus_within:
+            self.app.update_styles(self)
 
     def _on_mouse_scroll_down(self, event) -> None:
         if self.allow_vertical_scroll:
@@ -2399,33 +2411,41 @@ class Widget(DOMNode):
         self.scroll_to_region(message.region, animate=True)
 
     def action_scroll_home(self) -> None:
-        if self._allow_scroll:
+        if not self._allow_scroll:
-            self.scroll_home()
+            raise SkipAction()
+        self.scroll_home()
 
     def action_scroll_end(self) -> None:
-        if self._allow_scroll:
+        if not self._allow_scroll:
-            self.scroll_end()
+            raise SkipAction()
+        self.scroll_end()
 
     def action_scroll_left(self) -> None:
-        if self.allow_horizontal_scroll:
+        if not self.allow_horizontal_scroll:
-            self.scroll_left()
+            raise SkipAction()
+        self.scroll_left()
 
     def action_scroll_right(self) -> None:
-        if self.allow_horizontal_scroll:
+        if not self.allow_horizontal_scroll:
-            self.scroll_right()
+            raise SkipAction()
+        self.scroll_right()
 
     def action_scroll_up(self) -> None:
-        if self.allow_vertical_scroll:
+        if not self.allow_vertical_scroll:
-            self.scroll_up()
+            raise SkipAction()
+        self.scroll_up()
 
     def action_scroll_down(self) -> None:
-        if self.allow_vertical_scroll:
+        if not self.allow_vertical_scroll:
-            self.scroll_down()
+            raise SkipAction()
+        self.scroll_down()
 
     def action_page_down(self) -> None:
-        if self.allow_vertical_scroll:
+        if not self.allow_vertical_scroll:
-            self.scroll_page_down()
+            raise SkipAction()
+        self.scroll_page_down()
 
     def action_page_up(self) -> None:
-        if self.allow_vertical_scroll:
+        if not self.allow_vertical_scroll:
-            self.scroll_page_up()
+            raise SkipAction()
+        self.scroll_page_up()

src/textual/widgets/_static.py

@@ -24,7 +24,7 @@ def _check_renderable(renderable: object):
         )
 
 
-class Static(Widget):
+class Static(Widget, inherit_bindings=False):
     """A widget to display simple static content, or use as a base class for more complex widgets.
 
     Args:

tests/snapshot_tests/conftest.py

@@ -188,7 +188,7 @@ def pytest_terminal_summary(
     Displays the link to the snapshot report that was generated in a prior hook.
     """
     diffs = getattr(config, "_textual_snapshots", None)
-    console = Console()
+    console = Console(legacy_windows=False, force_terminal=True)
     if diffs:
         snapshot_report_location = config._textual_snapshot_html_report
         console.rule("[b red]Textual Snapshot Report", style="red")

tests/snapshot_tests/snapshot_apps/nested_auto_heights.py

@@ -0,0 +1,64 @@
+#!/usr/bin/env python3
+
+from __future__ import annotations
+
+from textual.app import App, ComposeResult
+from textual.containers import Vertical
+from textual.widgets import Static
+
+
+class NestedAutoApp(App[None]):
+    CSS = """
+    Screen {
+        background: red;
+    }
+
+    #my-static-container {
+        border: heavy lightgreen;
+        background: green;
+        height: auto;
+        max-height: 10;
+    }
+
+    #my-static-wrapper {
+        border: heavy lightblue;
+        background: blue;
+        width: auto;
+        height: auto;
+    }
+
+    #my-static {
+        border: heavy gray;
+        background: black;
+        width: auto;
+        height: auto;
+    }
+    """
+    BINDINGS = [
+        ("1", "1", "1"),
+        ("2", "2", "2"),
+        ("q", "quit", "Quit"),
+    ]
+
+    def compose(self) -> ComposeResult:
+        self._static = Static("", id="my-static")
+        yield Vertical(
+            Vertical(
+                self._static,
+                id="my-static-wrapper",
+            ),
+            id="my-static-container",
+        )
+
+    def action_1(self) -> None:
+        self._static.update(
+            "\n".join(f"Lorem {i} Ipsum {i} Sit {i}" for i in range(1, 21))
+        )
+
+    def action_2(self) -> None:
+        self._static.update("JUST ONE LINE")
+
+
+if __name__ == "__main__":
+    app = NestedAutoApp()
+    app.run()

tests/snapshot_tests/test_snapshots.py

@@ -101,7 +101,9 @@ def test_header_render(snap_compare):
 
 
 def test_list_view(snap_compare):
-    assert snap_compare(WIDGET_EXAMPLES_DIR / "list_view.py", press=["tab", "down", "down", "up"])
+    assert snap_compare(
+        WIDGET_EXAMPLES_DIR / "list_view.py", press=["tab", "down", "down", "up"]
+    )
 
 
 def test_textlog_max_lines(snap_compare):
@@ -160,6 +162,11 @@ def test_offsets(snap_compare):
     assert snap_compare("snapshot_apps/offsets.py")
 
 
+def test_nested_auto_heights(snap_compare):
+    """Test refreshing widget within a auto sized container"""
+    assert snap_compare("snapshot_apps/nested_auto_heights.py", press=["1", "2", "_"])
+
+
 # --- Other ---
 
 
@@ -169,4 +176,8 @@ def test_key_display(snap_compare):
 
 def test_demo(snap_compare):
     """Test the demo app (python -m textual)"""
-    assert snap_compare(Path("../../src/textual/demo.py"))
+    assert snap_compare(
+        Path("../../src/textual/demo.py"),
+        press=["down", "down", "down", "_"],
+        terminal_size=(100, 30),
+    )

tests/test_binding_inheritance.py

@@ -11,13 +11,13 @@ background relating to this.
 
 from __future__ import annotations
 
-import pytest
+from textual.actions import SkipAction
-
 from textual.app import App, ComposeResult
-from textual.widgets import Static
-from textual.screen import Screen
 from textual.binding import Binding
 from textual.containers import Container
+from textual.screen import Screen
+from textual.widget import Widget
+from textual.widgets import Static
 
 ##############################################################################
 # These are the movement keys within Textual; they kind of have a special
@@ -614,3 +614,40 @@ async def test_overlapping_priority_bindings() -> None:
             "app_e",
             "screen_f",
         ]
+
+
+async def test_skip_action() -> None:
+    """Test that a binding may be skipped by an action raising SkipAction"""
+
+    class Handle(Widget, can_focus=True):
+        BINDINGS = [("t", "test('foo')", "Test")]
+
+        def action_test(self, text: str) -> None:
+            self.app.exit(text)
+
+    no_handle_invoked = False
+
+    class NoHandle(Widget, can_focus=True):
+        BINDINGS = [("t", "test('bar')", "Test")]
+
+        def action_test(self, text: str) -> bool:
+            nonlocal no_handle_invoked
+            no_handle_invoked = True
+            raise SkipAction()
+
+    class SkipApp(App):
+        def compose(self) -> ComposeResult:
+            yield Handle(NoHandle())
+
+        def on_mount(self) -> None:
+            self.query_one(NoHandle).focus()
+
+    async with SkipApp().run_test() as pilot:
+        # Check the NoHandle widget has focus
+        assert pilot.app.query_one(NoHandle).has_focus
+        # Press the "t" key
+        await pilot.press("t")
+        # Check the action on the no handle widget was called
+        assert no_handle_invoked
+        # Check the return value, confirming that the action on Handle was called
+        assert pilot.app.return_value == "foo"

tests/test_reactive.py

@@ -88,8 +88,6 @@ async def test_watch_async_init_true():
     assert app.watcher_new_value == OLD_VALUE  # The value wasn't changed
 
 
-@pytest.mark.xfail(
-    reason="Reactive watcher is incorrectly always called the first time it is set, even if value is same [issue#1230]")
 async def test_watch_init_false_always_update_false():
     class WatcherInitFalse(App):
         count = reactive(0, init=False)
@@ -102,6 +100,10 @@ async def test_watch_init_false_always_update_false():
     async with app.run_test():
         app.count = 0  # Value hasn't changed, and always_update=False, so watch_count shouldn't run
         assert app.watcher_call_count == 0
+        app.count = 0
+        assert app.watcher_call_count == 0
+        app.count = 1
+        assert app.watcher_call_count == 1
 
 
 async def test_watch_init_true():