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 @@ def get_content_width(self, widget: Widget, container: Size, viewport: Size) ->
             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
-            width = max(
-                [
-                    placement.region.right + placement.margin.right
-                    for placement in placements
-                ],
-                default=0,
-            )
+            arrangement = widget._arrange(Size(0, 0))
+            return arrangement.total_region.right + arrangement.spacing.right
         return width
 
     def get_content_height(
@@ -139,13 +134,6 @@ def get_content_height(
             height = 0
         else:
             # Use a height of zero to ignore relative heights
-            placements = widget._arrange(Size(width, 0)).placements
-            height = max(
-                [
-                    placement.region.bottom + placement.margin.bottom
-                    for placement in placements
-                ],
-                default=0,
-            )
-
+            arrangement = widget._arrange(Size(width, 0))
+            height = arrangement.total_region.bottom + arrangement.spacing.bottom
         return height

src/textual/app.py

@@ -1817,7 +1817,7 @@ async def _close_all(self) -> None:
             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 @@
 
 """
 
-
 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 @@ def call_after_refresh(self, callback: Callable, *args, **kwargs) -> None:
         """
         # 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 @@ def _size_updated(
         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 @@ def _size_updated(
             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 @@ def __rich_console__(
 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 @@ def __init__(
 
         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 @@ def _arrange(self, size: Size) -> DockArrangeResult:
     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 @@ def get_content_height(self, container: Size, viewport: Size, width: int) -> int
         """
         if self.is_container:
             assert self._layout is not None
-            height = (
-                self._layout.get_content_height(
-                    self,
-                    container,
-                    viewport,
-                    width,
-                )
-                + self.scrollbar_size_horizontal
+            height = self._layout.get_content_height(
+                self,
+                container,
+                viewport,
+                width,
             )
         else:
             cache_key = width
@@ -913,8 +912,7 @@ def max_scroll_x(self) -> int:
         return max(
             0,
             self.virtual_size.width
-            - self.container_size.width
-            + self.scrollbar_size_vertical,
+            - (self.container_size.width - self.scrollbar_size_vertical),
         )
 
     @property
@@ -923,8 +921,7 @@ def max_scroll_y(self) -> int:
         return max(
             0,
             self.virtual_size.height
-            - self.container_size.height
-            + self.scrollbar_size_horizontal,
+            - (self.container_size.height - self.scrollbar_size_horizontal),
         )
 
     @property
@@ -985,34 +982,44 @@ def _refresh_scrollbars(self) -> None:
         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":
             show_horizontal = True
         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":
             show_vertical = True
         elif overflow_y == "auto":
             show_vertical = self.virtual_size.height > height
 
-        if (
-            overflow_x == "auto"
-            and show_vertical
-            and not show_horizontal
-            and self._stabilized_scrollbar_size != self.container_size
-        ):
-            show_horizontal = (
-                self.virtual_size.width + styles.scrollbar_size_vertical > width
+        # When a single scrollbar is shown, the other dimension changes, so we need to recalculate.
+        if show_vertical and not show_horizontal:
+            show_horizontal = self.virtual_size.width > (
+                width - styles.scrollbar_size_vertical
             )
-            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 @@ def _scroll_to(
         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 @@ def _arrange_scrollbars(self, region: Region) -> Iterable[tuple[Widget, Region]]
 
         if show_horizontal_scrollbar and show_vertical_scrollbar:
             (
-                _,
+                window_region,
                 vertical_scrollbar_region,
                 horizontal_scrollbar_region,
                 scrollbar_corner_gap,
@@ -2242,18 +2250,34 @@ def _arrange_scrollbars(self, region: Region) -> Iterable[tuple[Widget, Region]]
             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 @@ def _scroll_update(self, virtual_size: Size) -> None:
             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 @@ def refresh(
         """
         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 @@ def write(
             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."""