Avoid docks when scrolling (#2571)

* handle docked layers

* handle scroll better

* snapshot update

* remove commented out code

* superflous

* dock gutter

* snapshit

* snapshit test

* changelog

* mistake

* docstrings

* changelog

* whitespace

* missing punctuation

* ofx docstring

* Apply suggestions from code review

Co-authored-by: Rodrigo Girão Serrão <5621605+rodrigogiraoserrao@users.noreply.github.com>

---------

Co-authored-by: Rodrigo Girão Serrão <5621605+rodrigogiraoserrao@users.noreply.github.com>

CHANGELOG.md

@@ -24,10 +24,12 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 - Fixed `TreeNode.toggle` and `TreeNode.toggle_all` not posting a `Tree.NodeExpanded` or `Tree.NodeCollapsed` message https://github.com/Textualize/textual/issues/2535
 - `footer--description` component class was being ignored https://github.com/Textualize/textual/issues/2544
 - Pasting empty selection in `Input` would raise an exception https://github.com/Textualize/textual/issues/2563
+- Fix issue with scrolling and docks https://github.com/Textualize/textual/issues/2525
 
 ### Added
 
 - Class variable `AUTO_FOCUS` to screens https://github.com/Textualize/textual/issues/2457
+- Added `NULL_SPACING` and `NULL_REGION` to geometry.py
 
 ## [0.24.1] - 2023-05-08
 

src/textual/_compositor.py

@@ -33,7 +33,7 @@ from . import errors
 from ._cells import cell_len
 from ._context import visible_screen_stack
 from ._loop import loop_last
-from .geometry import NULL_OFFSET, Offset, Region, Size
+from .geometry import NULL_OFFSET, NULL_SPACING, Offset, Region, Size, Spacing
 from .strip import Strip, StripRenderable
 
 if TYPE_CHECKING:
@@ -71,6 +71,8 @@ class MapGeometry(NamedTuple):
     """The container [size][textual.geometry.Size] (area not occupied by scrollbars)."""
     virtual_region: Region
     """The [region][textual.geometry.Region] relative to the container (but not necessarily visible)."""
+    dock_gutter: Spacing
+    """Space from the container reserved by docked widgets."""
 
     @property
     def visible_region(self) -> Region:
@@ -484,7 +486,7 @@ class Compositor:
             # Widgets and regions in render order
             visible_widgets = [
                 (order, widget, region, clip)
-                for widget, (region, order, clip, _, _, _) in map.items()
+                for widget, (region, order, clip, _, _, _, _) in map.items()
                 if in_screen(region) and overlaps(clip, region)
             ]
             visible_widgets.sort(key=itemgetter(0), reverse=True)
@@ -522,6 +524,7 @@ class Compositor:
             layer_order: int,
             clip: Region,
             visible: bool,
+            dock_gutter: Spacing,
             _MapGeometry: type[MapGeometry] = MapGeometry,
         ) -> None:
             """Called recursively to place a widget and its children in the map.
@@ -591,10 +594,8 @@ class Compositor:
 
                     get_layer_index = layers_to_index.get
 
-                    scroll_spacing = arrange_result.scroll_spacing
-
                     # Add all the widgets
-                    for sub_region, margin, sub_widget, z, fixed, overlay in reversed(
+                    for sub_region, _, sub_widget, z, fixed, overlay in reversed(
                         placements
                     ):
                         layer_index = get_layer_index(sub_widget.layer, 0)
@@ -602,11 +603,6 @@ class Compositor:
                         if fixed:
                             widget_region = sub_region + placement_offset
                         else:
-                            total_region = total_region.union(
-                                sub_region.grow(
-                                    margin if layer_index else margin + scroll_spacing
-                                )
-                            )
                             widget_region = sub_region + placement_scroll_offset
 
                         widget_order = order + ((layer_index, z, layer_order),)
@@ -629,6 +625,7 @@ class Compositor:
                             layer_order,
                             no_clip if overlay else sub_clip,
                             visible,
+                            arrange_result.scroll_spacing,
                         )
 
                         layer_order -= 1
@@ -646,6 +643,7 @@ class Compositor:
                                 container_size,
                                 container_size,
                                 chrome_region,
+                                dock_gutter,
                             )
 
                     map[widget] = _MapGeometry(
@@ -655,6 +653,7 @@ class Compositor:
                         total_region.size,
                         container_size,
                         virtual_region,
+                        dock_gutter,
                     )
 
             elif visible:
@@ -666,6 +665,7 @@ class Compositor:
                     region.size,
                     container_size,
                     virtual_region,
+                    dock_gutter,
                 )
 
         # Add top level (root) widget
@@ -677,6 +677,7 @@ class Compositor:
             layer_order,
             size.region,
             True,
+            NULL_SPACING,
         )
         return map, widgets
 

src/textual/_layout.py

@@ -51,7 +51,8 @@ class DockArrangeResult:
         Returns:
             A Region.
         """
