-
Notifications
You must be signed in to change notification settings - Fork 102
feat: add custom_key_binding argument to add custom keyboard handlings #282
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
base: master
Are you sure you want to change the base?
Conversation
…-instruction-messages-for-the-multi-choice-questions feat: customize instruction messages for the multi choice questions
…port-to-questionary-confirm feat: Add customized instruction support to confirm
Ia 222 implement skip question
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR.
I've done a quick pass of code changes and made some suggestions. I think you've disabled the option for reviewers to apply changes themselves, so can you look over these changes and then apply them? Once you've done that, I'll do a more comprehensive review.
FYI, I might have missed changing custom_key_binding to custom_key_bindings in some places - please fix if I have!
Co-authored-by: Kian Cross <[email protected]>
Co-authored-by: Kian Cross <[email protected]>
Co-authored-by: Kian Cross <[email protected]>
Co-authored-by: Kian Cross <[email protected]>
Co-authored-by: Kian Cross <[email protected]>
Co-authored-by: Kian Cross <[email protected]>
Co-authored-by: Kian Cross <[email protected]>
Co-authored-by: Kian Cross <[email protected]>
Co-authored-by: Kian Cross <[email protected]>
Co-authored-by: Kian Cross <[email protected]>
…222-implement-skip-question
Co-authored-by: Kian Cross <[email protected]>
…odadata/ia-questionary into IA-222-implement-skip-question
|
Thanks for your review @kiancross, I fixed your feedbacks :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if a better interface might be
{"c": lambda: "custom"}or even just
{"c": "custom"}The other code would be modified to something like this
if custom_key_bindings is not None:
for key, func in custom_key_bindings.items():
bindings.add(key, eager=True)(lamba event: event.app.exit(result=func()))
# or, if `func` is just a string
bindings.add(key, eager=True)(lamba event: event.app.exit(result=func))The reason for this is that I don't think we expose event.app anywhere else, and in most (all?) places, we just return a value, which is handled by internal code, rather than requiring users to call event.app.exit explicitly.
What do you think?
What is the problem that this PR addresses?
Users were not able to customize keyboard inputs. Thanks to this PR, we can now customize the keyboard input as shown below thanks to new
custom_key_bindingargument. For now,custom_key_bindingfield has been added forselect,text,checkbox, andconfirm.We will add a custom key handling for the
sbutton in this example. Whenever user presss, we will return_skipped_string.custom_key_bindingargument takes a dictionary where the key is either a string orprompt_toolkit.keys.Keysobject the value is a callable that accepts aprompt_toolkit.key_binding.KeyBindingevent.or the same example can be written as
Checklist