Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix for percentage dimensions #4037

Merged
merged 6 commits into from
Jan 31, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
- `SelectionList` option IDs are usable as soon as the widget is instantiated https://github.com/Textualize/textual/issues/3903
- Fix issue with `Strip.crop` when crop window start aligned with strip end https://github.com/Textualize/textual/pull/3998
- Fixed Strip.crop_extend https://github.com/Textualize/textual/pull/4011
- Fix for percentage dimensions https://github.com/Textualize/textual/pull/4037
- Fixed a crash if the `TextArea` language was set but tree-sitter language binaries were not installed https://github.com/Textualize/textual/issues/4045
- Ensuring `TextArea.SelectionChanged` message only sends when the updated selection is different https://github.com/Textualize/textual/pull/3933
- Fixed declaration after nested rule set causing a parse error https://github.com/Textualize/textual/pull/4012
Expand Down
1 change: 0 additions & 1 deletion src/textual/_arrange.py
Original file line number Diff line number Diff line change
Expand Up @@ -160,5 +160,4 @@ def _arrange_dock_widgets(
_WidgetPlacement(dock_region, null_spacing, dock_widget, top_z, True)
)
dock_spacing = Spacing(top, right, bottom, left)

return (placements, dock_spacing)
6 changes: 0 additions & 6 deletions src/textual/css/scalar.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,6 @@ class Unit(Enum):
AUTO = 8


UNIT_EXCLUDES_BORDER = {Unit.CELLS, Unit.FRACTION, Unit.VIEW_WIDTH, Unit.VIEW_HEIGHT}

UNIT_SYMBOL = {
Unit.CELLS: "",
Unit.FRACTION: "fr",
Expand Down Expand Up @@ -206,10 +204,6 @@ def is_fraction(self) -> bool:
"""Check if the unit is a fraction."""
return self.unit == Unit.FRACTION

@property
def excludes_border(self) -> bool:
return self.unit in UNIT_EXCLUDES_BORDER

@property
def cells(self) -> int | None:
"""Check if the unit is explicit cells."""
Expand Down
6 changes: 4 additions & 2 deletions src/textual/layouts/horizontal.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,9 @@ def arrange(
add_placement = placements.append

child_styles = [child.styles for child in children]
box_margins: list[Spacing] = [styles.margin for styles in child_styles]
box_margins: list[Spacing] = [
styles.margin for styles in child_styles if styles.overlay != "screen"
]
Comment on lines +29 to +31
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see you changed this because you probably thought of an edge case that wasn't being covered; wouldn't this warrant a test?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It probably should. Going to leave it for now, because overlay isn't yet documented.

if box_margins:
resolve_margin = Size(
sum(
Expand All @@ -36,7 +38,7 @@ def arrange(
]
)
+ (box_margins[0].left + box_margins[-1].right),
max(
min(
Comment on lines -39 to +41
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same comment re: test

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is covered by a snapshot, but it probably does deserve a more specific test. Going to leave it for now though.

[
margin_top + margin_bottom
for margin_top, _, margin_bottom, _ in box_margins
Expand Down
2 changes: 1 addition & 1 deletion src/textual/layouts/vertical.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ def arrange(
]
if box_margins:
resolve_margin = Size(
max(
min(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same comment re: test

[
margin_right + margin_left
for _, margin_right, _, margin_left in box_margins
Expand Down
22 changes: 9 additions & 13 deletions src/textual/widget.py
Original file line number Diff line number Diff line change
Expand Up @@ -1071,18 +1071,14 @@ def _get_box_model(

# Container minus padding and border
content_container = container - gutter.totals
# The container including the content
sizing_container = content_container if is_border_box else container

if styles.width is None:
# No width specified, fill available space
content_width = Fraction(content_container.width - margin.width)
elif is_auto_width:
# When width is auto, we want enough space to always fit the content
content_width = Fraction(
self.get_content_width(
content_container - styles.margin.totals, viewport
)
self.get_content_width(content_container - margin.totals, viewport)
)
if styles.scrollbar_gutter == "stable" and styles.overflow_x == "auto":
content_width += styles.scrollbar_size_vertical
Expand All @@ -1095,15 +1091,15 @@ def _get_box_model(
# An explicit width
styles_width = styles.width
content_width = styles_width.resolve(
sizing_container - styles.margin.totals, viewport, width_fraction
container - margin.totals, viewport, width_fraction
)
if is_border_box and styles_width.excludes_border:
if is_border_box:
content_width -= gutter.width

if styles.min_width is not None:
# Restrict to minimum width, if set
min_width = styles.min_width.resolve(
content_container, viewport, width_fraction
container - margin.totals, viewport, width_fraction
)
if is_border_box:
min_width -= gutter.width
Expand All @@ -1112,7 +1108,7 @@ def _get_box_model(
if styles.max_width is not None:
# Restrict to maximum width, if set
max_width = styles.max_width.resolve(
content_container, viewport, width_fraction
container - margin.totals, viewport, width_fraction
)
if is_border_box:
max_width -= gutter.width
Expand All @@ -1139,15 +1135,15 @@ def _get_box_model(
styles_height = styles.height
# Explicit height set
content_height = styles_height.resolve(
sizing_container - styles.margin.totals, viewport, height_fraction
container - margin.totals, viewport, height_fraction
)
if is_border_box and styles_height.excludes_border:
rodrigogiraoserrao marked this conversation as resolved.
Show resolved Hide resolved
if is_border_box:
content_height -= gutter.height

if styles.min_height is not None:
# Restrict to minimum height, if set
min_height = styles.min_height.resolve(
content_container, viewport, height_fraction
container - margin.totals, viewport, height_fraction
)
if is_border_box:
min_height -= gutter.height
Expand All @@ -1156,7 +1152,7 @@ def _get_box_model(
if styles.max_height is not None:
# Restrict maximum height, if set
max_height = styles.max_height.resolve(
content_container, viewport, height_fraction
container - margin.totals, viewport, height_fraction
)
if is_border_box:
max_height -= gutter.height
Expand Down
1,972 changes: 1,066 additions & 906 deletions tests/snapshot_tests/__snapshots__/test_snapshots.ambr

Large diffs are not rendered by default.

35 changes: 35 additions & 0 deletions tests/snapshot_tests/snapshot_apps/input_percentage_width.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
from textual.app import App, ComposeResult
from textual.widgets import Input, TextArea, Static, Button, Label


class InputVsTextArea(App[None]):
CSS = """
Screen > *, Screen > *:focus {
width: 50%;
height: 1fr;
border: solid red;
}
App #ruler {
width: 1fr;
height: 1;
border: none;
}
"""

def compose(self) -> ComposeResult:
yield Label("[reverse]0123456789[/]0123456789" * 4, id="ruler")

input = Input()
input.cursor_blink = False
yield input

text_area = TextArea()
text_area.cursor_blink = False
yield text_area

yield Static()
yield Button()


if __name__ == "__main__":
InputVsTextArea().run()
6 changes: 6 additions & 0 deletions tests/snapshot_tests/test_snapshots.py
Original file line number Diff line number Diff line change
Expand Up @@ -992,3 +992,9 @@ def test_nested_specificity(snap_compare):
def test_tab_rename(snap_compare):
"""Test setting a new label for a tab amongst a TabbedContent."""
assert snap_compare(SNAPSHOT_APPS_DIR / "tab_rename.py")


def test_input_percentage_width(snap_compare):
"""Check percentage widths work correctly."""
# https://github.com/Textualize/textual/issues/3721
assert snap_compare(SNAPSHOT_APPS_DIR / "input_percentage_width.py")
4 changes: 2 additions & 2 deletions tests/test_box_model.py
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ def get_content_height(self, container: Size, parent: Size, width: int) -> int:
styles.max_width = "50%"

box_model = widget._get_box_model(Size(60, 20), Size(80, 24), one, one)
assert box_model == BoxModel(Fraction(30), Fraction(16), Spacing(1, 2, 3, 4))
assert box_model == BoxModel(Fraction(27), Fraction(16), Spacing(1, 2, 3, 4))


def test_height():
Expand Down Expand Up @@ -141,7 +141,7 @@ def get_content_height(self, container: Size, parent: Size, width: int) -> int:
styles.max_height = "50%"

box_model = widget._get_box_model(Size(60, 20), Size(80, 24), one, one)
assert box_model == BoxModel(Fraction(54), Fraction(10), Spacing(1, 2, 3, 4))
assert box_model == BoxModel(Fraction(54), Fraction(8), Spacing(1, 2, 3, 4))


def test_max():
Expand Down
Loading