Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix content width #1910

Merged
merged 32 commits into from
Mar 2, 2023
Merged
Show file tree
Hide file tree
Changes from 24 commits
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
29bc5de
fix calculation for scrollbars
willmcgugan Feb 28, 2023
3a92cec
added snapshot
willmcgugan Feb 28, 2023
440bcbf
Merge branch 'composit-region-refactor' into scroll-refactor
willmcgugan Feb 28, 2023
4b21a22
fix for name change
willmcgugan Feb 28, 2023
7eea84e
snapshot
willmcgugan Feb 28, 2023
62e3f2e
merge
willmcgugan Feb 28, 2023
44241bf
fix for textual colors
willmcgugan Feb 28, 2023
3cc2988
remove logs
willmcgugan Feb 28, 2023
10c388b
scrollbar logic
willmcgugan Mar 1, 2023
6a0e665
scroll logic
willmcgugan Mar 1, 2023
4038ed5
remove dead code
willmcgugan Mar 1, 2023
6563a39
snapshot tests
willmcgugan Mar 1, 2023
8a3d785
scrollbar mechanism
willmcgugan Mar 1, 2023
61cc7b0
tidy
willmcgugan Mar 1, 2023
61e5335
demo tweak
willmcgugan Mar 1, 2023
2b9db6f
preset window size
willmcgugan Mar 1, 2023
9ba94e5
no need for repaint
willmcgugan Mar 1, 2023
174b229
Restore repaint
willmcgugan Mar 1, 2023
3fc6313
wait for idle on pause
willmcgugan Mar 1, 2023
81dc5de
colors tweak
willmcgugan Mar 1, 2023
cddda7d
remove wait for idle
willmcgugan Mar 1, 2023
33ebc0f
snapshot
willmcgugan Mar 1, 2023
a6a0d81
small sleep
willmcgugan Mar 1, 2023
4d9ec32
change stabilizer
willmcgugan Mar 1, 2023
dde7564
Merge branch 'main' into fix-content-width
willmcgugan Mar 1, 2023
bceffa5
debug tweaks
willmcgugan Mar 2, 2023
8bfad7f
remove debug
willmcgugan Mar 2, 2023
74be89e
remove debug
willmcgugan Mar 2, 2023
695c4a7
snapshot test
willmcgugan Mar 2, 2023
fd148f3
docstring
willmcgugan Mar 2, 2023
a5fac4a
changelog
willmcgugan Mar 2, 2023
955181b
add pause
willmcgugan Mar 2, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/textual/_doc.py
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ async def auto_pilot(pilot: Pilot) -> None:
app = pilot.app
await pilot.press(*press)
await pilot.wait_for_scheduled_animations()
await pilot.pause()
await pilot.pause(0.05)
svg = app.export_screenshot(title=title)

app.exit(svg)
Expand Down
22 changes: 5 additions & 17 deletions src/textual/_layout.py
Original file line number Diff line number Diff line change
Expand Up @@ -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]:
Expand Down Expand Up @@ -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(
Expand All @@ -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
1 change: 1 addition & 0 deletions src/textual/box_model.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
)
Expand Down
2 changes: 1 addition & 1 deletion src/textual/cli/previews/colors.css
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ ColorButtons {
}

ColorButtons > Button {
width: 30;
width: 100%;
}

ColorsView {
Expand Down
1 change: 0 additions & 1 deletion src/textual/constants.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
"""


import os

from typing_extensions import Final
Expand Down
1 change: 1 addition & 0 deletions src/textual/demo.css
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,7 @@ Column {
height: auto;
min-height: 100vh;
align: center top;
overflow: hidden;
}


Expand Down
2 changes: 1 addition & 1 deletion src/textual/scroll_view.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch.

self.refresh()
if (
self._size != size
Expand Down
2 changes: 1 addition & 1 deletion src/textual/scrollbar.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.:

```
Expand Down
78 changes: 50 additions & 28 deletions src/textual/widget.py
Original file line number Diff line number Diff line change
Expand Up @@ -515,6 +515,8 @@ 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._stabilized_scrollbar_size = None
self._scrollbar_stabilizer = 0
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider adding docstrings to these two attributes.
self._stabilized_scrollbar_size is already typed in the body of Widget but _scrollbar_stabilizer is brand new.


def _get_virtual_dom(self) -> Iterable[Widget]:
"""Get widgets not part of the DOM.
Expand Down Expand Up @@ -848,14 +850,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
Expand Down Expand Up @@ -978,34 +977,36 @@ def _refresh_scrollbars(self) -> None:
styles = self.styles
overflow_x = styles.overflow_x
overflow_y = styles.overflow_y
width, height = self.container_size
width, height = self._container_size

show_horizontal = self.show_horizontal_scrollbar
show_horizontal = False
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you help me understand why the default behaviour is to turn off show_horizontal instead of leaving the previous value?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The previous value shouldn't come in to consideration. It should make the same calculation, no matter the current state of the scrollbars.

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
)
self._stabilized_scrollbar_size = self.container_size
if self._stabilized_scrollbar_size != self.container_size:
# 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
)
if show_horizontal and not show_vertical:
show_vertical = self.virtual_size.height > (
height - styles.scrollbar_size_horizontal
)

self._stabilized_scrollbar_size = self._container_size

self.show_horizontal_scrollbar = show_horizontal
self.show_vertical_scrollbar = show_vertical
Expand Down Expand Up @@ -2098,7 +2099,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,
Expand All @@ -2109,18 +2110,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.
Expand Down Expand Up @@ -2239,9 +2256,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)
Expand Down Expand Up @@ -2363,6 +2384,7 @@ def refresh(
"""
if layout and not self._layout_required:
self._layout_required = True
self._stabilized_scrollbar_size = None
for ancestor in self.ancestors:
if not isinstance(ancestor, Widget):
break
Expand Down
1 change: 0 additions & 1 deletion src/textual/widgets/_text_log.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
from rich.pretty import Pretty
from rich.protocol import is_renderable
from rich.segment import Segment
from rich.style import Style
from rich.text import Text

from .._cache import LRUCache
Expand Down
442 changes: 299 additions & 143 deletions tests/snapshot_tests/__snapshots__/test_snapshots.ambr

Large diffs are not rendered by default.

54 changes: 54 additions & 0 deletions tests/snapshot_tests/snapshot_apps/test_line_api_scrollbars.py
Original file line number Diff line number Diff line change
@@ -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()
4 changes: 4 additions & 0 deletions tests/snapshot_tests/test_snapshots.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 / "test_line_api_scrollbars.py", press=["_"])
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The app examples in snapshot_apps don't have the leading test_ in their names.
I don't think this will break anything, but it may create the expectation (for humans and for pytest) that there are tests written in this file.