From dd70a7a2dcc3db61c91fd02ff0b187f344f97ac8 Mon Sep 17 00:00:00 2001 From: Will McGugan Date: Thu, 27 Apr 2023 13:35:16 +0100 Subject: [PATCH] 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 | 10 +- src/textual/_resolve.py | 154 ++++++++++++++--- src/textual/css/_style_properties.py | 2 +- src/textual/layouts/horizontal.py | 2 +- src/textual/layouts/vertical.py | 2 +- src/textual/widget.py | 2 + .../__snapshots__/test_snapshots.ambr | 160 ++++++++++++++++++ .../snapshot_apps/fr_with_min.py | 50 ++++++ tests/snapshot_tests/test_snapshots.py | 7 +- tests/test_resolve.py | 68 +++++++- 10 files changed, 426 insertions(+), 31 deletions(-) create mode 100644 tests/snapshot_tests/snapshot_apps/fr_with_min.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 83496f9f2..3db42cba3 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/src/textual/_resolve.py b/src/textual/_resolve.py index cc826a298..e9dedf961 100644 --- a/src/textual/_resolve.py +++ b/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": - total_remaining = sum([width for width, _, _ in filter(None, box_models)]) - remaining_space = max(0, size.width - total_remaining - margin_width) - else: - total_remaining = sum([height for _, height, _ in filter(None, box_models)]) - remaining_space = max(0, size.height - total_remaining - margin_height) + if None not in box_models: + # No fr units, so we're done + return cast("list[BoxModel]", box_models) - fraction_unit = Fraction( - remaining_space, - int( - sum( - [ - dimension.value - for dimension in dimensions - if dimension and dimension.is_fraction - ] - ) + # If all box models have been calculated + widget_styles = [widget.styles for widget in widgets] + if resolve_dimension == "width": + total_remaining = int( + sum([width for width, _, _ in filter(None, box_models)], start=Fraction()) + ) + remaining_space = int(max(0, size.width - total_remaining - margin_width)) + 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 diff --git a/src/textual/css/_style_properties.py b/src/textual/css/_style_properties.py index ff09e19eb..a69d94207 100644 --- a/src/textual/css/_style_properties.py +++ b/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" ), diff --git a/src/textual/layouts/horizontal.py b/src/textual/layouts/horizontal.py index 4455ea20c..5b5c8fdae 100644 --- a/src/textual/layouts/horizontal.py +++ b/src/textual/layouts/horizontal.py @@ -53,7 +53,7 @@ class HorizontalLayout(Layout): size, parent.app.size, resolve_margin, - dimension="width", + resolve_dimension="width", ) margins = [ diff --git a/src/textual/layouts/vertical.py b/src/textual/layouts/vertical.py index b498fd637..148c1854f 100644 --- a/src/textual/layouts/vertical.py +++ b/src/textual/layouts/vertical.py @@ -50,7 +50,7 @@ class VerticalLayout(Layout): size, parent.app.size, resolve_margin, - dimension="height", + resolve_dimension="height", ) margins = [ diff --git a/src/textual/widget.py b/src/textual/widget.py index 2e72c2380..dd8e6b7dc 100644 --- a/src/textual/widget.py +++ b/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 diff --git a/tests/snapshot_tests/__snapshots__/test_snapshots.ambr b/tests/snapshot_tests/__snapshots__/test_snapshots.ambr index 41651754e..d63cb210f 100644 --- a/tests/snapshot_tests/__snapshots__/test_snapshots.ambr +++ b/tests/snapshot_tests/__snapshots__/test_snapshots.ambr @@ -14891,6 +14891,166 @@ ''' # --- +# name: test_fr_unit_with_min + ''' + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + ScreenSplitApp + + + + + + + + + + ⭘ScreenSplitApp + This is content This is content number 0 + number 0This is content number 1 + This is content ▄▄This is content number 2 + number 1This is content number 3 + This is content This is content number 4▁▁ + number 2This is content number 5 + This is content This is content number 6 + number 3This is content number 7 + This is content This is content number 8 + number 4This is content number 9 + This is content This is content number 10 + number 5This is content number 11 + This is content This is content number 12 + number 6This is content number 13 + This is content This is content number 14 + number 7This is content number 15 + This is content This is content number 16 + number 8This is content number 17 + This is content This is content number 18 + number 9This is content number 19 + This is content This is content number 20 + number 10This is content number 21 + + + + + + ''' +# --- # name: test_fr_units ''' diff --git a/tests/snapshot_tests/snapshot_apps/fr_with_min.py b/tests/snapshot_tests/snapshot_apps/fr_with_min.py new file mode 100644 index 000000000..110cdd8b1 --- /dev/null +++ b/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() diff --git a/tests/snapshot_tests/test_snapshots.py b/tests/snapshot_tests/test_snapshots.py index 9d6bed698..7b3d3ff0f 100644 --- a/tests/snapshot_tests/test_snapshots.py +++ b/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") diff --git a/tests/test_resolve.py b/tests/test_resolve.py index 50b4a44b0..c36e1a406 100644 --- a/tests/test_resolve.py +++ b/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)