Merge pull request #5703 from Textualize/collapsible-fix

Fix for auto height in collapsible

Author: willmcgugan

CHANGELOG.md

@@ -10,6 +10,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 ### Fixed
 
 - Fixed markup escaping edge cases https://github.com/Textualize/textual/pull/5697
+- Fixed incorrect auto height in Collapsible https://github.com/Textualize/textual/pull/5703
 
 ### Changed
 

src/textual/css/styles.py

@@ -1347,7 +1347,7 @@ def rich_style(self) -> Style:
 
     @property
     def gutter(self) -> Spacing:
-        """Get space around widget.
+        """Get space around widget (padding + border)
 
         Returns:
             Space around widget content.

src/textual/layout.py

@@ -261,9 +261,8 @@ def get_content_height(
                 child.styles.is_dynamic_height for child in widget.displayed_children
             ):
                 # An exception for containers with all dynamic height widgets
-                arrangement = widget._arrange(
-                    Size(width, container.height - widget.gutter.height)
-                )
+                arrangement = widget._arrange(Size(width, container.height))
+                return arrangement.total_region.bottom
             else:
                 arrangement = widget._arrange(Size(width, 0))
             height = arrangement.total_region.bottom

src/textual/widget.py

@@ -1519,7 +1519,7 @@ def _get_box_model(
         content_width = Fraction(_content_width)
         content_height = Fraction(_content_height)
         is_border_box = styles.box_sizing == "border-box"
-        gutter = styles.gutter
+        gutter = styles.gutter  # Padding plus border
         margin = styles.margin
 
         is_auto_width = styles.width and styles.width.is_auto
@@ -1590,7 +1590,11 @@ def _get_box_model(
         elif is_auto_height:
             # Calculate dimensions based on content
             content_height = Fraction(
-                self.get_content_height(content_container, viewport, int(content_width))
+                self.get_content_height(
+                    content_container - margin.totals,
+                    viewport,
+                    int(content_width),
+                )
             )
             if (
                 styles.overflow_y == "auto" and styles.scrollbar_gutter == "stable"
@@ -1629,12 +1633,12 @@ def _get_box_model(
             max_height = styles.max_height.resolve(
                 container - margin.totals, viewport, height_fraction
             )
+
             if is_border_box:
                 max_height -= gutter.height
             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/widgets/_collapsible.py

@@ -22,7 +22,7 @@ class CollapsibleTitle(Static, can_focus=True):
     CollapsibleTitle {
         width: auto;
         height: auto;
-        padding: 0 1 0 1;
+        padding: 0 1;
         text-style: $block-cursor-blurred-text-style;
         color: $block-cursor-blurred-foreground;
 
@@ -227,7 +227,8 @@ def _on_mount(self, event: events.Mount) -> None:
 
     def compose(self) -> ComposeResult:
         yield self._title
-        yield self.Contents(*self._contents_list)
+        with self.Contents():
+            yield from self._contents_list
 
     def compose_add_child(self, widget: Widget) -> None:
         """When using the context manager compose syntax, we want to attach nodes to the contents.

tests/snapshot_tests/__snapshots__/test_snapshots/test_collapsible_datatable.svg

@@ -56,6 +56,7 @@
 )
 from textual.theme import Theme
 from textual.widgets.text_area import BUILTIN_LANGUAGES, Selection, TextAreaTheme
+from textual.widgets.selection_list import Selection as SLSelection
 
 # These paths should be relative to THIS directory.
 WIDGET_EXAMPLES_DIR = Path("../../docs/examples/widgets")
@@ -3274,18 +3275,28 @@ async def run_before(pilot: Pilot) -> None:
     )
 
 
+# TODO: Is this fixable?
+@pytest.mark.xfail(
+    reason="This doesn't work as described, because of the height auto in Collapsible.Contents. "
+    "I suspect it isn't broken per se, it's just that the intuitive interpretation is not correct."
+)
 def test_collapsible_datatable(snap_compare):
     """Regression test for https://github.com/Textualize/textual/issues/5407
 
     You should see two collapsibles, where the first is expanded.
-    In the expanded coillapsible, you should see a DataTable filling the space,
+    In the expanded collapsible, you should see a DataTable filling the space,
     with all borders and both scrollbars visible.
     """
 
     class MyApp(App):
         CSS = """
         DataTable {
-        
+            max-height: 1fr;            
+            border: red;            
+        }
+        Collapsible {
+            max-height: 50%;
+            
         }
         """
 
@@ -3294,14 +3305,10 @@ def compose(self) -> ComposeResult:
             yield Collapsible(Label("hello"), id="c2")
 
         def on_mount(self) -> None:
-            self.query_one("#c1", Collapsible).styles.max_height = "50%"
-            self.query_one("#c2", Collapsible).styles.max_height = "50%"
-
             t1 = self.query_one("#t1", DataTable)
-            t1.styles.border = "heavy", "black"
             t1.add_column("A")
             for number in range(1, 100):
-                t1.add_row(str(number) + " " * 200)
+                t1.add_row(str(number) + " " * 100)
 
     assert snap_compare(MyApp())
 
@@ -3804,3 +3811,34 @@ async def run_before(pilot: Pilot) -> None:
         await pilot.click(Label, times=2)
 
     assert snap_compare(AllowSelectApp(), run_before=run_before)
+
+
+def test_select_list_in_collapsible(snap_compare):
+    """Regression test for https://github.com/Textualize/textual/issues/5690
+
+    The Collapsible should be expanded, and just fit a Selection List with 3 items.
+    """
+
+    class CustomWidget(Horizontal):
+        DEFAULT_CSS = """
+        CustomWidget {
+            height: auto;                	
+        }
+        """
+
+        def compose(self) -> ComposeResult:
+            with Collapsible(title="Toggle for options", collapsed=False):
+                yield SelectionList[int](
+                    SLSelection("first selection", 1),
+                    SLSelection("second selection", 2),
+                    SLSelection(
+                        "third selection", 3
+                    ),  # not visible after toggling collapsible
+                )
+
+    class MyApp(App):
+        def compose(self) -> ComposeResult:
+            yield CustomWidget()
+            yield Footer()
+
+    snap_compare(MyApp())