-        return self.spatial_map.total_region
+        _top, right, bottom, _left = self.scroll_spacing
+        return self.spatial_map.total_region.grow((0, right, bottom, 0))
 
     def get_visible_placements(self, region: Region) -> list[WidgetPlacement]:
         """Get the placements visible within the given region.

src/textual/_spatial_map.py

@@ -72,11 +72,11 @@ class SpatialMap(Generic[ValueType]):
         _region_to_grid = self._region_to_grid_coordinates
         total_region = self.total_region
         for region, fixed, overlay, value in regions_and_values:
-            if not overlay:
-                total_region = total_region.union(region)
             if fixed:
                 append_fixed(value)
             else:
+                if not overlay:
+                    total_region = total_region.union(region)
                 for grid in _region_to_grid(region):
                     get_grid_list(grid).append(value)
         self.total_region = total_region

src/textual/geometry.py

@@ -907,7 +907,7 @@ class Region(NamedTuple):
 
 
 class Spacing(NamedTuple):
-    """The spacing around a renderable, such as padding and border
+    """The spacing around a renderable, such as padding and border.
 
     Spacing is defined by four integers for the space at the top, right, bottom, and left of a region.
 
@@ -940,7 +940,7 @@ class Spacing(NamedTuple):
     top: int = 0
     """Space from the top of a region."""
     right: int = 0
-    """Space from the left of a region."""
+    """Space from the right of a region."""
     bottom: int = 0
     """Space from the bottom of a region."""
     left: int = 0
@@ -1095,3 +1095,9 @@ class Spacing(NamedTuple):
 
 NULL_OFFSET: Final = Offset(0, 0)
 """An [offset][textual.geometry.Offset] constant for (0, 0)."""
+
+NULL_REGION: Final = Region(0, 0, 0, 0)
+"""A [Region][textual.geometry.Region] constant for a null region (at the origin, with both width and height set to zero)."""
+
+NULL_SPACING: Final = Spacing(0, 0, 0, 0)
+"""A [Spacing][textual.geometry.Spacing] constant for no space."""

src/textual/screen.py

@@ -584,6 +584,7 @@ class Screen(Generic[ScreenResultType], Widget):
                         virtual_size,
                         container_size,
                         _,
+                        _,
                     ) in layers:
                         if widget in exposed_widgets:
                             if widget._size_updated(
@@ -614,6 +615,7 @@ class Screen(Generic[ScreenResultType], Widget):
                     virtual_size,
                     container_size,
                     _,
+                    _,
                 ) in layers:
                     widget._size_updated(region.size, virtual_size, container_size)
                     if widget in send_resize:

src/textual/widget.py

@@ -57,7 +57,7 @@ from .box_model import BoxModel
 from .css.query import NoMatches, WrongType
 from .css.scalar import ScalarOffset
 from .dom import DOMNode, NoScreen
-from .geometry import Offset, Region, Size, Spacing, clamp
+from .geometry import NULL_REGION, NULL_SPACING, Offset, Region, Size, Spacing, clamp
 from .layouts.vertical import VerticalLayout
 from .message import Message
 from .messages import CallbackType
@@ -1375,10 +1375,20 @@ class Widget(DOMNode):
         """
         try:
             return self.screen.find_widget(self).region
-        except NoScreen:
+        except (NoScreen, errors.NoWidget):
-            return Region()
+            return NULL_REGION
-        except errors.NoWidget:
+
-            return Region()
+    @property
+    def dock_gutter(self) -> Spacing:
+        """Space allocated to docks in the parent.
+
+        Returns:
+            Space to be subtracted from scrollable area.
+        """
+        try:
+            return self.screen.find_widget(self).dock_gutter
+        except (NoScreen, errors.NoWidget):
+            return NULL_SPACING
 
     @property
     def container_viewport(self) -> Region:
