-
Notifications
You must be signed in to change notification settings - Fork 780
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
Programmatic change of overflow doesn't update scrollbar #1616
Comments
An interesting finding comes from comparing a similar app that sets The two apps signal that the screen needs to be refreshed, but the gutter app refresh will eventually call from textual.app import App
from textual.containers import Vertical
from textual.widgets import Label
class MyApp(App[None]):
def compose(self):
yield Vertical(Label("bananas!"))
def on_mount(self):
self.query_one(Label).styles.width = "100%"
self.query_one(Label).styles.background = "red"
def on_click(self):
self.query_one(Vertical).styles.scrollbar_gutter = "stable"
if __name__ == "__main__":
MyApp().run() |
I found a partial fix assuming the To do the partial fix, all it takes is to go into Lines 1970 to 1980 in c6909b7
This must be edited to assume that scrollbars are shown if they were already flagged as to be shown OR if the respective overflow is set to def _get_scrollable_region(self, region: Region) -> Region:
# ...
show_horizontal = show_horizontal_scrollbar or (
self.styles.overflow_x == "scroll"
)
show_vertical = show_vertical_scrollbar or (self.styles.overflow_y == "scroll")
if show_horizontal and show_vertical:
(region, _, _, _) = region.split(
-scrollbar_size_vertical,
-scrollbar_size_horizontal,
)
elif show_vertical:
region, _ = region.split_vertical(-scrollbar_size_vertical)
elif show_horizontal:
region, _ = region.split_horizontal(-scrollbar_size_horizontal)
return region To verify this works, use the app shown below. Run the app and press Demo appfrom textual.app import App
from textual.containers import Vertical
class MyApp(App[None]):
def compose(self):
yield Vertical()
def on_key(self, event):
if event.key == "r":
self.screen._refresh_layout(full=True)
elif event.key == "s":
print("setting")
self.query_one(Vertical).styles.overflow_x = "scroll"
print("done")
elif event.key == "h":
self.query_one(Vertical).styles.overflow_x = "hidden"
if __name__ == "__main__":
MyApp().run() |
It seems that some methods that have to do with scrollbars are being called before the scrollbars are refreshed. Consider all of these statements that were found to be true:
There must be a fault in the logic that decides when to call As it stands, the only location that calls the method This must be called from |
I did lots of digging and investigation and I managed to sprinkle some Sadly, I can't articulate very well what led me to this conclusion, but it is in part due to the findings that follow. After many prints and following the code flow, I figured I wanted to keep track of the only call to So, tracking down the call to It is inside Now, the problem is that Similarly, we can get a full fix by also checking if the overflow style has now been changed to def _get_scrollable_region(self, region: Region) -> Region:
# ...
show_horizontal_scrollbar |= self.styles.overflow_x == "scroll"
show_horizontal_scrollbar &= self.styles.overflow_x != "hidden"
show_vertical_scrollbar |= self.styles.overflow_y == "scroll"
show_vertical_scrollbar &= self.styles.overflow_y != "hidden"
if show_horizontal_scrollbar and show_vertical_scrollbar:
(region, _, _, _) = region.split(
-scrollbar_size_vertical,
-scrollbar_size_horizontal,
)
elif show_vertical_scrollbar:
region, _ = region.split_vertical(-scrollbar_size_vertical)
elif show_horizontal_scrollbar:
region, _ = region.split_horizontal(-scrollbar_size_horizontal)
return region So, the fix is either adding this extra check in In the next comment I will share some instructions in case you want to see the final debugging that I made that helped me reach this conclusion. |
Check this comment if you want to reproduce my final debugging steps: Demo app I was using. Save this as
|
This reverts commit c74b81a.
Don't forget to star the repository! Follow @textualizeio for Textual updates. |
If you programmatically change the styles
overflow_x
oroverflow_y
, the layout of the app doesn't get updated properly and the scrollbar is likely to stick around (or not show, depending on what setting you had and what you changed it to).For example, if you run the app below, the horizontal scrollbar starts by being shown because
overflow-x: scroll
is set in the default CSS. If you click the app anywhere, the handler will setoverflow_x = "hidden"
for the container and yet the scrollbar will remain there. The scrollbar only disappears after you force a full refresh of the app, for example by resizing the terminal window.This is very similar to #1607 but there is something extra here because the strategy employed in #1610 to fix #1607 doesn't cut it for
overflow_x
/overflow_y
.I'm opening this issue to keep track of my investigation and to keep different things separated.
The text was updated successfully, but these errors were encountered: