From b005c13956be1e248cab3f79b8d9977947ecc5da Mon Sep 17 00:00:00 2001 From: Will McGugan Date: Tue, 28 Mar 2023 15:27:01 +0100 Subject: [PATCH] remove arrangement spacing (#2157) * remove arrangement spacing * snapshot test * comment * changelog --- CHANGELOG.md | 1 + src/textual/_arrange.py | 2 +- src/textual/_compositor.py | 5 +- src/textual/_layout.py | 6 +- .../__snapshots__/test_snapshots.ambr | 159 ++++++++++++++++++ .../snapshot_tests/snapshot_apps/layer_fix.py | 48 ++++++ tests/snapshot_tests/test_snapshots.py | 5 + tests/test_arrange.py | 5 - 8 files changed, 217 insertions(+), 14 deletions(-) create mode 100644 tests/snapshot_tests/snapshot_apps/layer_fix.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 10de68990..3aec5a144 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/). - Issue with parsing action strings whose arguments contained quoted closing parenthesis https://github.com/Textualize/textual/pull/2112 - Issues with parsing action strings with tuple arguments https://github.com/Textualize/textual/pull/2112 - Fix for tabs not invalidating https://github.com/Textualize/textual/issues/2125 +- Fixed scrollbar layers issue https://github.com/Textualize/textual/issues/1358 - Fix for interaction between pseudo-classes and widget-level render caches https://github.com/Textualize/textual/pull/2155 ### Changed diff --git a/src/textual/_arrange.py b/src/textual/_arrange.py index f1035688e..378d951eb 100644 --- a/src/textual/_arrange.py +++ b/src/textual/_arrange.py @@ -128,4 +128,4 @@ def arrange( placements.extend(layout_placements) - return DockArrangeResult(placements, arrange_widgets, scroll_spacing) + return DockArrangeResult(placements, arrange_widgets) diff --git a/src/textual/_compositor.py b/src/textual/_compositor.py index dd7816f9c..addec6ea8 100644 --- a/src/textual/_compositor.py +++ b/src/textual/_compositor.py @@ -484,7 +484,6 @@ class Compositor: # Arrange the layout arrange_result = widget._arrange(child_region.size) arranged_widgets = arrange_result.widgets - spacing = arrange_result.spacing widgets.update(arranged_widgets) if visible_only: @@ -513,9 +512,7 @@ class Compositor: if fixed: widget_region = sub_region + placement_offset else: - total_region = total_region.union( - sub_region.grow(spacing + margin) - ) + total_region = total_region.union(sub_region.grow(margin)) widget_region = sub_region + placement_scroll_offset widget_order = order + ( diff --git a/src/textual/_layout.py b/src/textual/_layout.py index 0cdfe5956..5c5dc2ff1 100644 --- a/src/textual/_layout.py +++ b/src/textual/_layout.py @@ -21,8 +21,6 @@ class DockArrangeResult: """A `WidgetPlacement` for every widget to describe it's location on screen.""" widgets: set[Widget] """A set of widgets in the arrangement.""" - spacing: Spacing - """Shared spacing around the widgets.""" _spatial_map: SpatialMap[WidgetPlacement] | None = None """A Spatial map to query widget placements.""" @@ -113,7 +111,7 @@ class Layout(ABC): else: # Use a size of 0, 0 to ignore relative sizes, since those are flexible anyway arrangement = widget._arrange(Size(0, 0)) - return arrangement.total_region.right + arrangement.spacing.right + return arrangement.total_region.right return width def get_content_height( @@ -135,5 +133,5 @@ class Layout(ABC): else: # Use a height of zero to ignore relative heights arrangement = widget._arrange(Size(width, 0)) - height = arrangement.total_region.bottom + arrangement.spacing.bottom + height = arrangement.total_region.bottom return height diff --git a/tests/snapshot_tests/__snapshots__/test_snapshots.ambr b/tests/snapshot_tests/__snapshots__/test_snapshots.ambr index 72fc4afec..0f3d41b3e 100644 --- a/tests/snapshot_tests/__snapshots__/test_snapshots.ambr +++ b/tests/snapshot_tests/__snapshots__/test_snapshots.ambr @@ -15508,6 +15508,165 @@ ''' # --- +# name: test_layer_fix + ''' + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + DialogIssueApp + + + + + + + + + + ⭘DialogIssueApp + + + + + + ╭───────────────────────────────────────╮ + ││ + ││ + ││ + ││ + ││ + │This should not cause a scrollbar to ap│ + ││ + ││ + ││ + ││ + ││ + ╰───────────────────────────────────────╯ + + + + +  D  Toggle the dialog  + + + + + ''' +# --- # name: test_layers ''' diff --git a/tests/snapshot_tests/snapshot_apps/layer_fix.py b/tests/snapshot_tests/snapshot_apps/layer_fix.py new file mode 100644 index 000000000..2cc8861a8 --- /dev/null +++ b/tests/snapshot_tests/snapshot_apps/layer_fix.py @@ -0,0 +1,48 @@ +from textual.app import App, ComposeResult +from textual.containers import Vertical +from textual.widgets import Header, Footer, Label +from textual.binding import Binding + + +class Dialog(Vertical): + def compose(self) -> ComposeResult: + """Compose the child widgets.""" + yield Label("This should not cause a scrollbar to appear") + + +class DialogIssueApp(App[None]): + CSS = """ + Screen { + layers: base dialog; + } + + .hidden { + display: none; + } + + Dialog { + align: center middle; + border: round red; + width: 50%; + height: 50%; + layer: dialog; + offset: 50% 50%; + } + """ + + BINDINGS = [ + Binding("d", "dialog", "Toggle the dialog"), + ] + + def compose(self) -> ComposeResult: + yield Header() + yield Vertical() + yield Dialog(classes="hidden") + yield Footer() + + def action_dialog(self) -> None: + self.query_one(Dialog).toggle_class("hidden") + + +if __name__ == "__main__": + DialogIssueApp().run() diff --git a/tests/snapshot_tests/test_snapshots.py b/tests/snapshot_tests/test_snapshots.py index 7c68f1405..ec268741b 100644 --- a/tests/snapshot_tests/test_snapshots.py +++ b/tests/snapshot_tests/test_snapshots.py @@ -340,3 +340,8 @@ def test_scrollbar_thumb_height(snap_compare): assert snap_compare( SNAPSHOT_APPS_DIR / "scrollbar_thumb_height.py", ) + + +def test_layer_fix(snap_compare): + # Check https://github.com/Textualize/textual/issues/1358 + assert snap_compare(SNAPSHOT_APPS_DIR / "layer_fix.py", press=["d"]) diff --git a/tests/test_arrange.py b/tests/test_arrange.py index 582f54bc1..38f5a191d 100644 --- a/tests/test_arrange.py +++ b/tests/test_arrange.py @@ -12,7 +12,6 @@ def test_arrange_empty(): result = arrange(container, [], Size(80, 24), Size(80, 24)) assert result.placements == [] assert result.widgets == set() - assert result.spacing == Spacing(0, 0, 0, 0) def test_arrange_dock_top(): @@ -31,7 +30,6 @@ def test_arrange_dock_top(): WidgetPlacement(Region(0, 1, 80, 23), Spacing(), child, order=0, fixed=False), ] assert result.widgets == {child, header} - assert result.spacing == Spacing(1, 0, 0, 0) def test_arrange_dock_left(): @@ -49,7 +47,6 @@ def test_arrange_dock_left(): WidgetPlacement(Region(10, 0, 70, 24), Spacing(), child, order=0, fixed=False), ] assert result.widgets == {child, header} - assert result.spacing == Spacing(0, 0, 0, 10) def test_arrange_dock_right(): @@ -67,7 +64,6 @@ def test_arrange_dock_right(): WidgetPlacement(Region(0, 0, 70, 24), Spacing(), child, order=0, fixed=False), ] assert result.widgets == {child, header} - assert result.spacing == Spacing(0, 10, 0, 0) def test_arrange_dock_bottom(): @@ -85,7 +81,6 @@ def test_arrange_dock_bottom(): WidgetPlacement(Region(0, 0, 80, 23), Spacing(), child, order=0, fixed=False), ] assert result.widgets == {child, header} - assert result.spacing == Spacing(0, 0, 1, 0) def test_arrange_dock_badly():