`DataTable` height maybe doesn't account for horizontal scrollbar?

[TomJGooding]

I haven't yet started digging into this, but I discovered a possible bug with the DataTable height when there is a horizontal scrollbar. If you run my quick example app below, the row is obscured by the scrollbar:

from textual.app import App, ComposeResult
from textual.widgets import DataTable

LORUM_IPSUM = "Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua."

class ExampleApp(App):
    def compose(self) -> ComposeResult:
        yield DataTable()

    def on_mount(self) -> None:
        table = self.query_one(DataTable)
        table.add_column("Column 1")
        table.add_column("Column 2")
        table.add_row(LORUM_IPSUM, LORUM_IPSUM)

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

[github-actions[bot]]

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

• How do I center a widget in a screen?

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

[ihabunek]

Can confirm this one, Textual 0.73.0.

E.g. here's a data without any scroll bars

If I resize the window by decreasing its width until horizontal scroll bar appears, then vertical scroll bar appears as well, even though there's enough room for the full table height:

[TomJGooding]

This looks like possibly an issue with how the base ScrollView handles the size with the scrollbars, rather than just the DataTable widget.

Here's another example with a Log widget where the height is set to auto:

Example Code

```python from textual.app import App, ComposeResult from textual.widgets import Footer, Log LORUM_IPSUM = "Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua." class ExampleApp(App): BINDINGS = [("space", "write_line", "Write Line")] CSS = """ Log { overflow: auto; height: auto; max-height: 1fr; border: solid $accent; } """ def compose(self) -> ComposeResult: yield Log(auto_scroll=False) yield Footer() def on_mount(self) -> None: log = self.query_one(Log) log.border_title = f"{log.line_count} lines" def action_write_line(self) -> None: log = self.query_one(Log) log.write_line(f"{log.line_count + 1}: {LORUM_IPSUM}") log.border_title = f"{log.line_count} lines" if __name__ == "__main__": app = ExampleApp() app.run() ```

This probably isn't the correct fix, but this simple change to ScrollView.get_content_height seems to work:

-        return self.virtual_size.height
+        return self.virtual_size.height + self.scrollbar_size_horizontal

[darrenburns]

@TomJGooding I think that may actually be the fix. It's probably a fair assumption that when height: auto in a ScrollView we don't want scrollbars to sit on top of content, so we need to add that extra leeway of + self.scrollbar_size_horizontal to make room for it in the content area. I'm guessing the same change would be required for the scrollbar on the other axis too.

@willmcgugan any thoughts?

[havranej]

@darrenburns Can confirm that this is an issue with the vertical scrollbar as well. Additionally, the guide makes it seem that the width of the scrollbar is accounted for – namely the two lines here

        # Crop the strip so that is covers the visible area
        strip = strip.crop(scroll_x, scroll_x + self.size.width)

[willmcgugan]

The PR above implements those changes. Not to get_content_width, but at a slightly lower level.

It fixes the MRE, and no tests broke!

correction one test broke, but it made it better!

[ihabunek]

@willmcgugan I can confirm this fixes the issue in my use case. Thanks!

[github-actions[bot]]

Don't forget to star the repository!

Follow @textualizeio for Textual updates.