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

Grid columns with multiple units resolves percent unit to wrong value #1406

Closed
rodrigogiraoserrao opened this issue Dec 19, 2022 · 4 comments · Fixed by #1640
Closed

Grid columns with multiple units resolves percent unit to wrong value #1406

rodrigogiraoserrao opened this issue Dec 19, 2022 · 4 comments · Fixed by #1640
Assignees
Labels
bug Something isn't working Task

Comments

@rodrigogiraoserrao
Copy link
Contributor

In a grid layout with mixed units in the style grid-columns, the column associated with 25% gets the wrong size.
However, using 25w or 25vw instead of 25% produces the correct result.

Here is the screenshot of the app where the middle column should take up 25% of the total width and the 1st and 4th columns should accommodate that:

Screenshot at Dec 19 17-30-02

grid_columns.py
from textual.app import App
from textual.containers import Grid
from textual.widgets import Label


class MyApp(App):
    def compose(self):
        yield Grid(
            Label("1fr"),
            Label("width = 16"),
            Label("middle"),
            Label("1fr"),
            Label("width = 16"),
        )


app = MyApp(css_path="grid_columns.css")
grid_columns.css
Grid {
    grid-size: 5 1;
    grid-columns: 1fr 16 25%;
}

Label {
    border: round white;
    content-align-horizontal: center;
    width: 100%;
    height: 100%;
}

Try running the app with textual run --dev grid_columns.css and change the value 25% to 25w or 25vw and notice it gets set to the correct value.

@rodrigogiraoserrao rodrigogiraoserrao added bug Something isn't working Task labels Jan 20, 2023
@Textualize Textualize deleted a comment from github-actions bot Jan 20, 2023
@rodrigogiraoserrao
Copy link
Contributor Author

A similar application containing a Horizontal layout and labels shows this might be a bug with grid-columns or the way grid columns are computed and not with the way units are resolved (the app below shows the labels correctly sized):

# relative_sizes.py
from textual.app import App
from textual.containers import Horizontal
from textual.widgets import Label


class MyApp(App):
    def compose(self):
        yield Horizontal(
            Label("1fr", classes="fr"),
            Label("width = 16", classes="fixed"),
            Label("middle", classes="percent"),
            Label("1fr", classes="fr"),
            Label("width = 16", classes="fixed"),
        )


app = MyApp(css_path="relative_sizes.css")
/* relative_sizes.css */
Label {
    border: round white;
    content-align-horizontal: center;
    height: 100%;
}

Label.fr {
    width: 1fr;
}

Label.fixed {
    width: 16;
}

Label.percent {
    width: 25%;
}

@rodrigogiraoserrao
Copy link
Contributor Author

rodrigogiraoserrao commented Jan 23, 2023

Printing the values of row_scalars and column_scalars at the very beginning of GridLayout.arrange in

column_scalars = styles.grid_columns or [Scalar.parse("1fr")]

shows that the unit % is relative to the terminal height instead of the terminal width, which is why the column isn't as wide as it should be:

[10:28:32] PRINT                                                      grid.py:27
(Scalar(value=1.0, unit=<Unit.FRACTION: 2>, percent_unit=<Unit.HEIGHT: 5>),
Scalar(value=16.0, unit=<Unit.CELLS: 1>, percent_unit=<Unit.WIDTH: 4>),
Scalar(value=25.0, unit=<Unit.PERCENT: 3>, percent_unit=<Unit.HEIGHT: 5>))

The screen recording below proves this, as making the terminal taller makes the middle column wider:

Screen.Recording.2023-01-23.at.10.32.37.mov

@rodrigogiraoserrao
Copy link
Contributor Author

Investigating this issue showed that there were two bugs related to the relative dimension that the scalars in grid-rows and grid-columns were pointing to.

On the one hand, the method StylesBuilder._process_grid_rows_or_columns had a bug in its if statement:

percent_unit=Unit.WIDTH if name == "rows" else Unit.HEIGHT,

Not only are the full names "grid-rows" and "grid-columns", but this relationship is reversed.
So, because this equality check would always evaluate to False, scalars in grid-rows/grid-columns that were being parsed from CSS files would always be relative to the height of the app.

On the other hand, ScalarListProperty is the property that is used for grid-rows/grid-columns and they are not aware of their percent_unit, so when there is a programmatic assignment, relative units will always be relative to the width, because that is the (current) default for Scalar.

@github-actions
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
bug Something isn't working Task
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant