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

Select widget crashes if removed right after selected #4782

Closed
davidbrochart opened this issue Jul 21, 2024 · 4 comments · Fixed by #4786
Closed

Select widget crashes if removed right after selected #4782

davidbrochart opened this issue Jul 21, 2024 · 4 comments · Fixed by #4786

Comments

@davidbrochart
Copy link
Contributor

The following code used to work, but now it crashes:

from textual import on
from textual.app import App, ComposeResult
from textual.widgets import Header, Select

LINES = """I must not fear.
Fear is the mind-killer.
Fear is the little-death that brings total obliteration.
I will face my fear.
I will permit it to pass over me and through me.""".splitlines()


class SelectApp(App):
    def compose(self) -> ComposeResult:
        self.select = Select((line, line) for line in LINES)
        self.select.watch_value = self.on_select
        yield Header()
        yield self.select

    def on_select(self):
        self.select.remove()


if __name__ == "__main__":
    app = SelectApp()
    app.run()
╭────────────────────────────────────────────────────── Traceback (most recent call last) ──────────────────────────────────────────────────────╮
│ /home/david/.local/share/hatch/env/virtual/jpterm/N1y3coPp/dev/lib/python3.12/site-packages/textual/widgets/_select.py:537 in update_focus    │
│                                                                                                                                               │
│   534 │   │   async def update_focus() -> None:                                                ╭──── locals ─────╮                            │
│   535 │   │   │   """Update focus and reset overlay."""                                        │ self = Select() │                            │
│   536 │   │   │   self.focus()                                                                 ╰─────────────────╯                            │
│ ❱ 537 │   │   │   self.expanded = False                                                                                                       │
│   538 │   │                                                                                                                                   │
│   539 │   │   self.call_after_refresh(update_focus)  # Prevents a little flicker                                                              │
│   540                                                                                                                                         │
│                                                                                                                                               │
│ /home/david/.local/share/hatch/env/virtual/jpterm/N1y3coPp/dev/lib/python3.12/site-packages/textual/widgets/_select.py:496 in _watch_expanded │
│                                                                                                                                               │
│   493 │                                                                                        ╭────── locals ───────╮                        │
│   494 │   def _watch_expanded(self, expanded: bool) -> None:                                   │ expanded = False    │                        │
│   495 │   │   """Display or hide overlay."""                                                   │     self = Select() │                        │
│ ❱ 496 │   │   overlay = self.query_one(SelectOverlay)                                          ╰─────────────────────╯                        │
│   497 │   │   self.set_class(expanded, "-expanded")                                                                                           │
│   498 │   │   if expanded:                                                                                                                    │
│   499 │   │   │   overlay.focus()                                                                                                             │
╰───────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────╯
NoMatches: No nodes match <DOMQuery query='SelectOverlay'> on Select()

It seems that a check is needed before trying to hide the choices, in case the widget was removed.

Textual Diagnostics

Versions

Name Value
Textual 0.73.0
Rich 13.7.1

Python

Name Value
Version 3.12.4
Implementation CPython
Compiler GCC 12.3.0
Executable /home/david/.local/share/hatch/env/virtual/jpterm/N1y3coPp/dev/bin/python

Operating System

Name Value
System Linux
Release 6.8.0-38-generic
Version #38-Ubuntu SMP PREEMPT_DYNAMIC Fri Jun 7 15:25:01 UTC 2024

Terminal

Name Value
Terminal Application WezTerm (20240203-110809-5046fc22)
TERM xterm-256color
COLORTERM truecolor
FORCE_COLOR Not set
NO_COLOR Not set

Rich Console options

Name Value
size width=145, height=34
legacy_windows False
min_width 1
max_width 145
is_terminal True
encoding utf-8
max_height 34
justify None
overflow None
no_wrap False
highlight None
markup None
height None
@Textualize Textualize deleted a comment from github-actions bot Jul 22, 2024
@willmcgugan
Copy link
Collaborator

self.select.watch_value = self.on_select

Whats the thinking behind monkey patching here? You may be right about the fix, but I wouldn't recommend adding a watcher like that.

@davidbrochart
Copy link
Contributor Author

davidbrochart commented Jul 22, 2024

Right, this is the only way I could reproduce the bug in a minimal reproducible example. But I also saw it in a bigger project that is using @on(Select.Changed) and I had to add this logic as a workaround:

    @on(Select.Changed)
    async def select_changed(self, event: Select.Changed) -> None:
        while True:
            await asyncio.sleep(0)
            if not self.select.expanded:
                break
        self.select.remove()

@willmcgugan
Copy link
Collaborator

Got it. I'll have a fix for the next release. In the meantime, I'd recommend this as a workaround:

    def on_select(self):
        self.call_later(self.select.remove)

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
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants