fix for min and max with fr unints (#2390)

* fix for min and max with fr unints

* snapshit

* forgot snapshit tests

* fix resolve

* added unit tests

* Windoze fix

CHANGELOG.md

@@ -5,7 +5,12 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](http://keepachangelog.com/)
 and this project adheres to [Semantic Versioning](http://semver.org/).
 
-## Unreleased
+## [0.22.0] - Unreleased
+
+### Fixed
+
+- Fixed broken fr units when there is a min or max dimension https://github.com/Textualize/textual/issues/2378
+- Fixed plain text in Markdown code blocks with no syntax being difficult to read https://github.com/Textualize/textual/issues/2400
 
 ### Added
 
@@ -15,9 +20,6 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 
 - All `textual.containers` are now `1fr` in relevant dimensions by default https://github.com/Textualize/textual/pull/2386
 
-### Fixed
-
-- Fixed plain text in Markdown code blocks with no syntax being difficult to read https://github.com/Textualize/textual/issues/2400
 
 ## [0.21.0] - 2023-04-26
 

src/textual/_resolve.py

@@ -2,12 +2,13 @@ from __future__ import annotations
 
 from fractions import Fraction
 from itertools import accumulate
-from typing import TYPE_CHECKING, Sequence, cast
+from typing import TYPE_CHECKING, Iterable, Sequence, cast
 
 from typing_extensions import Literal
 
 from .box_model import BoxModel
 from .css.scalar import Scalar
+from .css.styles import RenderStyles
 from .geometry import Size
 
 if TYPE_CHECKING:
@@ -78,13 +79,105 @@ def resolve(
     return results
 
 
+def resolve_fraction_unit(
+    widget_styles: Iterable[RenderStyles],
+    size: Size,
+    viewport_size: Size,
+    remaining_space: Fraction,
+    resolve_dimension: Literal["width", "height"] = "width",
+) -> Fraction:
+    """Calculate the fraction
+
+    Args:
+        widget_styles: Styles for widgets with fraction units.
+        size: Container size.
+        viewport_size: Viewport size.
+        remaining_space: Remaining space for fr units.
+        resolve_dimension: Which dimension to resolve.
+
+    Returns:
+        The value of 1fr.
+    """
+    if not remaining_space or not widget_styles:
+        return Fraction(1)
+
+    def resolve_scalar(
+        scalar: Scalar | None, fraction_unit: Fraction = Fraction(1)
+    ) -> Fraction | None:
+        """Resolve a scalar if it is not None.
+
+        Args:
+            scalar: Optional scalar to resolve.
+            fraction_unit: Size of 1fr.
+
+        Returns:
+            Fraction if resolved, otherwise None.
+        """
+        return (
+            None
+            if scalar is None
+            else scalar.resolve(size, viewport_size, fraction_unit)
+        )
+
+    resolve: list[tuple[Scalar, Fraction | None, Fraction | None]] = []
+
+    if resolve_dimension == "width":
+        resolve = [
+            (
+                cast(Scalar, styles.width),
+                resolve_scalar(styles.min_width),
+                resolve_scalar(styles.max_width),
+            )
+            for styles in widget_styles
+        ]
+    else:
+        resolve = [
+            (
+                cast(Scalar, styles.height),
+                resolve_scalar(styles.min_height),
+                resolve_scalar(styles.max_height),
+            )
+            for styles in widget_styles
+        ]
+
+    resolved: list[Fraction | None] = [None] * len(resolve)
+    remaining_fraction = Fraction(sum(scalar.value for scalar, _, _ in resolve))
+
+    while True:
+        remaining_space_changed = False
+        resolve_fraction = Fraction(remaining_space, remaining_fraction)
+        for index, (scalar, min_value, max_value) in enumerate(resolve):
+            value = resolved[index]
+            if value is None:
+                resolved_scalar = scalar.resolve(size, viewport_size, resolve_fraction)
+                if min_value is not None and resolved_scalar < min_value:
+                    remaining_space -= min_value
+                    remaining_fraction -= Fraction(scalar.value)
+                    resolved[index] = min_value
+                    remaining_space_changed = True
+                elif max_value is not None and resolved_scalar > max_value:
+                    remaining_space -= max_value
+                    remaining_fraction -= Fraction(scalar.value)
+                    resolved[index] = max_value
+                    remaining_space_changed = True
+
+        if not remaining_space_changed:
+            break
+
+    return (
+        Fraction(remaining_space, remaining_fraction)
+        if remaining_space
+        else Fraction(1)
+    )
+
+
 def resolve_box_models(
     dimensions: list[Scalar | None],
     widgets: list[Widget],
     size: Size,
     viewport_size: Size,
     margin: Size,
-    dimension: Literal["width", "height"] = "width",
+    resolve_dimension: Literal["width", "height"] = "width",
 ) -> list[BoxModel]:
     """Resolve box models for a list of dimensions
 
@@ -94,7 +187,7 @@ def resolve_box_models(
         size: Size of container.
         viewport_size: Viewport size.
         margin: Total space occupied by margin
-        dimensions: Which dimension to resolve.
+        resolve_dimension: Which dimension to resolve.
 
     Returns:
         List of resolved box models.
@@ -106,6 +199,7 @@ def resolve_box_models(
 
     margin_size = size - margin
 
+    # Fixed box models
     box_models: list[BoxModel | None] = [
         (
             None
@@ -117,30 +211,46 @@ def resolve_box_models(
         for (_dimension, widget) in zip(dimensions, widgets)
     ]
 
-    if dimension == "width":
+    if None not in box_models:
-        total_remaining = sum([width for width, _, _ in filter(None, box_models)])
+        # No fr units, so we're done
-        remaining_space = max(0, size.width - total_remaining - margin_width)
+        return cast("list[BoxModel]", box_models)
-    else:
-        total_remaining = sum([height for _, height, _ in filter(None, box_models)])
-        remaining_space = max(0, size.height - total_remaining - margin_height)
 
-    fraction_unit = Fraction(
+    # If all box models have been calculated
-        remaining_space,
+    widget_styles = [widget.styles for widget in widgets]
-        int(
+    if resolve_dimension == "width":
-            sum(
+        total_remaining = int(
-                [
+            sum([width for width, _, _ in filter(None, box_models)], start=Fraction())
-                    dimension.value
+        )
-                    for dimension in dimensions
+        remaining_space = int(max(0, size.width - total_remaining - margin_width))
-                    if dimension and dimension.is_fraction
+        fraction_unit = resolve_fraction_unit(
-                ]
+            [
-            )
+                styles
+                for styles in widget_styles
+                if styles.width is not None and styles.width.is_fraction
+            ],
+            size,
+            viewport_size,
+            Fraction(remaining_space),
+            resolve_dimension,
         )
-        or 1,
-    )
-    if dimension == "width":
         width_fraction = fraction_unit
         height_fraction = Fraction(margin_size.height)
     else:
+        total_remaining = int(
+            sum([height for _, height, _ in filter(None, box_models)], start=Fraction())
+        )
+        remaining_space = int(max(0, size.height - total_remaining - margin_height))
+        fraction_unit = resolve_fraction_unit(
+            [
+                styles
+                for styles in widget_styles
+                if styles.height is not None and styles.height.is_fraction
+            ],
+            size,
+            viewport_size,
+            Fraction(remaining_space),
+            resolve_dimension,
+        )
         width_fraction = Fraction(margin_size.width)
         height_fraction = fraction_unit
 

src/textual/css/_style_properties.py

@@ -187,7 +187,7 @@ class ScalarProperty:
                 new_value = Scalar.parse(value)
             except ScalarParseError:
                 raise StyleValueError(
-                    "unable to parse scalar from {value!r}",
+                    f"unable to parse scalar from {value!r}",
                     help_text=scalar_help_text(
                         property_name=self.name, context="inline"
                     ),

src/textual/layouts/horizontal.py

@@ -53,7 +53,7 @@ class HorizontalLayout(Layout):
             size,
             parent.app.size,
             resolve_margin,
-            dimension="width",
+            resolve_dimension="width",
         )
 
         margins = [

src/textual/layouts/vertical.py

@@ -50,7 +50,7 @@ class VerticalLayout(Layout):
             size,
             parent.app.size,
             resolve_margin,
-            dimension="height",
+            resolve_dimension="height",
         )
 
         margins = [

src/textual/widget.py

@@ -2699,6 +2699,8 @@ class Widget(DOMNode):
             or self.virtual_size != virtual_size
             or self._container_size != container_size
         ):
+            if self._size != size:
+                self._set_dirty()
             self._size = size
             if layout:
                 self.virtual_size = virtual_size

tests/snapshot_tests/snapshot_apps/fr_with_min.py

@@ -0,0 +1,50 @@
+from textual.app import App, ComposeResult
+from textual.containers import Horizontal, Vertical, VerticalScroll
+from textual.widgets import Header, Footer, Static
+
+
+class ScreenSplitApp(App[None]):
+    CSS = """
+    Horizontal {
+        width: 1fr;        
+    }
+
+    Vertical {
+        width: 1fr;
+        background: blue;
+        min-width: 20;
+    }
+
+    #scroll1 {
+        width: 1fr;
+        background: $panel;
+    }
+
+    #scroll2 {
+        width: 2fr;
+        background: $panel;
+    }
+
+    Static {
+        width: 1fr;
+        content-align: center middle;
+        background: $boost;
+    }
+
+    """
+
+    def compose(self) -> ComposeResult:
+        yield Header()
+        with Horizontal():
+            yield Vertical()
+            with VerticalScroll(id="scroll1"):
+                for n in range(100):
+                    yield Static(f"This is content number {n}")
+            with VerticalScroll(id="scroll2"):
+                for n in range(100):
+                    yield Static(f"This is content number {n}")
+        yield Footer()
+
+
+if __name__ == "__main__":
+    ScreenSplitApp().run()

tests/snapshot_tests/test_snapshots.py

@@ -319,7 +319,7 @@ def test_demo(snap_compare):
     """Test the demo app (python -m textual)"""
     assert snap_compare(
         Path("../../src/textual/demo.py"),
-        press=["down", "down", "down", "wait:250"],
+        press=["down", "down", "down", "wait:500"],
         terminal_size=(100, 30),
     )
 
@@ -460,3 +460,8 @@ def test_scroll_to_center(snap_compare):
 def test_quickly_change_tabs(snap_compare):
     # https://github.com/Textualize/textual/issues/2229
     assert snap_compare(SNAPSHOT_APPS_DIR / "quickly_change_tabs.py", press=["p"])
+
+
+def test_fr_unit_with_min(snap_compare):
+    # https://github.com/Textualize/textual/issues/2378
+    assert snap_compare(SNAPSHOT_APPS_DIR / "fr_with_min.py")

tests/test_resolve.py

@@ -1,8 +1,11 @@
+from fractions import Fraction
+
 import pytest
 
-from textual._resolve import resolve
+from textual._resolve import resolve, resolve_fraction_unit
 from textual.css.scalar import Scalar
 from textual.geometry import Size
+from textual.widget import Widget
 
 
 def test_resolve_empty():
@@ -56,3 +59,66 @@ def test_resolve(scalars, total, gutter, result):
         )
         == result
     )
+
+
+def test_resolve_fraction_unit():
+    """Test resolving fraction units in combination with minimum widths."""
+    widget1 = Widget()
+    widget2 = Widget()
+    widget3 = Widget()
+
+    widget1.styles.width = "1fr"
+    widget1.styles.min_width = 20
+
+    widget2.styles.width = "2fr"
+    widget2.styles.min_width = 10
+
+    widget3.styles.width = "1fr"
+
+    styles = (widget1.styles, widget2.styles, widget3.styles)
+
+    # Try with width 80.
+    # Fraction unit should one fourth of width
+    assert resolve_fraction_unit(
+        styles,
+        Size(80, 24),
+        Size(80, 24),
+        Fraction(80),
+        resolve_dimension="width",
+    ) == Fraction(20)
+
+    # Try with width 50
+    # First widget is fixed at 20
+    # Remaining three widgets have 30 to play with
+    # Fraction is 10
+    assert resolve_fraction_unit(
+        styles,
+        Size(80, 24),
+        Size(80, 24),
+        Fraction(50),
+        resolve_dimension="width",
+    ) == Fraction(10)
+
+    # Try with width 35
+    # First widget fixed at 20
+    # Fraction is 5
+    assert resolve_fraction_unit(
+        styles,
+        Size(80, 24),
+        Size(80, 24),
+        Fraction(35),
+        resolve_dimension="width",
+    ) == Fraction(5)
+
+    # Try with width 32
+    # First widget fixed at 20
+    # Second widget is fixed at 10
+    # Remaining widget has all the space
+    # Fraction is 2
+    assert resolve_fraction_unit(
+        styles,
+        Size(80, 24),
+        Size(80, 24),
+        Fraction(32),
+        resolve_dimension="width",
+    ) == Fraction(2)