textual
textual copied to clipboard
A Widget with {margin: 1} style in a ScrollableContainer causes infinite resizes and scrollbars flicker
When presenting a ScrollableContainer with a Widget that has styles above in it, there will have a infinite resizes. Example:
from textual.containers import ScrollableContainer
from textual.widget import Widget
from textual.widgets import Static
class SuspiciousWidget(Widget):
DEFAULT_CSS = 'SuspiciousWidget {margin: 1}'
class A(App):
def compose(self) -> ComposeResult:
with ScrollableContainer():
yield Static()
yield SuspiciousWidget()
A().run()
Screen Record:
'textual diagnose' result:
Textual Diagnostics
Versions
Name | Value |
---|---|
Textual | 0.50.0 |
Rich | 13.7.0 |
Python
Name | Value |
---|---|
Version | 3.11.6 |
Implementation | CPython |
Compiler | GCC 13.2.1 20230801 |
Executable | /home/labo/prog/NCSbox/venv/bin/python |
Operating System
Name | Value |
---|---|
System | Linux |
Release | 6.7.4-zen1-1-zen |
Version | (HASH)1 ZEN SMP PREEMPT_DYNAMIC Mon, 05 Feb 2024 22:07:37 +0000 |
Terminal
Name | Value |
---|---|
Terminal Application | Unknown |
TERM | xterm-256color |
COLORTERM | truecolor |
FORCE_COLOR | Not set |
NO_COLOR | Not set |
Rich Console options
Name | Value |
---|---|
size | width=75, height=27 |
legacy_windows | False |
min_width | 1 |
max_width | 75 |
is_terminal | True |
encoding | utf-8 |
max_height | 27 |
justify | None |
overflow | None |
no_wrap | False |
highlight | None |
markup | None |
height | None |
By the way, this problem happened since Textual 0.48.0. Versions below it are OK.
I found the scrollbars flicker even if you remove the margin - unfortunately this might be a bug introduced with #4037?
The underlying error actually seems to go way back.
Take this app:
from textual.app import App, ComposeResult
from textual.widget import Widget
class A(App[None]):
CSS = """
Screen {
overflow: auto auto;
}
#one {
height: 1;
}
#two {
height: 100%;
}
"""
def compose(self) -> ComposeResult:
yield Widget(id="one")
yield Widget(id="two")
if __name__ == "__main__":
A().run()
At v0.19.0 this app doesn't generate this flickering error but it does at v0.19.1.
The source of the issue was commit 4f7b2d0.
Reverting that back now reveals two failed tests:
FAILED tests/snapshot_tests/test_snapshots.py::test_css_property[min_width.py] - AssertionError: assert False
FAILED tests/snapshot_tests/test_snapshots.py::test_auto_table - AssertionError: assert False
It looks like the reason those two tests fail is that some unnecessary scrollbar gutters stick around on app startup.
Ok, I've banged my head against the wall for a while and now I understand what the source of the issue is / what needs to be done.
Consider the app below and suppose the terminal size is 80 x 24.
from textual.app import App, ComposeResult
from textual.widget import Widget
class A(App[None]):
CSS = """
Screen {
overflow: auto auto;
}
#one {
height: 1;
}
#two {
height: 100%;
}
"""
def compose(self) -> ComposeResult:
yield Widget(id="one")
yield Widget(id="two")
if __name__ == "__main__":
A().run()
When the two widgets are mounted the screen is reflowed and we eventually end up at Widget._refresh_scrollbars
.
At this time, #one
is set to have a height of 1 and #two
is set to have a height of 24.
Because the heights of these two are greater than 24, which is the virtual height, we determine that we will (rightfully) need a vertical scrollbar:
https://github.com/Textualize/textual/blob/c7370f33203497d15ce65528d6f92ffbc58200a0/src/textual/widget.py#L1463-L1464
A bit further down, we have a second check to see if we need a horizontal scrollbar. Before, the widths of the widgets were at 80 (100% of the container) which fits perfectly, so we didn't need a scrollbar. But now, we do a second check because the horizontal space just decreased a bit. This will make it look like there is need for a horizontal scrollbar:
https://github.com/Textualize/textual/blob/c7370f33203497d15ce65528d6f92ffbc58200a0/src/textual/widget.py#L1467-L1470
However, this isn't needed because the children of this container have relative sizes and as soon as the vertical scrollbar gets added, they will be resized to take up less horizontal space. Therefore, a possible fix is to change the check shown above and make sure we only show the horizontal if the children cannot be resized to fit within the slightly reduced space.
However, this isn't all.
Suppose that we add width: 30
to our two widgets.
Now, their widths are way under the 80 terminal width but the flickering will still occur.
Why?
Again, when we get to _refresh_scrollbars
, we'll have that the virtual size of the screen is 80 x 24.
The heights will be too big and so we'll need a vertical scrollbar.
Then, in the second check for the horizontal scrollbar, we'll have the same situation because the virtual width of the screen will be 80.
In this case, we won't need a horizontal scrollbar because the children are short enough and because it's only the virtual width of the screen itself that is making it look like we're running out of space, when the virtual width of the screen will be happy to go down to 68 to accomodate the scrollbars.
Given the above, it looks like the fix is to go through the children of the widget that is having its scrollbars refreshed and checking if any of them is:
- exactly the size of the dimension we're looking at; and
- if that dimension can be updated automatically.
A rough implementation of this (that probably doesn't handle edge cases) would be to replace
https://github.com/Textualize/textual/blob/c7370f33203497d15ce65528d6f92ffbc58200a0/src/textual/widget.py#L1467-L1470
with something like
if overflow_x == "auto" and show_vertical and not show_horizontal:
show_horizontal = any(
child.size.width == width
and child.styles.width
and (child.styles.width.is_cells or child.styles.width.is_auto)
for child in self.children
)
Running the tests shows a single failing test that has to do with RichLog
.
Rich logs have no children and whether they get scrollbars or not depends on the content written to the rich log.
Adding an extra checks seems to work:
if overflow_x == "auto" and show_vertical and not show_horizontal:
if self.children:
show_horizontal = any(
child.size.width == width
and child.styles.width
and (child.styles.width.is_cells or child.styles.width.is_auto)
for child in self.children
)
else:
show_horizontal = self.virtual_size.width > (
width - styles.scrollbar_size_vertical
)
However this feels like patching the symptom instead of fixing the underlying issue.
Perhaps, widgets like RichLog
– where the existence of scrollbars doesn't depend on sub-widgets but on the content itself – should grow needs_horizontal_scrollbar
and needs_vertical_scrollbar
methods that can be called in this situation.