Merge pull request #5253 from Textualize/fix-scroll-placements

Fix scroll placements

Author: willmcgugan

CHANGELOG.md

@@ -5,12 +5,15 @@ 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/).
 
-## [Unreleased]
+
+## [0.86.2] - 2024-11-18
 
 ### Fixed
 
+- Fixed visibility glitch for widgets with an offset https://github.com/Textualize/textual/pull/5253
 - Fixed theme variables being unavailable in code until refresh_css was called https://github.com/Textualize/textual/pull/5254
 
+
 ## [0.86.1] - 2024-11-16
 
 ### Fixed
@@ -2549,6 +2552,9 @@ https://textual.textualize.io/blog/2022/11/08/version-040/#version-040
 - New handler system for messages that doesn't require inheritance
 - Improved traceback handling
 
+
+[0.86.2]: https://github.com/Textualize/textual/compare/v0.86.1...v0.86.2
+[0.86.1]: https://github.com/Textualize/textual/compare/v0.86.0...v0.86.1
 [0.86.0]: https://github.com/Textualize/textual/compare/v0.85.2...v0.86.0
 [0.85.2]: https://github.com/Textualize/textual/compare/v0.85.1...v0.85.2
 [0.85.1]: https://github.com/Textualize/textual/compare/v0.85.0...v0.85.1

pyproject.toml

@@ -1,6 +1,6 @@
 [tool.poetry]
 name = "textual"
-version = "0.86.1"
+version = "0.86.2"
 homepage = "https://github.com/Textualize/textual"
 repository = "https://github.com/Textualize/textual"
 documentation = "https://textual.textualize.io/"

src/textual/_arrange.py

@@ -6,7 +6,7 @@
 from typing import TYPE_CHECKING, Iterable, Mapping, Sequence
 
 from textual._partition import partition
-from textual.geometry import Region, Size, Spacing
+from textual.geometry import NULL_OFFSET, NULL_SPACING, Region, Size, Spacing
 from textual.layout import DockArrangeResult, WidgetPlacement
 
 if TYPE_CHECKING:
