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

Margin-right does not work properly when RadioSet is present on the screen. #4332

Closed
jakubziebin opened this issue Mar 25, 2024 · 12 comments · Fixed by #4441
Closed

Margin-right does not work properly when RadioSet is present on the screen. #4332

jakubziebin opened this issue Mar 25, 2024 · 12 comments · Fixed by #4441

Comments

@jakubziebin
Copy link

I have observed strange margin-right behavior when RadioSet is present on the screen. Consider an example like this:

from textual.app import App, ComposeResult
from textual.containers import Grid, ScrollableContainer
from textual.widgets import TabbedContent, TabPane, Input, RadioSet, RadioButton


class MyTabPane(TabPane):
    DEFAULT_CSS = """
    Grid {
        grid-rows: auto;
        width: 1fr;
        grid-gutter: 1;
        background: $panel;
        align: center middle;
        margin-right: 20;
    }
    """

    def compose(self) -> ComposeResult:
        with ScrollableContainer():
            with RadioSet():
                yield RadioButton("test_button_1")
                yield RadioButton("test_button_2")
            with Grid():
                yield Input()
                yield Input()


class MyApp(App):
    def compose(self) -> ComposeResult:
        with TabbedContent():
            yield MyTabPane("TEST")


MyApp().run()

When running the application, the entire margin will be placed outside the screen content and will be accessible by scrolling. When removing ScrollableContainer the margin will not be visible. I have not observed similar situations without RadioSet.

@Textualize Textualize deleted a comment from github-actions bot Mar 25, 2024
@davep
Copy link
Contributor

davep commented Mar 25, 2024

I've not dived onto the why of this yet, but this doesn't seem to be anything to do with RadioSet in particular. The same effect can be achieved with Label or Button, for example.

@TomJGooding
Copy link
Contributor

TomJGooding commented Mar 26, 2024

This looks to me like the effect of a widget with 100% width (i.e. the Input) inside a container with a width of 1fr?

Here's a slighlly reduced example. Compare before and after uncommenting the CSS on line 16:

from textual.containers import Container, ScrollableContainer
from textual.widgets import Input, Button

from textual.app import App, ComposeResult


class ExampleApp(App):
    CSS = """
    #outer {
        border: solid red;
    }

    #inner {
        margin-right: 20;
        border: solid blue;
        /* width: 100%; */
    }
"""

    def compose(self) -> ComposeResult:
        with ScrollableContainer(id="outer"):
            yield Button()
            with Container(id="inner"):
                yield Input()


if __name__ == "__main__":
    app = ExampleApp()
    app.run()

@TomJGooding
Copy link
Contributor

TomJGooding commented Mar 26, 2024

It took me a minute to figure out why changing the width then wasn't working in your example above, but it looks like your CSS was missing the parent selector like this:

class MyTabPane(TabPane):
    DEFAULT_CSS = """
    MyTabPane Grid {

I confess I don't understand why the rest of the CSS still seems to work though! I'm still not caught up on nested CSS so not sure if this might have revealed some sort of bug...?

@jakubziebin
Copy link
Author

Honestly it looks like a workaround to me..., because what is the reason why width: 1fr causes the margin-right to not work properly? IMO it's a common case that a widget with width: 100% is mounted inside a container with width: 1fr and I don't see any reason why this margin would not work in this case. Please correct me if I am wrong.

@davep
Copy link
Contributor

davep commented Mar 29, 2024

I wonder if this is actually another instance of #4360.

@jakubziebin
Copy link
Author

Yes, thanks for your help! I look forward to the solution.

@jakubziebin
Copy link
Author

IMO, worth knowing, we observed this problem on version 0.52.1.

@davep
Copy link
Contributor

davep commented Mar 29, 2024

Ahh, okay, likely not related then.

@TomJGooding
Copy link
Contributor

TomJGooding commented Mar 29, 2024

Just to clarify, there was a change in this margin behaviour but back in v0.48.0 following #4037.

@jakubziebin I was only trying to clarify the likely cause with a MRE (as actually nothing to do with a RadioSet). I'm not saying this behaviour is correct or incorrect (and as you'll see from the discussion in the PR above, margin behaviour is a bit of a contentious issue!)

@jakubziebin
Copy link
Author

I encountered one more strange behavior of the margin:

from textual.app import App, ComposeResult
from textual.widgets import Static, Input, Button
from textual.containers import Container


class MyContainer(Container):
    DEFAULT_CSS = """
    MyContainer {
        height: auto;
        #content {
            height: auto;
            background: $panel;
        }
    
        #container-with-button {
            align: center middle;
            height: auto;
        }
        
        Input {
            margin-bottom: 1;
        }
    }
    """

    def compose(self) -> ComposeResult:
        with Container(id="content"):
            yield Static("Example static")
            yield Input()
            with Container(id="container-with-button"):
                yield Button("Click me")


class MyApp(App):
    def compose(self) -> ComposeResult:
        yield MyContainer()


MyApp().run()

Part of the button is cut off because of the bottom margin of the input. I wasn't sure if this should be posted in another issue, but I thought it was somewhat related, let me know if it should be converted to another issue.

@TomJGooding
Copy link
Contributor

TomJGooding commented Apr 2, 2024

Part of the button is cut off because of the bottom margin of the input.

I don't think it is the margin. If you try deleting the CSS for the Input from your example, you'll see the button is still cut off.

Looks to me something about the middle alignment throwing off the sizing.

EDIT: Interestingly, if you add a second Button all seems to work as expected.

from textual.app import App, ComposeResult
from textual.containers import Container
from textual.widgets import Button


class ExampleApp(App):
    CSS = """
    #outer-container {
        height: auto;
        border: solid red;
    }

    #btn-container {
        /* align: center middle; */
        height: auto;
        border: solid blue;
    }
    """

    def compose(self) -> ComposeResult:
        with Container(id="outer-container"):
            with Container(id="btn-container"):
                yield Button()
                # yield Button()


if __name__ == "__main__":
    ExampleApp().run()

Copy link

Don't forget to star the repository!

Follow @textualizeio for Textual updates.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants