-
Notifications
You must be signed in to change notification settings - Fork 814
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
Changes from 4 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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" | ||
] | ||
if box_margins: | ||
resolve_margin = Size( | ||
sum( | ||
|
@@ -36,7 +38,7 @@ def arrange( | |
] | ||
) | ||
+ (box_margins[0].left + box_margins[-1].right), | ||
max( | ||
min( | ||
Comment on lines
-39
to
+41
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same comment re: test There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -29,7 +29,7 @@ def arrange( | |
] | ||
if box_margins: | ||
resolve_margin = Size( | ||
max( | ||
min( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 | ||
|
Large diffs are not rendered by default.
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() |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.