@@ -2263,7 +2273,7 @@ class Widget(DOMNode):
             else:
                 scroll_offset = container.scroll_to_region(
                     region,
-                    spacing=widget.parent.gutter,
+                    spacing=widget.parent.gutter + widget.dock_gutter,
                     animate=animate,
                     speed=speed,
                     duration=duration,

tests/snapshot_tests/snapshot_apps/dock_scroll2.py

@@ -0,0 +1,33 @@
+from textual.app import App
+from textual.widgets import Header, Label, Footer
+
+
+# Same as dock_scroll.py but with 2 labels
+class TestApp(App):
+    BINDINGS = [("ctrl+q", "app.quit", "Quit")]
+    CSS = """
+    
+    Label {
+        border: solid red;
+    }
+    Footer {
+        height: 4;
+    }
+    """
+
+    def compose(self):
+        text = (
+            "this is a sample sentence and here are some words".replace(" ", "\n") * 2
+        )
+        yield Header()
+        yield Label(text)
+        yield Label(text)
+        yield Footer()
+
+    def on_mount(self):
+        self.dark = False
+
+
+if __name__ == "__main__":
+    app = TestApp()
+    app.run()

tests/snapshot_tests/snapshot_apps/dock_scroll_off_by_one.py

@@ -0,0 +1,17 @@
+from textual.app import App, ComposeResult
+from textual.widgets import Checkbox, Footer
+
+
+class ScrollOffByOne(App):
+    def compose(self) -> ComposeResult:
+        for number in range(1, 100):
+            yield Checkbox(str(number))
+        yield Footer()
+
+    def on_mount(self) -> None:
+        self.query_one("Screen").scroll_end()
+
+
+app = ScrollOffByOne()
+if __name__ == "__main__":
+    app.run()

tests/snapshot_tests/snapshot_apps/horizontal_auto_width.css

@@ -19,6 +19,6 @@
 
 #horizontal {
     width: auto;
-    height: auto;
+    height: 4;
     background: darkslateblue;
-}
+}

tests/snapshot_tests/snapshot_apps/scroll_to.py

@@ -0,0 +1,19 @@
+from textual.app import App, ComposeResult
+from textual.widgets import Checkbox, Footer
+
+
+class ScrollOffByOne(App):
+    """Scroll to item 50."""
+
+    def compose(self) -> ComposeResult:
+        for number in range(1, 100):
+            yield Checkbox(str(number), id=f"number-{number}")
+        yield Footer()
+
+    def on_ready(self) -> None:
+        self.query_one("#number-50").scroll_visible()
+
+
+app = ScrollOffByOne()
+if __name__ == "__main__":
+    app.run()

tests/snapshot_tests/test_snapshots.py

@@ -203,9 +203,11 @@ def test_option_list(snap_compare):
     assert snap_compare(WIDGET_EXAMPLES_DIR / "option_list_options.py")
     assert snap_compare(WIDGET_EXAMPLES_DIR / "option_list_tables.py")
 
+
 def test_option_list_build(snap_compare):
     assert snap_compare(SNAPSHOT_APPS_DIR / "option_list.py")
 
+
 def test_progress_bar_indeterminate(snap_compare):
     assert snap_compare(WIDGET_EXAMPLES_DIR / "progress_bar_isolated_.py", press=["f"])
 
@@ -457,6 +459,23 @@ def test_dock_scroll(snap_compare):
     assert snap_compare(SNAPSHOT_APPS_DIR / "dock_scroll.py", terminal_size=(80, 25))
 
 
+def test_dock_scroll2(snap_compare):
+    # https://github.com/Textualize/textual/issues/2525
+    assert snap_compare(SNAPSHOT_APPS_DIR / "dock_scroll2.py", terminal_size=(80, 25))
+
+
+def test_dock_scroll_off_by_one(snap_compare):
+    # https://github.com/Textualize/textual/issues/2525
+    assert snap_compare(
+        SNAPSHOT_APPS_DIR / "dock_scroll_off_by_one.py", terminal_size=(80, 25)
+    )
+
+
+def test_scroll_to(snap_compare):
+    # https://github.com/Textualize/textual/issues/2525
+    assert snap_compare(SNAPSHOT_APPS_DIR / "scroll_to.py", terminal_size=(80, 25))
+
+
 def test_auto_fr(snap_compare):
     # https://github.com/Textualize/textual/issues/2220
     assert snap_compare(SNAPSHOT_APPS_DIR / "auto_fr.py", terminal_size=(80, 25))