Merge pull request #608 from Textualize/scroll-to

Improvements to scroll_to_widget

poetry.lock

@@ -500,7 +500,7 @@ testing = ["pytest", "pytest-benchmark"]
 
 [[package]]
 name = "pre-commit"
-version = "2.19.0"
+version = "2.20.0"
 description = "A framework for managing and maintaining multi-language pre-commit hooks."
 category = "dev"
 optional = false
@@ -654,7 +654,7 @@ pyyaml = "*"
 
 [[package]]
 name = "rich"
-version = "12.4.4"
+version = "12.5.1"
 description = "Render rich text, tables, progress bars, syntax highlighting, markdown and more to the terminal"
 category = "main"
 optional = false
@@ -780,7 +780,7 @@ dev = ["aiohttp", "click", "msgpack"]
 [metadata]
 lock-version = "1.1"
 python-versions = "^3.7"
-content-hash = "8ce8d66466dad1b984673595ebd0cc7bc0d28c7a672269e9b5620c242d87d9ad"
+content-hash = "2d0f99d7fb563eb0b34cda9542ecf87c35cf5944a67510625969ec7b046b6d03"
 
 [metadata.files]
 aiohttp = [
@@ -1295,10 +1295,7 @@ pluggy = [
     {file = "pluggy-1.0.0-py2.py3-none-any.whl", hash = "sha256:74134bbf457f031a36d68416e1509f34bd5ccc019f0bcc952c7b909d06b37bd3"},
     {file = "pluggy-1.0.0.tar.gz", hash = "sha256:4224373bacce55f955a878bf9cfa763c1e360858e330072059e10bad68531159"},
 ]
-pre-commit = [
+pre-commit = []
-    {file = "pre_commit-2.19.0-py2.py3-none-any.whl", hash = "sha256:10c62741aa5704faea2ad69cb550ca78082efe5697d6f04e5710c3c229afdd10"},
-    {file = "pre_commit-2.19.0.tar.gz", hash = "sha256:4233a1e38621c87d9dda9808c6606d7e7ba0e087cd56d3fe03202a01d2919615"},
-]
 py = [
     {file = "py-1.11.0-py2.py3-none-any.whl", hash = "sha256:607c53218732647dff4acdfcd50cb62615cedf612e72d1724fb1a0cc6405b378"},
     {file = "py-1.11.0.tar.gz", hash = "sha256:51c75c4126074b472f746a24399ad32f6053d1b34b68d2fa41e558e6f4a98719"},
@@ -1375,10 +1372,7 @@ pyyaml-env-tag = [
     {file = "pyyaml_env_tag-0.1-py3-none-any.whl", hash = "sha256:af31106dec8a4d68c60207c1886031cbf839b68aa7abccdb19868200532c2069"},
     {file = "pyyaml_env_tag-0.1.tar.gz", hash = "sha256:70092675bda14fdec33b31ba77e7543de9ddc88f2e5b99160396572d11525bdb"},
 ]
-rich = [
+rich = []
-    {file = "rich-12.4.4-py3-none-any.whl", hash = "sha256:d2bbd99c320a2532ac71ff6a3164867884357da3e3301f0240090c5d2fdac7ec"},
-    {file = "rich-12.4.4.tar.gz", hash = "sha256:4c586de507202505346f3e32d1363eb9ed6932f0c2f63184dea88983ff4971e2"},
-]
 six = [
     {file = "six-1.16.0-py2.py3-none-any.whl", hash = "sha256:8abb2f1d86890a2dfb989f9a77cfcfd3e47c2a354b01111771326f8aa26e0254"},
     {file = "six-1.16.0.tar.gz", hash = "sha256:1e61c37477a1626458e36f7b1d82aa5c9b094fa4802892072e49de9c60c4c926"},

pyproject.toml

@@ -22,7 +22,7 @@ textual = "textual.cli.cli:run"
 
 [tool.poetry.dependencies]
 python = "^3.7"
-rich = "^12.4.3"
+rich = "^12.5.0"
 #rich = {path="../rich", develop=true}
 importlib-metadata = "^4.11.3"
 typing-extensions = { version = "^4.0.0", python = "<3.8" }

sandbox/will/basic.css

@@ -66,8 +66,8 @@ DataTable {
 }
 
 #header {
-  color: $text-primary-background-darken-1;
+  color: $text-secondary-background-darken-1;
-  background: $primary-background-darken-1;
+  background: $secondary-background-darken-1;
   height: 3;
   content-align: center middle;
 }
@@ -109,6 +109,7 @@ Tweet {
 
 .code {  
   height: auto;
+  
 }
 
 

sandbox/will/buttons.css

@@ -0,0 +1,24 @@
+
+Button {
+    margin: 1;
+    width: 100%;
+}
+
+Vertical {
+    height: auto;       
+}
+
+Horizontal {    
+    height: auto;    
+}
+
+Horizontal Button {
+    width: 20;
+    
+    margin: 1 2     ;
+}
+
+#scroll {
+    height: 10; 
+   
+}

sandbox/will/scroll.py

@@ -0,0 +1,46 @@
+from textual import layout, events
+from textual.app import App, ComposeResult
+from textual.widgets import Button
+
+
+class ButtonsApp(App[str]):
+    def compose(self) -> ComposeResult:
+        yield layout.Vertical(
+            Button("default", id="foo"),
+            Button("Where there is a Will"),
+            Button("There is a Way"),
+            Button("There can be only one"),
+            Button.success("success", id="bar"),
+            layout.Horizontal(
+                Button("Where there is a Will"),
+                Button("There is a Way"),
+                Button("There can be only one"),
+                Button.warning("warning", id="baz"),
+                Button("Where there is a Will"),
+                Button("There is a Way"),
+                Button("There can be only one"),
+                id="scroll",
+            ),
+            Button.error("error", id="baz"),
+            Button("Where there is a Will"),
+            Button("There is a Way"),
+            Button("There can be only one"),
+        )
+
+    def handle_pressed(self, event: Button.Pressed) -> None:
+        self.app.bell()
+
+    async def on_key(self, event: events.Key) -> None:
+        await self.dispatch_key(event)
+
+    def key_d(self):
+        self.dark = not self.dark
+
+
+app = ButtonsApp(
+    log_path="textual.log", css_path="buttons.css", watch_css=True, log_verbosity=2
+)
+
+if __name__ == "__main__":
+    result = app.run()
+    print(repr(result))

src/textual/_compositor.py

@@ -54,11 +54,12 @@ class ReflowResult(NamedTuple):
 class MapGeometry(NamedTuple):
     """Defines the absolute location of a Widget."""
 
-    region: Region  # The region occupied by the widget
+    region: Region  # The (screen) region occupied by the widget
     order: tuple[int, ...]  # A tuple of ints defining the painting order
     clip: Region  # A region to clip the widget by (if a Widget is within a container)
     virtual_size: Size  # The virtual size  (scrollable region) of a widget if it is a container
     container_size: Size  # The container size (area not occupied by scrollbars)
+    virtual_region: Region  # The region relative to the container (but not necessarily visible)
 
     @property
     def visible_region(self) -> Region:
@@ -271,8 +272,7 @@ class Compositor:
 
         # Get a map of regions
         self.regions = {
-            widget: (region, clip)
+            widget: (region, clip) for widget, (region, _order, clip, *_) in map.items()
-            for widget, (region, _order, clip, _, _) in map.items()
         }
 
         # Widgets with changed size
@@ -326,6 +326,7 @@ class Compositor:
 
         def add_widget(
             widget: Widget,
+            virtual_region: Region,
             region: Region,
             order: tuple[int, ...],
             clip: Region,
@@ -379,6 +380,7 @@ class Compositor:
                         if sub_widget is not None:
                             add_widget(
                                 sub_widget,
+                                sub_region,
                                 sub_region + placement_offset,
                                 order + (z,),
                                 sub_clip,
@@ -394,6 +396,7 @@ class Compositor:
                         clip,
                         container_size,
                         container_size,
+                        chrome_region,
                     )
 
                 map[widget] = MapGeometry(
@@ -402,16 +405,22 @@ class Compositor:
                     clip,
                     total_region.size,
                     container_size,
+                    virtual_region,
                 )
 
             else:
                 # Add the widget to the map
                 map[widget] = MapGeometry(
-                    region + layout_offset, order, clip, region.size, container_size
+                    region + layout_offset,
+                    order,
+                    clip,
+                    region.size,
+                    container_size,
+                    virtual_region,
                 )
 
         # Add top level (root) widget
-        add_widget(root, size.region, (0,), size.region)
+        add_widget(root, size.region, size.region, (0,), size.region)
         return map, widgets
 
     def __iter__(self) -> Iterator[tuple[Widget, Region, Region, Size, Size]]:
@@ -423,7 +432,7 @@ class Compositor:
         """
         layers = sorted(self.map.items(), key=lambda item: item[1].order, reverse=True)
         intersection = Region.intersection
-        for widget, (region, _order, clip, virtual_size, container_size) in layers:
+        for widget, (region, _order, clip, virtual_size, container_size, *_) in layers:
             yield (
                 widget,
                 intersection(region, clip),
@@ -517,7 +526,7 @@ class Compositor:
         intersection = Region.intersection
         extend = list.extend
 
-        for region, order, clip, _, _ in self.map.values():
+        for region, order, clip, *_ in self.map.values():
             region = intersection(region, clip)
             if region and (region in screen_region):
                 x, y, region_width, region_height = region
@@ -547,13 +556,13 @@ class Compositor:
                 overlaps = crop.overlaps
                 mapped_regions = [
                     (widget, region, order, clip)
-                    for widget, (region, order, clip, _, _) in self.map.items()
+                    for widget, (region, order, clip, *_) in self.map.items()
                     if widget.visible and not widget.is_transparent and overlaps(crop)
                 ]
             else:
                 mapped_regions = [
                     (widget, region, order, clip)
-                    for widget, (region, order, clip, _, _) in self.map.items()
+                    for widget, (region, order, clip, *_) in self.map.items()
                     if widget.visible and not widget.is_transparent
                 ]
 
@@ -594,7 +603,6 @@ class Compositor:
         ]
         return segment_lines
 
-    @timer("render")
     def render(self, full: bool = False) -> RenderableType | None:
         """Render a layout.
 

src/textual/app.py

@@ -81,7 +81,7 @@ LayoutDefinition = "dict[str, Any]"
 
 
 DEFAULT_COLORS = ColorSystem(
-    primary="#406e8e",
+    primary="#2A4E6E",
     secondary="#ffa62b",
     warning="#ffa62b",
     error="#ba3c5b",
@@ -645,6 +645,7 @@ class App(Generic[ReturnType], DOMNode):
                 # Change focus
                 self.focused = widget
                 # Send focus event
+                self.screen.scroll_to_widget(widget)
                 widget.post_message_no_wait(events.Focus(self))
                 widget.emit_no_wait(events.DescendantFocus(self))
 
@@ -926,7 +927,6 @@ class App(Generic[ReturnType], DOMNode):
         stylesheet.update(self.app, animate=animate)
         self.screen._refresh_layout(self.size, full=True)
 
-    @timer("_display")
     def _display(self, renderable: RenderableType | None) -> None:
         """Display a renderable within a sync.
 

src/textual/dom.py

@@ -449,8 +449,10 @@ class DOMNode(MessagePump):
         """
         _append = self.children._append
         for node in nodes:
+            node.set_parent(self)
             _append(node)
         for node_id, node in named_nodes.items():
+            node.set_parent(self)
             _append(node)
             node.id = node_id
 

src/textual/geometry.py

@@ -569,8 +569,27 @@ class Region(NamedTuple):
         )
         return new_region
 
+    def grow(self, margin: tuple[int, int, int, int]) -> Region:
+        """Grow a region by adding spacing.
+
+        Args:
+            margin (Spacing): Defines how many cells to grow the Region by at each edge.
+
+        Returns:
+            Region: New region.
+        """
+
+        top, right, bottom, left = margin
+        x, y, width, height = self
+        return Region(
+            x=x - left,
+            y=y - top,
+            width=max(0, width + left + right),
+            height=max(0, height + top + bottom),
+        )
+
     def shrink(self, margin: tuple[int, int, int, int]) -> Region:
-        """Shrink a region by pushing each edge inwards.
+        """Shrink a region by subtracting spacing.
 
         Args:
             margin (Spacing): Defines how many cells to shrink the Region by at each edge.

src/textual/layouts/horizontal.py

@@ -57,7 +57,9 @@ class HorizontalLayout(Layout):
             )
             next_x = x + content_width
             region = Region(int(x), offset_y, int(next_x - int(x)), int(content_height))
-            max_height = max(max_height, content_height)
+            max_height = max(
+                max_height, content_height + offset_y + box_model.margin.bottom
+            )
             add_placement(WidgetPlacement(region, widget, 0))
             x = next_x + margin
             max_width = x

src/textual/widget.py

@@ -289,10 +289,12 @@ class Widget(DOMNode):
     def watch_scroll_x(self, new_value: float) -> None:
         self.horizontal_scrollbar.position = int(new_value)
         self.refresh(layout=True)
+        self.horizontal_scrollbar.refresh()
 
     def watch_scroll_y(self, new_value: float) -> None:
         self.vertical_scrollbar.position = int(new_value)
         self.refresh(layout=True)
+        self.vertical_scrollbar.refresh()
 
     def validate_scroll_x(self, value: float) -> float:
         return clamp(value, 0, self.max_scroll_x)
@@ -307,7 +309,7 @@ class Widget(DOMNode):
         return clamp(value, 0, self.max_scroll_y)
 
     @property
-    def max_scroll_x(self) -> float:
+    def max_scroll_x(self) -> int:
         """The maximum value of `scroll_x`."""
         return max(
             0,
@@ -317,7 +319,7 @@ class Widget(DOMNode):
         )
 
     @property
-    def max_scroll_y(self) -> float:
+    def max_scroll_y(self) -> int:
         """The maximum value of `scroll_y`."""
         return max(
             0,
@@ -469,6 +471,16 @@ class Widget(DOMNode):
         except errors.NoWidget:
             return Region()
 
+    @property
+    def virtual_region(self) -> Region:
+        """The widget region relative to it's container. Which may not be visible,
+        depending on scroll offset.
+        """
+        try:
+            return self.screen.find_widget(self).virtual_region
+        except errors.NoWidget:
+            return Region()
+
     @property
     def window_region(self) -> Region:
         """The region within the scrollable area that is currently visible.
@@ -688,9 +700,7 @@ class Widget(DOMNode):
         )
 
     def scroll_to_widget(self, widget: Widget, *, animate: bool = True) -> bool:
-        """Starting from `widget`, travel up the DOM to this node, scrolling all containers such that
+        """Scroll scrolling to bring a widget in to view.
-        every widget is visible within its parent container. This will, in the majority of cases,
-        bring the target widget into
 
         Args:
             widget (Widget): A descendant widget.
@@ -700,54 +710,25 @@ class Widget(DOMNode):
             bool: True if any scrolling has occurred in any descendant, otherwise False.
         """
 
-        # TODO: Update this to use scroll_to_region
+        # Grow the region by the margin so to keep the margin in view.
-        scrolls = set()
+        region = widget.virtual_region.grow(widget.styles.margin)
+        scrolled = False
 
-        node = widget.parent
+        while isinstance(widget.parent, Widget) and widget is not self:
-        child = widget
+            container = widget.parent
-        while node:
+            scroll_offset = container.scroll_to_region(region, animate=animate)
-            try:
+            if scroll_offset:
-                widget_region = child.region
+                scrolled = True
-                container_region = node.region
-            except (errors.NoWidget, AttributeError):
-                return False
 
-            if widget_region in container_region:
+            # Adjust the region by the amount we just scrolled it, and convert to
-                # Widget is visible, nothing to do
+            # it's parent's virtual coordinate system.
-                child = node
+            region = (
-                node = node.parent
+                region.translate(-scroll_offset)
-                continue
+                .translate(-widget.scroll_offset)
-
+                .translate(container.virtual_region.offset)
-            # We can either scroll so the widget is at the top of the container, or so that
+            ).intersection(container.virtual_region)
-            # it is at the bottom. We want to pick which has the shortest distance
+            widget = container
-            top_delta = widget_region.offset - container_region.origin
+        return scrolled
-
-            bottom_delta = widget_region.offset - (
-                container_region.origin
-                + Offset(0, container_region.height - widget_region.height)
-            )
-
-            if widget_region.width > container_region.width:
-                delta_x = top_delta.x
-            else:
-                delta_x = min(top_delta.x, bottom_delta.x, key=abs)
-
-            if widget_region.height > container_region.height:
-                delta_y = top_delta.y
-            else:
-                delta_y = min(top_delta.y, bottom_delta.y, key=abs)
-
-            scrolled = node.scroll_relative(
-                delta_x or None, delta_y or None, animate=animate, duration=0.2
-            )
-            scrolls.add(scrolled)
-
-            if node == self:
-                break
-            child = node
-            node = node.parent
-
-        return any(scrolls)
 
     def scroll_to_region(
         self, region: Region, *, spacing: Spacing | None = None, animate: bool = True
@@ -763,13 +744,18 @@ class Widget(DOMNode):
             spacing (Spacing): Space to subtract from the window region.
 
         Returns:
-            bool: True if the window was scrolled.
+            Offset: The distance that was scrolled.
         """
 
         window = self.content_region.at_offset(self.scroll_offset)
         if spacing is not None:
             window = window.shrink(spacing)
-        delta = Region.get_scroll_to_visible(window, region)
+        delta_x, delta_y = Region.get_scroll_to_visible(window, region)
+        scroll_x, scroll_y = self.scroll_offset
+        delta = Offset(
+            clamp(scroll_x + delta_x, 0, self.max_scroll_x) - scroll_x,
+            clamp(scroll_y + delta_y, 0, self.max_scroll_y) - scroll_y,
+        )
         if delta:
             self.scroll_relative(
                 delta.x or None,
@@ -781,7 +767,7 @@ class Widget(DOMNode):
 
     def __init_subclass__(
         cls,
-        can_focus: bool = True,
+        can_focus: bool = False,
         can_focus_children: bool = True,
         inherit_css: bool = True,
     ) -> None:

src/textual/widgets/_button.py

@@ -43,7 +43,7 @@ class Button(Widget, can_focus=True):
     }
 
     Button:focus {
-        text-style: bold underline;        
+        text-style: bold reverse;        
     }
 
     Button:hover {
@@ -183,6 +183,7 @@ class Button(Widget, can_focus=True):
 
     def render(self) -> RenderableType:
         label = self.label.copy()
+        label = Text.assemble(" ", label, " ")
         label.stylize(self.text_style)
         return label
 

src/textual/widgets/_data_table.py

@@ -104,7 +104,7 @@ class Coord(NamedTuple):
         return Coord(row + 1, column)
 
 
-class DataTable(ScrollView, Generic[CellType]):
+class DataTable(ScrollView, Generic[CellType], can_focus=True):
 
     CSS = """
     DataTable {

tests/test_geometry.py

@@ -251,6 +251,12 @@ def test_region_shrink():
     assert region.shrink(margin) == Region(x=14, y=11, width=44, height=46)
 
 
+def test_region_grow():
+    margin = Spacing(top=1, right=2, bottom=3, left=4)
+    region = Region(x=10, y=10, width=50, height=50)
+    assert region.grow(margin) == Region(x=6, y=9, width=56, height=54)
+
+
 def test_region_intersection():
     assert Region(0, 0, 100, 50).intersection(Region(10, 10, 10, 10)) == Region(
         10, 10, 10, 10