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

Additional containers #1957

Closed
willmcgugan opened this issue Mar 7, 2023 · 12 comments · Fixed by #2030
Closed

Additional containers #1957

willmcgugan opened this issue Mar 7, 2023 · 12 comments · Fixed by #2030
Assignees
Labels
enhancement New feature or request Task

Comments

@willmcgugan
Copy link
Collaborator

We need a few additions / modifications to containers.py

  • Center to horizontally align contents (width 100%, height auto)
  • Middle to vertically align contents.
  • Horizontal should have scrollbars disabled by default
  • Vertical should have scrollbars disabled by default
  • HorizontalScroll should have horizontal layout and scrollbars auto
  • VerticalScroll should have vertical layout and scrollbars auto
@willmcgugan willmcgugan changed the title Add container types Additional containers Mar 7, 2023
@darrenburns
Copy link
Member

There's some text about whether Horizontal and Vertical have scrollbars enabled or not in the "Layout" page of the guide. We'll need to make sure we update that page if we change the behaviour.

@rodrigogiraoserrao rodrigogiraoserrao added enhancement New feature or request Task labels Mar 8, 2023
@rodrigogiraoserrao rodrigogiraoserrao self-assigned this Mar 8, 2023
@rodrigogiraoserrao
Copy link
Contributor

Trying to think as someone who just found Textual, a container called Vertical seems to be the simplest vertical container there is and it seems perfectly reasonable to expect that container to have scrollbars by default.
In fact, the name Vertical only implies a specific layout and it does not imply a change to the value of overflow, which is auto by default.

Why don't we let Horizontal and Vertical have their scrollbars by default, and add something like HorizontalNoScroll and VerticalNoScroll instead?

@willmcgugan
Copy link
Collaborator Author

The problem is that people liberally add these containers, but they generally only want a single container to scroll.

If they mess up the styling, and everyone will first attempt, it can be very confusing when you get a scrollbar you don't want. Because you don't know which container really is scrolling.

I think its easier to design a UI if you have to explicitly say which container(s) should scroll.

@rodrigogiraoserrao
Copy link
Contributor

Then, should the default value of overflow be overflow: hidden hidden?

rodrigogiraoserrao added a commit that referenced this issue Mar 9, 2023
@rodrigogiraoserrao
Copy link
Contributor

@willmcgugan what non-overflowing version are you going for?

  1. don't overflow in either direction:
    • Horizontal and Vertical set overflow: hidden hidden.
  2. don't overflow in the direction of the layout:
    • Horizontal sets overflow-x: hidden; and
    • Vertical sets overflow-y: hidden.

@rodrigogiraoserrao
Copy link
Contributor

Also, do you literally mean for Center to align contents, or should it just align children?
(Ditto for Middle.)

@davep
Copy link
Contributor

davep commented Mar 9, 2023

(not answering for Will, of course) I think it would only make sense for it to act on the immediate children What's being spoken about here is sort of a follow on from this FAQ.

@willmcgugan
Copy link
Collaborator Author

@willmcgugan what non-overflowing version are you going for?

  1. don't overflow in either direction:

    • Horizontal and Vertical set overflow: hidden hidden.

This one.

Center should align children.

rodrigogiraoserrao added a commit that referenced this issue Mar 9, 2023
Related issues: #1957.
rodrigogiraoserrao added a commit that referenced this issue Mar 9, 2023
This container will centre children horizontally.

Related issues: #1957.
rodrigogiraoserrao added a commit that referenced this issue Mar 9, 2023
Related issues: #1957.
rodrigogiraoserrao added a commit that referenced this issue Mar 9, 2023
rodrigogiraoserrao added a commit that referenced this issue Mar 9, 2023
Related issues: #1957.
@rodrigogiraoserrao
Copy link
Contributor

@willmcgugan I think it is more intuitive if the default styling of Center/Middle is to have their relevant dimension be 1fr instead of 100% to fill all the space available instead of being forcefully 100% of the container.

Let me know your decision and then I am ready to PR this.

@willmcgugan
Copy link
Collaborator Author

Leave the Center / Middle dimensions as blank, as it will compensate for margin.

@rodrigogiraoserrao rodrigogiraoserrao linked a pull request Mar 13, 2023 that will close this issue
3 tasks
@rodrigogiraoserrao
Copy link
Contributor

After a quick chat, we agreed we'd set the relevant dimension to 1fr instead of 100% or leaving it unset.

@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
enhancement New feature or request Task
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants