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 OptionHighlighted message leakage #4224

Closed
davep opened this issue Feb 27, 2024 · 2 comments · Fixed by #4280
Closed

Select OptionHighlighted message leakage #4224

davep opened this issue Feb 27, 2024 · 2 comments · Fixed by #4280
Assignees
Labels
bug Something isn't working Task

Comments

@davep
Copy link
Contributor

davep commented Feb 27, 2024

The Select widget makes use of OptionList; when the user opens the Select to pick a value, OptionHighlighted messages "leak" out of the Select. This could result in unexpected messages for the developer who has an OptionList and a Select in their DOM and who doesn't expect to actually have two OptionLists that might send messages.

I think we should either stop the OptionHighlighted from bubbling outside of the Select, or we should turn them into SelectionHighlighted if it's felt necessary that they can be handled.

To illustrate:

from textual import on
from textual.app import App, ComposeResult
from textual.widgets import OptionList, Select, Log

class LeakySelectApp(App[None]):

    def compose(self) -> ComposeResult:
        yield Log()
        yield OptionList(*(f"This is option {n}" for n in range(10)))
        yield Select[int](((f"This is selection {n}", n) for n in range(10)))

    @on(OptionList.OptionHighlighted)
    def log_some_option(self, event: OptionList.OptionHighlighted) -> None:
        self.query_one(Log).write_line(f"{event}")

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

I think the dev would reasonably expect to only get calls to log_some_option from the OptionList they composed in.

@davep davep added bug Something isn't working Task labels Feb 27, 2024
davep added a commit to davep/textual-sandbox that referenced this issue Feb 27, 2024
@willmcgugan
Copy link
Collaborator

I think we should quell those messages. I can't see a use for an additional message that reports what the user has currently selected.

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
2 participants