Merge pull request #1610 from Textualize/fix-1607

Fix #1607 to allow programmatic style changes

CHANGELOG.md

@@ -26,6 +26,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 ### Fixed
 
 - Fixed stuck screen  https://github.com/Textualize/textual/issues/1632
+- Fixed programmatic style changes not refreshing children layouts when parent widget did not change size https://github.com/Textualize/textual/issues/1607
 - Fixed relative units in `grid-rows` and `grid-columns` being computed with respect to the wrong dimension https://github.com/Textualize/textual/issues/1406
 - Fixed bug with animations that were triggered back to back, where the second one wouldn't start https://github.com/Textualize/textual/issues/1372
 - Fixed bug with animations that were scheduled where all but the first would be skipped https://github.com/Textualize/textual/issues/1372

src/textual/css/_style_properties.py

@@ -60,9 +60,23 @@ PropertySetType = TypeVar("PropertySetType")
 
 
 class GenericProperty(Generic[PropertyGetType, PropertySetType]):
-    def __init__(self, default: PropertyGetType, layout: bool = False) -> None:
+    """Descriptor that abstracts away common machinery for other style descriptors.
+
+    Args:
+        default: The default value (or a factory thereof) of the property.
+        layout: Whether to refresh the node layout on value change.
+        refresh_children: Whether to refresh the node children on value change.
+    """
+
+    def __init__(
+        self,
+        default: PropertyGetType,
+        layout: bool = False,
+        refresh_children: bool = False,
+    ) -> None:
         self.default = default
         self.layout = layout
+        self.refresh_children = refresh_children
 
     def validate_value(self, value: object) -> PropertyGetType:
         """Validate the setter value.
@@ -88,11 +102,11 @@ class GenericProperty(Generic[PropertyGetType, PropertySetType]):
         _rich_traceback_omit = True
         if value is None:
             obj.clear_rule(self.name)
-            obj.refresh(layout=self.layout)
+            obj.refresh(layout=self.layout, children=self.refresh_children)
             return
         new_value = self.validate_value(value)
         if obj.set_rule(self.name, new_value):
-            obj.refresh(layout=self.layout)
+            obj.refresh(layout=self.layout, children=self.refresh_children)
 
 
 class IntegerProperty(GenericProperty[int, int]):
@@ -202,8 +216,16 @@ class ScalarProperty:
 
 
 class ScalarListProperty:
-    def __init__(self, percent_unit: Unit) -> None:
+    """Descriptor for lists of scalars.
+
+    Args:
+        percent_unit: The dimension to which percentage scalars will be relative to.
+        refresh_children: Whether to refresh the node children on value change.
+    """
+
+    def __init__(self, percent_unit: Unit, refresh_children: bool = False) -> None:
         self.percent_unit = percent_unit
+        self.refresh_children = refresh_children
 
     def __set_name__(self, owner: Styles, name: str) -> None:
         self.name = name
@@ -218,7 +240,7 @@ class ScalarListProperty:
     ) -> None:
         if value is None:
             obj.clear_rule(self.name)
-            obj.refresh(layout=True)
+            obj.refresh(layout=True, children=self.refresh_children)
             return
         parse_values: Iterable[str | float]
         if isinstance(value, str):
@@ -237,7 +259,7 @@ class ScalarListProperty:
                     else parse_value
                 )
         if obj.set_rule(self.name, tuple(scalars)):
-            obj.refresh(layout=True)
+            obj.refresh(layout=True, children=self.refresh_children)
 
 
 class BoxProperty:
@@ -682,12 +704,25 @@ class OffsetProperty:
 class StringEnumProperty:
     """Descriptor for getting and setting string properties and ensuring that the set
     value belongs in the set of valid values.