@@ -133,7 +133,8 @@ def _arrange_dock_widgets(
     region_offset = region.offset
     size = region.size
     width, height = size
-    null_spacing = Spacing()
+    null_spacing = NULL_SPACING
+    null_offset = NULL_OFFSET
 
     top = right = bottom = left = 0
 
@@ -173,6 +174,7 @@ def _arrange_dock_widgets(
         append_placement(
             _WidgetPlacement(
                 dock_region.translate(region_offset),
+                null_offset,
                 null_spacing,
                 dock_widget,
                 top_z,
@@ -202,7 +204,8 @@ def _arrange_split_widgets(
     placements: list[WidgetPlacement] = []
     append_placement = placements.append
     view_region = size.region
-    null_spacing = Spacing()
+    null_spacing = NULL_SPACING
+    null_offset = NULL_OFFSET
 
     for split_widget in split_widgets:
         split = split_widget.styles.split
@@ -226,7 +229,9 @@ def _arrange_split_widgets(
             raise AssertionError("invalid value for split edge")  # pragma: no-cover
 
         append_placement(
-            _WidgetPlacement(split_region, null_spacing, split_widget, 1, True)
+            _WidgetPlacement(
+                split_region, null_offset, null_spacing, split_widget, 1, True
+            )
         )
 
     return placements, view_region

src/textual/_compositor.py

@@ -33,7 +33,7 @@
 from textual._cells import cell_len
 from textual._context import visible_screen_stack
 from textual._loop import loop_last
-from textual.geometry import NULL_OFFSET, NULL_SPACING, Offset, Region, Size, Spacing
+from textual.geometry import NULL_SPACING, Offset, Region, Size, Spacing
 from textual.map_geometry import MapGeometry
 from textual.strip import Strip, StripRenderable
 
@@ -535,8 +535,6 @@ def _arrange_root(
             Compositor map and set of widgets.
         """
 
-        ORIGIN = NULL_OFFSET
-
         map: CompositorMap = {}
         widgets: set[Widget] = set()
         add_new_widget = widgets.add
@@ -580,15 +578,9 @@ def add_widget(
                 add_new_widget(widget)
             else:
                 add_new_invisible_widget(widget)
-            styles_offset = styles.offset
-            layout_offset = (
-                styles_offset.resolve(region.size, clip.size)
-                if styles_offset
-                else ORIGIN
-            )
 
             # Container region is minus border
-            container_region = region.shrink(styles.gutter).translate(layout_offset)
+            container_region = region.shrink(styles.gutter)
             container_size = container_region.size
 
             # Widgets with scrollbars (containers or scroll view) require additional processing
@@ -643,15 +635,25 @@ def add_widget(
                         )
 
                     # Add all the widgets
-                    for sub_region, _, sub_widget, z, fixed, overlay in reversed(
-                        placements
-                    ):
+                    for (
+                        sub_region,
+                        sub_region_offset,
+                        _,
+                        sub_widget,
+                        z,
+                        fixed,
+                        overlay,
+                    ) in reversed(placements):
                         layer_index = get_layer_index(sub_widget.layer, 0)
                         # Combine regions with children to calculate the "virtual size"
                         if fixed:
-                            widget_region = sub_region + placement_offset
+                            widget_region = (
+                                sub_region + sub_region_offset + placement_offset
+                            )
                         else:
-                            widget_region = sub_region + placement_scroll_offset
+                            widget_region = (
+                                sub_region + sub_region_offset + placement_scroll_offset
+                            )
 
                         widget_order = order + ((layer_index, z, layer_order),)
 
@@ -699,7 +701,7 @@ def add_widget(
                             )
 
                     map[widget] = _MapGeometry(
-                        region + layout_offset,
+                        region,
                         order,
                         clip,
                         total_region.size,
@@ -711,27 +713,23 @@ def add_widget(
             elif visible:
                 # Add the widget to the map
 
-                widget_region = region + layout_offset
-
                 if widget.absolute_offset is not None:
                     margin = styles.margin
-                    widget_region = widget_region.at_offset(
-                        widget.absolute_offset + margin.top_left
-                    )
-                    widget_region = widget_region.translate(
-                        styles.offset.resolve(widget_region.grow(margin).size, size)
+                    region = region.at_offset(widget.absolute_offset + margin.top_left)
+                    region = region.translate(
+                        styles.offset.resolve(region.grow(margin).size, size)
                     )
                 has_rule = styles.has_rule
                 if has_rule("constrain_x") or has_rule("constrain_y"):
-                    widget_region = widget_region.constrain(
+                    region = region.constrain(
                         styles.constrain_x,
                         styles.constrain_y,
                         styles.margin,
                         size.region,
                     )
 
                 map[widget._render_widget] = _MapGeometry(
-                    widget_region,
+                    region,
                     order,
                     clip,
                     region.size,

src/textual/_spatial_map.py

@@ -6,7 +6,7 @@
 
 from typing_extensions import TypeAlias
 
-from textual.geometry import Region
+from textual.geometry import Offset, Region
 
 ValueType = TypeVar("ValueType")
 GridCoordinate: TypeAlias = "tuple[int, int]"
@@ -57,27 +57,27 @@ def _region_to_grid_coordinates(self, region: Region) -> Iterable[GridCoordinate
         )
 
     def insert(
-        self, regions_and_values: Iterable[tuple[Region, bool, bool, ValueType]]
+        self, regions_and_values: Iterable[tuple[Region, Offset, bool, bool, ValueType]]
     ) -> None:
         """Insert values into the Spatial map.
 
         Values are associated with their region in Euclidean space, and a boolean that
         indicates fixed regions. Fixed regions don't scroll and are always visible.
 
         Args:
-            regions_and_values: An iterable of (REGION, FIXED, OVERLAY, VALUE).
+            regions_and_values: An iterable of (REGION, OFFSET, FIXED, OVERLAY, VALUE).
         """
         append_fixed = self._fixed.append
         get_grid_list = self._map.__getitem__
         _region_to_grid = self._region_to_grid_coordinates
         total_region = self.total_region
-        for region, fixed, overlay, value in regions_and_values:
+        for region, offset, fixed, overlay, value in regions_and_values:
             if fixed:
                 append_fixed(value)
             else:
                 if not overlay:
                     total_region = total_region.union(region)
-                for grid in _region_to_grid(region):
+                for grid in _region_to_grid(region + offset):
                     get_grid_list(grid).append(value)
         self.total_region = total_region
 

src/textual/layout.py

@@ -39,6 +39,7 @@ def spatial_map(self) -> SpatialMap[WidgetPlacement]:
             self._spatial_map.insert(
                 (
                     placement.region.grow(placement.margin),
+                    placement.offset,
                     placement.fixed,
                     placement.overlay,
                     placement,
@@ -75,7 +76,7 @@ def get_visible_placements(self, region: Region) -> list[WidgetPlacement]:
         culled_placements = [
             placement
             for placement in visible_placements
-            if placement.fixed or overlaps(placement.region)
+            if placement.fixed or overlaps(placement.region + placement.offset)
         ]
         return culled_placements
 
@@ -84,6 +85,7 @@ class WidgetPlacement(NamedTuple):
     """The position, size, and relative order of a widget within its parent."""
 
     region: Region
+    offset: Offset
     margin: Spacing
     widget: Widget
     order: int = 0
@@ -92,7 +94,7 @@ class WidgetPlacement(NamedTuple):
 
     @classmethod
     def translate(
-        cls, placements: list[WidgetPlacement], offset: Offset
+        cls, placements: list[WidgetPlacement], translate_offset: Offset
     ) -> list[WidgetPlacement]:
         """Move all placements by a given offset.
 
@@ -103,10 +105,18 @@ def translate(
         Returns:
             Placements with adjusted region, or same instance if offset is null.
         """
-        if offset:
+        if translate_offset:
             return [
-                cls(region + offset, margin, layout_widget, order, fixed, overlay)
-                for region, margin, layout_widget, order, fixed, overlay in placements
+                cls(
+                    region + translate_offset,
+                    offset,
+                    margin,
+                    layout_widget,
+                    order,
+                    fixed,
+                    overlay,
+                )
+                for region, offset, margin, layout_widget, order, fixed, overlay in placements
             ]
         return placements
 

src/textual/layouts/grid.py

@@ -5,7 +5,7 @@
 
 from textual._resolve import resolve
 from textual.css.scalar import Scalar
-from textual.geometry import Region, Size, Spacing
+from textual.geometry import NULL_OFFSET, Region, Size, Spacing
 from textual.layout import ArrangeResult, Layout, WidgetPlacement
 
 if TYPE_CHECKING:
@@ -258,6 +258,7 @@ def apply_height_limits(widget: Widget, height: int) -> int:
         rows = resolve(row_scalars, size.height, gutter_horizontal, size, viewport)
 
         placements: list[WidgetPlacement] = []
+        _WidgetPlacement = WidgetPlacement
         add_placement = placements.append
         widgets: list[Widget] = []
         add_widget = widgets.append
@@ -272,26 +273,34 @@ def apply_height_limits(widget: Widget, height: int) -> int:
             x2, cell_width = columns[min(max_column, column + column_span)]
             y2, cell_height = rows[min(max_row, row + row_span)]
             cell_size = Size(cell_width + x2 - x, cell_height + y2 - y)
-            width, height, margin = widget._get_box_model(
+            box_width, box_height, margin = widget._get_box_model(
                 cell_size,
                 viewport,
                 Fraction(cell_size.width),
                 Fraction(cell_size.height),
             )
             if self.stretch_height and len(children) > 1:
-                height = (
-                    height
-                    if (height > cell_size.height)
-                    else Fraction(cell_size.height)
-                )
+                if box_height <= cell_size.height:
+                    box_height = Fraction(cell_size.height)
+
             region = (
-                Region(x, y, int(width + margin.width), int(height + margin.height))
+                Region(
+                    x, y, int(box_width + margin.width), int(box_height + margin.height)
+                )
                 .crop_size(cell_size)
                 .shrink(margin)
             )
+
+            placement_offset = (
+                styles.offset.resolve(cell_size, viewport)
+                if styles.has_rule("offset")
+                else NULL_OFFSET
+            )
+
             add_placement(
-                WidgetPlacement(
+                _WidgetPlacement(
                     region + offset,
+                    placement_offset,
                     (
                         margin
                         if gutter_spacing is None