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

Off-by-one error in data table height when a horizontal scrollbar is needed #3556

Closed
godlygeek opened this issue Oct 18, 2023 · 12 comments
Closed

Comments

@godlygeek
Copy link

When the terminal is tall enough to display all rows of a DataTable, but not wide enough to display all columns, the table chooses a height for itself that's one row too short.

For example, try resizing your terminal to 45 columns by 24 rows, and then run python docs/examples/widgets/data_table.py, and you'll see something like:

image

If you hit PageDn to scroll that, you'll see just one more line:

image

It seems like the data table is choosing to only use 10 lines out of the available 24 because it only has 10 rows worth of data to display, then it decides it needs a horizontal scrollbar, and suddenly it's got 10 rows of data and only 9 remaining rows to display them in, so it winds up adding a vertical scrollbar as well.

This seems unexpected to me. I don't think there should be a vertical scrollbar when the widget has plenty of room to grow vertically.

Textual Diagnostics

Versions

Name Value
Textual 0.40.0
Rich 13.6.0

Python

Name Value
Version 3.11.5
Implementation CPython
Compiler GCC 10.2.1 20210130 (Red Hat 10.2.1-11)
Executable /home/mwoznisk/.pyenv/versions/3.11/envs/textual/bin/python3

Operating System

Name Value
System Linux
Release 5.15.90.1-microsoft-standard-WSL2
Version #1 SMP Fri Jan 27 02:56:13 UTC 2023

Terminal

Name Value
Terminal Application Unknown
TERM putty-256color
COLORTERM truecolor
FORCE_COLOR Not set
NO_COLOR Not set

Rich Console options

Name Value
size width=239, height=64
legacy_windows False
min_width 1
max_width 239
is_terminal True
encoding utf-8
max_height 64
justify None
overflow None
no_wrap False
highlight None
markup None
height None
@github-actions
Copy link

We found the following entries in the FAQ which you may find helpful:

Feel free to close this issue if you found an answer in the FAQ. Otherwise, please give us a little time to review.

This is an automated reply, generated by FAQtory

@godlygeek
Copy link
Author

Gah, I missed #1289 when searching for this (I was only looking at open issues, not closed ones, since I was testing against main). Well - I understand why you don't like the idea of the data table growing when scrollbars are shown. I think the current behavior is surprising and unintuitive, but I can understand why having the data table grow or shrink by 1 row when the terminal gets narrower or wider might be surprising, too.

@godlygeek
Copy link
Author

Alright, I guess this is wontfix.

@github-actions
Copy link

Don't forget to star the repository!

Follow @textualizeio for Textual updates.

@godlygeek
Copy link
Author

Actually - reopening this. #1289 (comment) suggests a workaround - setting the scrollbar to always visible. But, as far as I can tell, that doesn't actually work around the issue. I see the same problem even if I add:

table.styles.overflow_x = "scroll"

to the on_mount of docs/examples/widgets/data_table.py.

Am I doing something wrong?

@godlygeek godlygeek reopened this Oct 18, 2023
@willmcgugan
Copy link
Collaborator

When the scrollbars appear they don't adjust the height of the widget. So when you get a horizontal bar it takes up one line of the widget, and forces a vertical scrollbar.

The solution is scrollbar-gutter: stable, which reserves space for scrollbars regardless if they are visible:

In CSS:

    DataTable {
        scrollbar-gutter: stable;        
    }

You may be thinking why not adjust the auto height when a scrollbar is visible. And I did to, but that makes it impossible to create consistent layouts. And you can get in to a situation where a scrollbar pops in, forces a layout, which makes another scrollbar disappear, and forces another layout. And the whole UI flickers as a result.

@godlygeek
Copy link
Author

I tried scrollbar-gutter: stable; but it didn't help. It's possible I got something wrong while testing, but it's documented as only saving space for a vertical scrollbar, not a horizonal one, and in my testing that seemed to be the case.

@willmcgugan
Copy link
Collaborator

Works for me. Can you share your code?

@godlygeek
Copy link
Author

Not at the moment, but I'll retest when I get a chance and get back to you.

@godlygeek
Copy link
Author

You're right, that works fine. Editing the docs/examples/widgets/data_table.py to do:

class TableApp(App):
    DEFAULT_CSS = """
        DataTable {
            scrollbar-gutter: stable;
        }
    """
    ...

does eliminate that vertical scrollbar. No idea what I did wrong the first time, but that's good enough for me!

The docs should probably be fixed to indicate that scrollbar-gutter: stable saves space for both the horizontal and vertical scrollbar, though - https://textual.textualize.io/styles/scrollbar_gutter/ currently only mentions "vertical", repeatedly.

Would you like a PR that basically just replaces "a vertical scrollbar" with "scrollbars" in those docs?

@willmcgugan
Copy link
Collaborator

A doc PR would be very welcome!

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

No branches or pull requests

2 participants