From b36ee03dc9b8afbc8ca0e3f4f730d44130231f46 Mon Sep 17 00:00:00 2001 From: Will McGugan Date: Mon, 14 Oct 2024 16:57:46 +0100 Subject: [PATCH 1/6] fix fr resolve --- src/textual/_resolve.py | 15 ++++++++--- tests/snapshot_tests/test_snapshots.py | 35 +++++++++++++++++++++++++- 2 files changed, 45 insertions(+), 5 deletions(-) diff --git a/src/textual/_resolve.py b/src/textual/_resolve.py index 7fbd4751d6..e6d7d25b1b 100644 --- a/src/textual/_resolve.py +++ b/src/textual/_resolve.py @@ -196,10 +196,14 @@ def resolve_box_models( Returns: List of resolved box models. """ + + margins = [widget.styles.margin for widget in widgets] + margin_width, margin_height = margin - fraction_width = Fraction(max(0, size.width - margin_width)) - fraction_height = Fraction(max(0, size.height - margin_height)) + fraction_width = Fraction(size.width) + fraction_height = Fraction(size.height) + fraction_zero = Fraction(0) margin_size = size - margin @@ -209,10 +213,13 @@ def resolve_box_models( None if _dimension is not None and _dimension.is_fraction else widget._get_box_model( - size, viewport_size, fraction_width, fraction_height + size, + viewport_size, + max(fraction_zero, fraction_width - margin.width), + max(fraction_zero, fraction_height - margin.height), ) ) - for (_dimension, widget) in zip(dimensions, widgets) + for (_dimension, widget, margin) in zip(dimensions, widgets, margins) ] if None not in box_models: diff --git a/tests/snapshot_tests/test_snapshots.py b/tests/snapshot_tests/test_snapshots.py index dca33964cf..cd53ab7b31 100644 --- a/tests/snapshot_tests/test_snapshots.py +++ b/tests/snapshot_tests/test_snapshots.py @@ -8,7 +8,7 @@ from textual import events, on from textual.app import App, ComposeResult from textual.binding import Binding, Keymap -from textual.containers import Center, Grid, Middle, Vertical, VerticalScroll +from textual.containers import Center, Container, Grid, Middle, Vertical, VerticalScroll from textual.pilot import Pilot from textual.renderables.gradient import LinearGradient from textual.screen import ModalScreen, Screen @@ -2336,3 +2336,36 @@ def compose(self) -> ComposeResult: yield Label("100%") assert snap_compare(BackgroundTintApp()) + + +def test_fr_and_margin(snap_compare): + class FRApp(App): + CSS = """ + #first-container { + background: green; + height: auto; + } + + #second-container { + margin: 2; + background: red; + height: auto; + } + + #third-container { + margin: 4; + background: blue; + } + """ + + def compose(self) -> ComposeResult: + with Container(id="first-container"): + yield Label("No margin - should extend to left and right") + + with Container(id="second-container"): + yield Label("A margin of 2, should be 2 cells around the text") + + with Center(id="third-container"): + yield Label("A margin of 4, should be 4 cells around the text") + + assert snap_compare(FRApp()) From c1673389244e9f6f6d3cbc17f27d33a81041a5ba Mon Sep 17 00:00:00 2001 From: Will McGugan Date: Mon, 14 Oct 2024 17:00:10 +0100 Subject: [PATCH 2/6] snapshot --- tests/snapshot_tests/test_snapshots.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tests/snapshot_tests/test_snapshots.py b/tests/snapshot_tests/test_snapshots.py index cd53ab7b31..a8a35dfeb0 100644 --- a/tests/snapshot_tests/test_snapshots.py +++ b/tests/snapshot_tests/test_snapshots.py @@ -2339,6 +2339,10 @@ def compose(self) -> ComposeResult: def test_fr_and_margin(snap_compare): + """Regression test for https://github.com/Textualize/textual/issues/5116""" + + # Check margins can be independently applied to widgets with fr unites + class FRApp(App): CSS = """ #first-container { From f6f695facc2bb097a062d56dc651797763d8b1ba Mon Sep 17 00:00:00 2001 From: Will McGugan Date: Mon, 14 Oct 2024 17:02:29 +0100 Subject: [PATCH 3/6] snapshot --- .../test_snapshots/test_fr_and_margin.svg | 153 ++++++++++++++++++ 1 file changed, 153 insertions(+) create mode 100644 tests/snapshot_tests/__snapshots__/test_snapshots/test_fr_and_margin.svg diff --git a/tests/snapshot_tests/__snapshots__/test_snapshots/test_fr_and_margin.svg b/tests/snapshot_tests/__snapshots__/test_snapshots/test_fr_and_margin.svg new file mode 100644 index 0000000000..f7ec88b079 --- /dev/null +++ b/tests/snapshot_tests/__snapshots__/test_snapshots/test_fr_and_margin.svg @@ -0,0 +1,153 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + FRApp + + + + + + + + + + No margin - should extend to left and right                                      + + +A margin of 2, should be 2 cells around the text                             + + + + +            A margin of 4, should be 4 cells around the text             + + + + + + + + + + + + + + + + + + From 41441df0db2a10732334f162c5cdaf1ff2716d05 Mon Sep 17 00:00:00 2001 From: Will McGugan Date: Mon, 14 Oct 2024 17:15:30 +0100 Subject: [PATCH 4/6] simplify --- src/textual/_resolve.py | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/src/textual/_resolve.py b/src/textual/_resolve.py index e6d7d25b1b..1bd05ae1fb 100644 --- a/src/textual/_resolve.py +++ b/src/textual/_resolve.py @@ -197,14 +197,10 @@ def resolve_box_models( List of resolved box models. """ - margins = [widget.styles.margin for widget in widgets] - margin_width, margin_height = margin - fraction_width = Fraction(size.width) fraction_height = Fraction(size.height) fraction_zero = Fraction(0) - margin_size = size - margin # Fixed box models @@ -215,11 +211,11 @@ def resolve_box_models( else widget._get_box_model( size, viewport_size, - max(fraction_zero, fraction_width - margin.width), - max(fraction_zero, fraction_height - margin.height), + max(fraction_zero, fraction_width - widget.styles.margin.width), + max(fraction_zero, fraction_height - widget.styles.margin.height), ) ) - for (_dimension, widget, margin) in zip(dimensions, widgets, margins) + for (_dimension, widget) in zip(dimensions, widgets) ] if None not in box_models: From bcaad58926a3dbb594b62d7db3c7018a55b1686e Mon Sep 17 00:00:00 2001 From: Will McGugan Date: Mon, 14 Oct 2024 17:19:52 +0100 Subject: [PATCH 5/6] simpler test --- .../test_snapshots/test_fr_and_margin.svg | 114 +++++++++--------- tests/snapshot_tests/test_snapshots.py | 3 +- 2 files changed, 59 insertions(+), 58 deletions(-) diff --git a/tests/snapshot_tests/__snapshots__/test_snapshots/test_fr_and_margin.svg b/tests/snapshot_tests/__snapshots__/test_snapshots/test_fr_and_margin.svg index f7ec88b079..82abecfc79 100644 --- a/tests/snapshot_tests/__snapshots__/test_snapshots/test_fr_and_margin.svg +++ b/tests/snapshot_tests/__snapshots__/test_snapshots/test_fr_and_margin.svg @@ -19,134 +19,134 @@ font-weight: 700; } - .terminal-3157579860-matrix { + .terminal-952358996-matrix { font-family: Fira Code, monospace; font-size: 20px; line-height: 24.4px; font-variant-east-asian: full-width; } - .terminal-3157579860-title { + .terminal-952358996-title { font-size: 18px; font-weight: bold; font-family: arial; } - .terminal-3157579860-r1 { fill: #ddeedd } -.terminal-3157579860-r2 { fill: #c5c8c6 } -.terminal-3157579860-r3 { fill: #e1e1e1 } -.terminal-3157579860-r4 { fill: #ffdddd } -.terminal-3157579860-r5 { fill: #ddddff } + .terminal-952358996-r1 { fill: #ddeedd } +.terminal-952358996-r2 { fill: #c5c8c6 } +.terminal-952358996-r3 { fill: #e1e1e1 } +.terminal-952358996-r4 { fill: #ffdddd } +.terminal-952358996-r5 { fill: #ddddff } - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - FRApp + FRApp - + - - No margin - should extend to left and right                                      - - -A margin of 2, should be 2 cells around the text                             - - - - -            A margin of 4, should be 4 cells around the text             - - - - - - - - - - - - - - + + No margin - should extend to left and right                                      + + +A margin of 2, should be 2 cells around the text                             + + + + +A margin of 4, should be 4 cells around the text                         + + + + + + + + + + + + + + diff --git a/tests/snapshot_tests/test_snapshots.py b/tests/snapshot_tests/test_snapshots.py index a8a35dfeb0..8ec68c0414 100644 --- a/tests/snapshot_tests/test_snapshots.py +++ b/tests/snapshot_tests/test_snapshots.py @@ -2359,6 +2359,7 @@ class FRApp(App): #third-container { margin: 4; background: blue; + height: auto; } """ @@ -2369,7 +2370,7 @@ def compose(self) -> ComposeResult: with Container(id="second-container"): yield Label("A margin of 2, should be 2 cells around the text") - with Center(id="third-container"): + with Container(id="third-container"): yield Label("A margin of 4, should be 4 cells around the text") assert snap_compare(FRApp()) From 11122f9a8cde2c21914dc4c69e1958c23a778cb0 Mon Sep 17 00:00:00 2001 From: Will McGugan Date: Mon, 14 Oct 2024 17:35:34 +0100 Subject: [PATCH 6/6] optimize --- src/textual/_resolve.py | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/textual/_resolve.py b/src/textual/_resolve.py index 1bd05ae1fb..dce4663281 100644 --- a/src/textual/_resolve.py +++ b/src/textual/_resolve.py @@ -203,6 +203,8 @@ def resolve_box_models( fraction_zero = Fraction(0) margin_size = size - margin + margins = [widget.styles.margin.totals for widget in widgets] + # Fixed box models box_models: list[BoxModel | None] = [ ( @@ -211,11 +213,13 @@ def resolve_box_models( else widget._get_box_model( size, viewport_size, - max(fraction_zero, fraction_width - widget.styles.margin.width), - max(fraction_zero, fraction_height - widget.styles.margin.height), + max(fraction_zero, fraction_width - margin_width), + max(fraction_zero, fraction_height - margin_height), ) ) - for (_dimension, widget) in zip(dimensions, widgets) + for (_dimension, widget, (margin_width, margin_height)) in zip( + dimensions, widgets, margins + ) ] if None not in box_models: