Fix content width (#1910)

* fix calculation for scrollbars

* added snapshot

* fix for name change

* snapshot

* fix for textual colors

* remove logs

* scrollbar logic

* scroll logic

* remove dead code

* snapshot tests

* scrollbar mechanism

* tidy

* demo tweak

* preset window size

* no need for repaint

* Restore repaint

* wait for idle on pause

* colors tweak

* remove wait for idle

* snapshot

* small sleep

* change stabilizer

* debug tweaks

* remove debug

* remove debug

* snapshot test

* docstring

* changelog

* add pause

CHANGELOG.md

@@ -21,10 +21,9 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 ### Fixed
 
 - Scrolling with cursor keys now moves just one cell https://github.com/Textualize/textual/issues/1897
-
-### Fixed
-
 - Fix exceptions in watch methods being hidden on startup https://github.com/Textualize/textual/issues/1886
+- Fixed scrollbar size miscalculation https://github.com/Textualize/textual/pull/1910
+- Fixed slow exit on some terminals https://github.com/Textualize/textual/issues/1920
 
 ## [0.12.1] - 2023-02-25
 

src/textual/_layout.py

@@ -25,6 +25,7 @@ class DockArrangeResult:
     """Shared spacing around the widgets."""
 
     _spatial_map: SpatialMap[WidgetPlacement] | None = None
+    """A Spatial map to query widget placements."""
 
     @property
     def spatial_map(self) -> SpatialMap[WidgetPlacement]:
@@ -111,14 +112,8 @@ class Layout(ABC):
             width = 0
         else:
             # Use a size of 0, 0 to ignore relative sizes, since those are flexible anyway
-            placements = widget._arrange(Size(0, 0)).placements
+            arrangement = widget._arrange(Size(0, 0))
-            width = max(
+            return arrangement.total_region.right + arrangement.spacing.right
-                [
-                    placement.region.right + placement.margin.right
-                    for placement in placements
-                ],
-                default=0,
-            )
         return width
 
     def get_content_height(
@@ -139,13 +134,6 @@ class Layout(ABC):
             height = 0
         else:
             # Use a height of zero to ignore relative heights
-            placements = widget._arrange(Size(width, 0)).placements
+            arrangement = widget._arrange(Size(width, 0))
-            height = max(
+            height = arrangement.total_region.bottom + arrangement.spacing.bottom
-                [
-                    placement.region.bottom + placement.margin.bottom
-                    for placement in placements
-                ],
-                default=0,
-            )
-
         return height

src/textual/app.py

@@ -1817,7 +1817,7 @@ class App(Generic[ReturnType], DOMNode):
             await child._close_messages()
 
     async def _shutdown(self) -> None:
-        self._begin_update()  # Prevents any layout / repaint while shutting down
+        self._begin_batch()  # Prevents any layout / repaint while shutting down
         driver = self._driver
         self._running = False
         if driver is not None:

src/textual/box_model.py

@@ -125,6 +125,7 @@ def get_box_model(
         content_height = min(content_height, max_height)
 
     content_height = max(Fraction(0), content_height)
+
     model = BoxModel(
         content_width + gutter.width, content_height + gutter.height, margin
     )

src/textual/cli/previews/colors.css

@@ -9,7 +9,7 @@ ColorButtons {
 }
 
 ColorButtons > Button {
-    width: 30;
+    width: 100%;
 }
 
 ColorsView {

src/textual/constants.py

@@ -3,7 +3,6 @@ Constants that we might want to expose via the public API.
 
 """
 
-
 import os
 
 from typing_extensions import Final

src/textual/demo.css

@@ -102,6 +102,7 @@ Column {
     height: auto;
     min-height: 100vh;
     align: center top;
+    overflow: hidden;
 }
 
 

src/textual/message_pump.py

@@ -340,6 +340,7 @@ class MessagePump(metaclass=MessagePumpMeta):
         """
         # We send the InvokeLater message to ourselves first, to ensure we've cleared
         # out anything already pending in our own queue.
+
         message = messages.InvokeLater(self, partial(callback, *args, **kwargs))
         self.post_message_no_wait(message)
 

src/textual/scroll_view.py

@@ -82,7 +82,7 @@ class ScrollView(Widget):
         Returns:
             True if anything changed, or False if nothing changed.
         """
-        if self._size != size or container_size != container_size:
+        if self._size != size or self._container_size != container_size:
             self.refresh()
         if (
             self._size != size
@@ -93,7 +93,6 @@ class ScrollView(Widget):
             virtual_size = self.virtual_size
             self._container_size = size - self.styles.gutter.totals
             self._scroll_update(virtual_size)
-            self.scroll_to(self.scroll_x, self.scroll_y, animate=False)
             return True
         else:
             return False

src/textual/scrollbar.py

@@ -197,7 +197,7 @@ class ScrollBarRender:
 class ScrollBar(Widget):
     renderer: ClassVar[Type[ScrollBarRender]] = ScrollBarRender
     """The class used for rendering scrollbars.
-    This can be overriden and set to a ScrollBarRender-derived class
+    This can be overridden and set to a ScrollBarRender-derived class
     in order to delegate all scrollbar rendering to that class. E.g.:
 
     ```

src/textual/widget.py

@@ -274,7 +274,8 @@ class Widget(DOMNode):
 
         self._styles_cache = StylesCache()
         self._rich_style_cache: dict[str, tuple[Style, Style]] = {}
-        self._stabilized_scrollbar_size: Size | None = None
+        self._stabilize_scrollbar: tuple[Size, str, str] | None = None
+        """Used to prevent scrollbar logic getting stuck in an infinite loop."""
         self._lock = Lock()
 
         super().__init__(
@@ -520,6 +521,7 @@ class Widget(DOMNode):
     def _clear_arrangement_cache(self) -> None:
         """Clear arrangement cache, forcing a new arrange operation."""
         self._arrangement_cache.clear()
+        self._stabilize_scrollbar = None
 
     def _get_virtual_dom(self) -> Iterable[Widget]:
         """Get widgets not part of the DOM.
@@ -855,14 +857,11 @@ class Widget(DOMNode):
         """
         if self.is_container:
             assert self._layout is not None
-            height = (
+            height = self._layout.get_content_height(
-                self._layout.get_content_height(
+                self,
-                    self,
+                container,
-                    container,
+                viewport,
-                    viewport,
+                width,
-                    width,
-                )
-                + self.scrollbar_size_horizontal
             )
         else:
             cache_key = width
@@ -913,8 +912,7 @@ class Widget(DOMNode):
         return max(
             0,
             self.virtual_size.width
-            - self.container_size.width
+            - (self.container_size.width - self.scrollbar_size_vertical),
-            + self.scrollbar_size_vertical,
         )
 
     @property
@@ -923,8 +921,7 @@ class Widget(DOMNode):
         return max(
             0,
             self.virtual_size.height
-            - self.container_size.height
+            - (self.container_size.height - self.scrollbar_size_horizontal),
-            + self.scrollbar_size_horizontal,
         )
 
     @property
@@ -985,9 +982,18 @@ class Widget(DOMNode):
         styles = self.styles
         overflow_x = styles.overflow_x
         overflow_y = styles.overflow_y
-        width, height = self.container_size
 
-        show_horizontal = self.show_horizontal_scrollbar
+        stabilize_scrollbar = (
+            self.container_size,
+            overflow_x,
+            overflow_y,
+        )
+        if self._stabilize_scrollbar == stabilize_scrollbar:
+            return
+
+        width, height = self._container_size
+
+        show_horizontal = False
         if overflow_x == "hidden":
             show_horizontal = False
         elif overflow_x == "scroll":
@@ -995,7 +1001,7 @@ class Widget(DOMNode):
         elif overflow_x == "auto":
             show_horizontal = self.virtual_size.width > width
 
-        show_vertical = self.show_vertical_scrollbar
+        show_vertical = False
         if overflow_y == "hidden":
             show_vertical = False
         elif overflow_y == "scroll":
@@ -1003,16 +1009,17 @@ class Widget(DOMNode):
         elif overflow_y == "auto":
             show_vertical = self.virtual_size.height > height
 
-        if (
+        # When a single scrollbar is shown, the other dimension changes, so we need to recalculate.
-            overflow_x == "auto"
+        if show_vertical and not show_horizontal:
-            and show_vertical
+            show_horizontal = self.virtual_size.width > (
-            and not show_horizontal
+                width - styles.scrollbar_size_vertical
-            and self._stabilized_scrollbar_size != self.container_size
-        ):
-            show_horizontal = (
-                self.virtual_size.width + styles.scrollbar_size_vertical > width
             )
-            self._stabilized_scrollbar_size = self.container_size
+        if show_horizontal and not show_vertical:
+            show_vertical = self.virtual_size.height > (
+                height - styles.scrollbar_size_horizontal
+            )
+
+        self._stabilize_scrollbar = stabilize_scrollbar
 
         self.show_horizontal_scrollbar = show_horizontal
         self.show_vertical_scrollbar = show_vertical
@@ -1454,6 +1461,7 @@ class Widget(DOMNode):
         Returns:
             `True` if the scroll position changed, otherwise `False`.
         """
+
         maybe_scroll_x = x is not None and (self.allow_horizontal_scroll or force)
         maybe_scroll_y = y is not None and (self.allow_vertical_scroll or force)
         scrolled_x = scrolled_y = False
@@ -2231,7 +2239,7 @@ class Widget(DOMNode):
 
         if show_horizontal_scrollbar and show_vertical_scrollbar:
             (
-                _,
+                window_region,
                 vertical_scrollbar_region,
                 horizontal_scrollbar_region,
                 scrollbar_corner_gap,
@@ -2242,18 +2250,34 @@ class Widget(DOMNode):
             if scrollbar_corner_gap:
                 yield self.scrollbar_corner, scrollbar_corner_gap
             if vertical_scrollbar_region:
-                yield self.vertical_scrollbar, vertical_scrollbar_region
+                scrollbar = self.vertical_scrollbar
+                scrollbar.window_virtual_size = self.virtual_size.height
+                scrollbar.window_size = window_region.height
+                yield scrollbar, vertical_scrollbar_region
             if horizontal_scrollbar_region:
-                yield self.horizontal_scrollbar, horizontal_scrollbar_region
+                scrollbar = self.horizontal_scrollbar
+                scrollbar.window_virtual_size = self.virtual_size.width
+                scrollbar.window_size = window_region.width
+                yield scrollbar, horizontal_scrollbar_region
 
         elif show_vertical_scrollbar:
-            _, scrollbar_region = region.split_vertical(-scrollbar_size_vertical)
+            window_region, scrollbar_region = region.split_vertical(
+                -scrollbar_size_vertical
+            )
             if scrollbar_region:
-                yield self.vertical_scrollbar, scrollbar_region
+                scrollbar = self.vertical_scrollbar
+                scrollbar.window_virtual_size = self.virtual_size.height
+                scrollbar.window_size = window_region.height
+                yield scrollbar, scrollbar_region
         elif show_horizontal_scrollbar:
-            _, scrollbar_region = region.split_horizontal(-scrollbar_size_horizontal)
+            window_region, scrollbar_region = region.split_horizontal(
+                -scrollbar_size_horizontal
+            )
             if scrollbar_region:
-                yield self.horizontal_scrollbar, scrollbar_region
+                scrollbar = self.horizontal_scrollbar
+                scrollbar.window_virtual_size = self.virtual_size.width
+                scrollbar.window_size = window_region.width
+                yield scrollbar, scrollbar_region
 
     def get_pseudo_classes(self) -> Iterable[str]:
         """Pseudo classes for a widget.
@@ -2374,9 +2398,13 @@ class Widget(DOMNode):
             self.vertical_scrollbar.window_size = (
                 height - self.scrollbar_size_horizontal
             )
+            if self.vertical_scrollbar._repaint_required:
+                self.call_next(self.vertical_scrollbar.refresh)
         if self.show_horizontal_scrollbar:
             self.horizontal_scrollbar.window_virtual_size = virtual_size.width
             self.horizontal_scrollbar.window_size = width - self.scrollbar_size_vertical
+            if self.horizontal_scrollbar._repaint_required:
+                self.call_next(self.horizontal_scrollbar.refresh)
 
         self.scroll_x = self.validate_scroll_x(self.scroll_x)
         self.scroll_y = self.validate_scroll_y(self.scroll_y)
@@ -2498,6 +2526,7 @@ class Widget(DOMNode):
         """
         if layout and not self._layout_required:
             self._layout_required = True
+            self._stabilize_scrollbar = None
             for ancestor in self.ancestors:
                 if not isinstance(ancestor, Widget):
                     break

src/textual/widgets/_text_log.py

@@ -152,7 +152,7 @@ class TextLog(ScrollView, can_focus=True):
             self.refresh()
             self.lines = self.lines[-self.max_lines :]
         self.virtual_size = Size(self.max_width, len(self.lines))
-        self.scroll_end(animate=False, speed=100)
+        self.scroll_end(animate=False)
 
     def clear(self) -> None:
         """Clear the text log."""

tests/snapshot_tests/snapshot_apps/line_api_scrollbars.py

@@ -0,0 +1,54 @@
+from rich.text import Text
+
+from textual.app import App, ComposeResult
+from textual.containers import Vertical
+from textual.widget import Widget
+from textual.widgets import TextLog
+
+
+class MyWidget(Widget):
+    def render(self):
+        return Text(
+            "\n".join(f"{n} 0123456789" for n in range(20)),
+            no_wrap=True,
+            overflow="hidden",
+            justify="left",
+        )
+
+
+class ScrollViewApp(App):
+    CSS = """
+    Screen {
+        align: center middle;
+    }
+    
+    TextLog {
+        width:13;
+        height:10;        
+    }
+
+    Vertical{
+        width:13;
+        height: 10;
+        overflow: scroll;
+        overflow-x: auto;
+    }
+    MyWidget {
+        width:13;
+        height:auto;
+    }
+
+    """
+
+    def compose(self) -> ComposeResult:
+        yield TextLog()
+        yield Vertical(MyWidget())
+
+    def on_ready(self) -> None:
+        self.query_one(TextLog).write("\n".join(f"{n} 0123456789" for n in range(20)))
+        self.query_one(Vertical).scroll_end(animate=False)
+
+
+if __name__ == "__main__":
+    app = ScrollViewApp()
+    app.run()

tests/snapshot_tests/test_snapshots.py

@@ -256,3 +256,7 @@ def test_disabled_widgets(snap_compare):
 
 def test_focus_component_class(snap_compare):
     assert snap_compare(SNAPSHOT_APPS_DIR / "focus_component_class.py", press=["tab"])
+
+
+def test_line_api_scrollbars(snap_compare):
+    assert snap_compare(SNAPSHOT_APPS_DIR / "line_api_scrollbars.py", press=["_"])

tests/tree/test_tree_messages.py

@@ -46,6 +46,7 @@ async def test_tree_node_selected_message() -> None:
     """Selecting a node should result in a selected message being emitted."""
     async with TreeApp().run_test() as pilot:
         await pilot.press("enter")
+        await pilot.pause()
         assert pilot.app.messages == ["NodeExpanded", "NodeSelected"]
 
 
@@ -54,6 +55,7 @@ async def test_tree_node_selected_message_no_auto() -> None:
     async with TreeApp().run_test() as pilot:
         pilot.app.query_one(MyTree).auto_expand = False
         await pilot.press("enter")
+        await pilot.pause()
         assert pilot.app.messages == ["NodeSelected"]
 
 
@@ -61,6 +63,7 @@ async def test_tree_node_expanded_message() -> None:
     """Expanding a node should result in an expanded message being emitted."""
     async with TreeApp().run_test() as pilot:
         await pilot.press("space")
+        await pilot.pause()
         assert pilot.app.messages == ["NodeExpanded"]
 
 
@@ -68,6 +71,7 @@ async def test_tree_node_collapsed_message() -> None:
     """Collapsing a node should result in a collapsed message being emitted."""
     async with TreeApp().run_test() as pilot:
         await pilot.press("space", "space")
+        await pilot.pause()
         assert pilot.app.messages == ["NodeExpanded", "NodeCollapsed"]
 
 
@@ -75,4 +79,5 @@ async def test_tree_node_highlighted_message() -> None:
     """Highlighting a node should result in a highlighted message being emitted."""
     async with TreeApp().run_test() as pilot:
         await pilot.press("enter", "down")
+        await pilot.pause()
         assert pilot.app.messages == ["NodeExpanded", "NodeSelected", "NodeHighlighted"]