+
+    Args:
+        valid_values: The set of valid values that the descriptor can take.
+        default: The default value (or a factory thereof) of the property.
+        layout: Whether to refresh the node layout on value change.
+        refresh_children: Whether to refresh the node children on value change.
     """
 
-    def __init__(self, valid_values: set[str], default: str, layout=False) -> None:
+    def __init__(
+        self,
+        valid_values: set[str],
+        default: str,
+        layout: bool = False,
+        refresh_children: bool = False,
+    ) -> None:
         self._valid_values = valid_values
         self._default = default
         self._layout = layout
+        self._refresh_children = refresh_children
 
     def __set_name__(self, owner: StylesBase, name: str) -> None:
         self.name = name
@@ -721,7 +756,7 @@ class StringEnumProperty:
         if value is None:
             if obj.clear_rule(self.name):
                 self._before_refresh(obj, value)
-                obj.refresh(layout=self._layout)
+                obj.refresh(layout=self._layout, children=self._refresh_children)
         else:
             if value not in self._valid_values:
                 raise StyleValueError(
@@ -734,7 +769,7 @@ class StringEnumProperty:
                 )
             if obj.set_rule(self.name, value):
                 self._before_refresh(obj, value)
-                obj.refresh(layout=self._layout)
+                obj.refresh(layout=self._layout, children=self._refresh_children)
 
 
 class OverflowProperty(StringEnumProperty):

src/textual/css/styles.py

@@ -265,26 +265,36 @@ class StylesBase(ABC):
     scrollbar_background_hover = ColorProperty("#444444")
     scrollbar_background_active = ColorProperty("black")
 
-    scrollbar_gutter = StringEnumProperty(VALID_SCROLLBAR_GUTTER, "auto")
+    scrollbar_gutter = StringEnumProperty(
+        VALID_SCROLLBAR_GUTTER, "auto", layout=True, refresh_children=True
+    )
 
     scrollbar_size_vertical = IntegerProperty(default=1, layout=True)
     scrollbar_size_horizontal = IntegerProperty(default=1, layout=True)
 
-    align_horizontal = StringEnumProperty(VALID_ALIGN_HORIZONTAL, "left")
-    align_vertical = StringEnumProperty(VALID_ALIGN_VERTICAL, "top")
+    align_horizontal = StringEnumProperty(
+        VALID_ALIGN_HORIZONTAL, "left", layout=True, refresh_children=True
+    )
+    align_vertical = StringEnumProperty(
+        VALID_ALIGN_VERTICAL, "top", layout=True, refresh_children=True
+    )
     align = AlignProperty()
 
     content_align_horizontal = StringEnumProperty(VALID_ALIGN_HORIZONTAL, "left")
     content_align_vertical = StringEnumProperty(VALID_ALIGN_VERTICAL, "top")
     content_align = AlignProperty()
 
-    grid_rows = ScalarListProperty(percent_unit=Unit.HEIGHT)
-    grid_columns = ScalarListProperty(percent_unit=Unit.WIDTH)
+    grid_rows = ScalarListProperty(percent_unit=Unit.HEIGHT, refresh_children=True)
+    grid_columns = ScalarListProperty(percent_unit=Unit.WIDTH, refresh_children=True)
 
-    grid_size_columns = IntegerProperty(default=1, layout=True)
-    grid_size_rows = IntegerProperty(default=0, layout=True)
-    grid_gutter_horizontal = IntegerProperty(default=0, layout=True)
-    grid_gutter_vertical = IntegerProperty(default=0, layout=True)
+    grid_size_columns = IntegerProperty(default=1, layout=True, refresh_children=True)
+    grid_size_rows = IntegerProperty(default=0, layout=True, refresh_children=True)
+    grid_gutter_horizontal = IntegerProperty(
+        default=0, layout=True, refresh_children=True
+    )
+    grid_gutter_vertical = IntegerProperty(
+        default=0, layout=True, refresh_children=True
+    )
 
     row_span = IntegerProperty(default=1, layout=True)
     column_span = IntegerProperty(default=1, layout=True)

tests/css/test_programmatic_style_changes.py

@@ -0,0 +1,95 @@
+import pytest
+
+from textual.app import App
+from textual.containers import Grid
+from textual.screen import Screen
+from textual.widgets import Label
+
+
+@pytest.mark.parametrize(
+    "style, value",
+    [
+        ("grid_size_rows", 3),
+        ("grid_size_columns", 3),
+        ("grid_gutter_vertical", 4),
+        ("grid_gutter_horizontal", 4),
+        ("grid_rows", "1fr 3fr"),
+        ("grid_columns", "1fr 3fr"),
+    ],
+)
+async def test_programmatic_style_change_updates_children(style: str, value: object):
+    """Regression test for #1607 https://github.com/Textualize/textual/issues/1607
+
+    Some programmatic style changes to a widget were not updating the layout of the
+    children widgets, which seemed to be happening when the style change did not affect
+    the size of the widget but did affect the layout of the children.
+
+    This test, in particular, checks the attributes that _should_ affect the size of the
+    children widgets.
+    """
+
+    class MyApp(App[None]):
+        CSS = """
+        Grid { grid-size: 2 2; }
+        Label { width: 100%; height: 100%; }
+        """
+
+        def compose(self):
+            yield Grid(
+                Label("one"),
+                Label("two"),
+                Label("three"),
+                Label("four"),
+            )
+
+    app = MyApp()
+
+    async with app.run_test() as pilot:
+        sizes = [(lbl.size.width, lbl.size.height) for lbl in app.screen.query(Label)]
+
+        setattr(app.query_one(Grid).styles, style, value)
+        await pilot.pause()
+
+        assert sizes != [
+            (lbl.size.width, lbl.size.height) for lbl in app.screen.query(Label)
+        ]
+
+
+@pytest.mark.parametrize(
+    "style, value",
+    [
+        ("align_horizontal", "right"),
+        ("align_vertical", "bottom"),
+        ("align", ("right", "bottom")),
+    ],
+)
+async def test_programmatic_align_change_updates_children_position(
+    style: str, value: str
+):
+    """Regression test for #1607 for the align(_xxx) styles.
+
+    See https://github.com/Textualize/textual/issues/1607.
+    """
+
+    class MyApp(App[None]):
+        CSS = "Grid { grid-size: 2 2; }"
+
+        def compose(self):
+            yield Grid(
+                Label("one"),
+                Label("two"),
+                Label("three"),
+                Label("four"),
+            )
+
+    app = MyApp()
+
+    async with app.run_test() as pilot:
+        offsets = [(lbl.region.x, lbl.region.y) for lbl in app.screen.query(Label)]
+
+        setattr(app.query_one(Grid).styles, style, value)
+        await pilot.pause()
+
+        assert offsets != [
+            (lbl.region.x, lbl.region.y) for lbl in app.screen.query(Label)
+        ]

tests/snapshot_tests/snapshot_apps/programmatic_scrollbar_gutter_change.py

@@ -0,0 +1,29 @@
+from textual.app import App
+from textual.containers import Grid
+from textual.widgets import Label
+
+
+class ProgrammaticScrollbarGutterChange(App[None]):
+    CSS = """
+    Grid { grid-size: 2 2; scrollbar-size: 5 5; }
+    Label { width: 100%; height: 100%; background: red; }
+    """
+
+    def compose(self):
+        yield Grid(
+            Label("one"),
+            Label("two"),
+            Label("three"),
+            Label("four"),
+        )
+
+    def on_key(self, event):
+        if event.key == "s":
+            self.query_one(Grid).styles.scrollbar_gutter = "stable"
+
+
+app = ProgrammaticScrollbarGutterChange()
+
+
+if __name__ == "__main__":
+    app().run()

tests/snapshot_tests/test_snapshots.py

@@ -179,6 +179,16 @@ def test_nested_auto_heights(snap_compare):
     assert snap_compare("snapshot_apps/nested_auto_heights.py", press=["1", "2", "_"])
 
 
+def test_programmatic_scrollbar_gutter_change(snap_compare):
+    """Regression test for #1607 https://github.com/Textualize/textual/issues/1607
+
+    See also tests/css/test_programmatic_style_changes.py for other related regression tests.
+    """
+    assert snap_compare(
+        "snapshot_apps/programmatic_scrollbar_gutter_change.py", press=["s"]
+    )
+
+
 # --- Other ---