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

Vertical scrollbar loss in horizontal layout #3687

Closed
davep opened this issue Nov 15, 2023 · 3 comments · Fixed by #3712
Closed

Vertical scrollbar loss in horizontal layout #3687

davep opened this issue Nov 15, 2023 · 3 comments · Fixed by #3712
Labels
bug Something isn't working Task

Comments

@davep
Copy link
Contributor

davep commented Nov 15, 2023

This is a bit of a tricky one to narrow down; it started out with a missing scrollbar in the right-most column of my current pet project, and ended up with a bit of a MRE-making rabbit hole; but I think I got there. Take this code:

from textual.app import App, ComposeResult
from textual.containers import Horizontal
from textual.widgets import OptionList

class MissingScrollbarApp(App[None]):

    CSS = """
    OptionList {
        height: 1fr;
    }

    #left {
        min-width: 25;
    }

    #middle {
        width: 5fr;
    }

    #right {
        min-width: 30;
    }
    """

    def compose(self) -> ComposeResult:
        options = [str(n) for n in range(200)]
        with Horizontal():
            yield OptionList(*options, id="left")
            yield OptionList(*options, id="middle")
            yield OptionList(*options, id="right")

if __name__ == "__main__":
    MissingScrollbarApp().run()

The numbers are mostly important, and derive from the layout I had in my application, and are whittled down to what I hope is the bare minimum to reproduce. Also of note is in my app it's two OptionLists and a VerticalScroll in the third column, but the above keeps it simple and has the same effect.

So, given the above, you'd expect a vertical scrollbar on each of the "panels". But, what you actually get is the third one gets cut off:

Screenshot 2023-11-15 at 21 25 01

If I make the terminal sufficiently wide enough, it eventually appears:

Screenshot 2023-11-15 at 21 25 48

I had to drag this out to about 220 characters wide.

There is also an intermediate step, between no scrollbar, and scrollbar, which shows half a scrollbar:

Screenshot 2023-11-15 at 21 27 20

I originally tried to recreate this using just three VerticalScroll but couldn't get the no-scrollbar effect (but did get the half-scrollbar); but as soon as I threw the OptionList into the mix (so perhaps a ScrollView thing?).

Tagging @willmcgugan for triage.

@davep davep added bug Something isn't working Task labels Nov 15, 2023
@williballenthin
Copy link
Contributor

Hm, maybe the other Will? I suppose I could take a stab though I wouldn't be particularly efficient at it.

@davep
Copy link
Contributor Author

davep commented Nov 15, 2023

@williballenthin Hah! Sorry! for some reason GitHub has decided you need to be at the top of the @will completion.

Yes, @willmcgugan

davep added a commit to davep/tinboard that referenced this issue Nov 15, 2023
Mostly there with how I want it to look, but I need to spend a bit of time
thinking about how I'm going to do the tags. Using links in Markdown is
great and all, but it's a bit keyboard-hostile. I want to be
keyboard-friendly too.

So this is a WiP commit at the end of the evening, and the next time I'm in
here I'll have a proper think about how I'm going to do this.

See also Textualize/textual#3687 as a related
issue that I've discovered while making these changes (it explains why the
scrollbar in the details pane isn't showing at the moment, if it's needed).
davep added a commit to davep/textual-sandbox that referenced this issue Nov 16, 2023
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
bug Something isn't working Task
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants