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

feat: allow any update_events on InputText #376

Merged
merged 2 commits into from
Nov 15, 2023

Conversation

iisakkirotko
Copy link
Collaborator

@iisakkirotko iisakkirotko commented Nov 13, 2023

Previously on_value would be triggered whenever enter was pressed or InputText lost focus. With this PR we would allow people to modify these events.

Copy link
Collaborator Author

Current dependencies on/for this PR:

This stack of pull requests is managed by Graphite.

Previously `on_value` would be triggered whenever enter was pressed or `InputText` lost focus. Now, when `enter_only = True`, losing focus will not trigger this.
Copy link
Contributor

@maartenbreddels maartenbreddels left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should have this feature, but I wonder if we should have a different API.
What about update_events=["blur", "keyup.enter"] as default arguments? This also allows a user to use "keyup.shift.enter" for instance.
@mariobuikhuizen ?

@iisakkirotko
Copy link
Collaborator Author

I think we should have this feature, but I wonder if we should have a different API. What about update_events=["blur", "keyup.enter"] as default arguments? This also allows a user to use "keyup.shift.enter" for instance. @mariobuikhuizen ?

Sounds like a good idea. We could then also support an arbitrary number of events as triggers. We should then also properly document the bug that causes only one event of a type to be picked up (unless that was fixed and I've forgotten).

Copy link
Contributor

@maartenbreddels maartenbreddels left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we don't need the overload anymore, and the docstring needs change.

Weird btw, that mypy didn't complain about the overload arguments not matching the function arguments.

Copy link
Collaborator Author

I think we don't need the overload anymore, and the docstring needs change.

Should be fixed now

@iisakkirotko iisakkirotko changed the title feat: enter_only option for InputText feat: allow any update_events on InputText Nov 14, 2023
@maartenbreddels maartenbreddels merged commit d65e9d2 into master Nov 15, 2023
23 checks passed
@maartenbreddels
Copy link
Contributor

I think this is very useful for #341 as well, such that we can bind to shift-enter

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 this pull request may close these issues.

2 participants