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

SelectionList with long lines throws an exception when the window is too small #2900

Closed
valberg opened this issue Jul 7, 2023 · 6 comments · Fixed by #2970
Closed

SelectionList with long lines throws an exception when the window is too small #2900

valberg opened this issue Jul 7, 2023 · 6 comments · Fixed by #2970
Assignees
Labels
bug Something isn't working Task

Comments

@valberg
Copy link

valberg commented Jul 7, 2023

Textual Diagnostics

Versions

Name Value
Textual 0.29.0
Rich 13.4.2

Python

Name Value
Version 3.11.3
Implementation CPython
Compiler GCC 13.1.1 20230429
Executable /home/valberg/.pyenv/versions/textual-dev/bin/python

Operating System

Name Value
System Linux
Release 6.4.1-arch2-1
Version #1 SMP PREEMPT_DYNAMIC Tue, 04 Jul 2023 08:39:40 +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=62, height=34
legacy_windows False
min_width 1
max_width 62
is_terminal True
encoding utf-8
max_height 34
justify None
overflow None
no_wrap False
highlight None
markup None
height None

Bug report

Running the following in a smallish window, or resizing below the length of the selection length:

from textual.app import App, ComposeResult
from textual.widgets import Header, Footer, SelectionList


class TestApp(App):

    def compose(self) -> ComposeResult:
        yield Header()

        yield SelectionList(
            ("xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx", 1),
        )

        yield Footer()


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

results in the following exception:

OptionDoesNotExist: There is no option with an index of 1
@valberg valberg changed the title SelectionList with long lines breaks when the container is too small SelectionList with long lines throws an exception when the container is too small Jul 7, 2023
@github-actions
Copy link

github-actions bot commented Jul 7, 2023

We found the following entry 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

@valberg valberg changed the title SelectionList with long lines throws an exception when the container is too small SelectionList with long lines throws an exception when the window is too small Jul 7, 2023
@davep davep added bug Something isn't working Task labels Jul 7, 2023
@davep
Copy link
Contributor

davep commented Jul 7, 2023

Good catch; some work should either be done to truncate options longer than the width, or OptioinList needs to be improved to handle the underlying selection list's wrapping of long options. If I quickly hack Textual to eat the error you can see how the underlying lines and the check boxes get out of sync too.

Screenshot 2023-07-07 at 12 34 04

@valberg
Copy link
Author

valberg commented Jul 7, 2023

I have worked around it by simply having shorter labels - for now :)

If you have any pointers on how this could be solved, and where to solve it, I might be able to put in the grunt work. But as everybody else I'm short for time, so it's a big maybe :)

@davep
Copy link
Contributor

davep commented Jul 7, 2023

It's all down to how SelectionList sits on top of OptionList, and how the former has been done with "each thing only takes up one line" in mind, but the latter is super flexible and can have one option spanning many lines. So really I guess the first thing for us to do is to decide if we should restrict the prompts for SelectionList (that was the intention: to have it act like a CheckBox-adjacent take on RadioSet) so they never "wrap", or if we relax that and then how and where the checks go when a prompt spans multiple lines.

I sense it's a quick design chat we'll need internally first before fixing this.

@zdovcj
Copy link

zdovcj commented Jul 18, 2023

I have also tried to size my content upon resize event, but it doesn't solve the issue. In this case, it will even crash on increasing the terminal size. I assume that the function that sets the content length might be called before the the code that crashes.

Code to reproduce the crash upon increasing:

import textual.widgets
import textual.app
import textual.events
import textual  

class App(textual.app.App):

    @textual.on(textual.events.Resize)
    def resize_handler(self):
        width = self.size.width
        text = 'a' * (width - 4)
        widget = self.query_one(textual.widgets.SelectionList)
        widget.clear_options()
        widget.add_option((text, 0, None))

    def compose(self):
        yield textual.widgets.SelectionList (('random_text', 0), id='widget_id')

if __name__ == '__main__':
    App().run()

@github-actions
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.

3 participants