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

fix #4877: add customizable keybind for closing modals #6573

Conversation

ian-h-chamberlain
Copy link

Fixes #4877

I got frustrated enough by this issue to try tackling it myself! This creates a new keyboard shortcut for closing Modals, which defaults to Escape (the previously hardcoded behavior). I'll probably set it to Ctrl-C for myself to restore the old-old behavior before #4877 was reopened.

There are a couple other places that had 'Escape' hardcoded for some kind of keydown event, but it seems mostly harmless - AFAIK this was the biggest painful hardcoded shortcut.

A couple notes on the implementation — I'm happy to add a test somewhere if that's wanted, but I wasn't sure exactly where to look. I did manual testing and it seems to work but I could use a pointer if there's a UI test that could correspond to this change.

Also, happy to take feedback on the description of the keyboard shortcut as well as the help text, I just plumbed in something quick to verify functionality.

Fixes Kong#4877

There are a couple other places that had 'Escape' hardcoded for some
kind of keydown event, but it seems mostly harmless - AFAIK this was the
biggest painful hardcoded shortcut.
@ian-h-chamberlain ian-h-chamberlain changed the title Add customizable keybind for closing modals fix #4877: add customizable keybind for closing modals Sep 27, 2023
@kobenguyent
Copy link
Contributor

FYI @ian-h-chamberlain #6518 (comment)

@ian-h-chamberlain
Copy link
Author

Dang, thanks for the pointer, I guess this change might not be accepted either. I'd ask the maintainers to consider it as a stopgap until the refactor they mentioned is ready, but if that's happening soon then I suppose it's fine, as long as the use case in #4877 is still supported!

Guess I should have tried to fix this much longer ago and maybe it could have worked in the meantime 😅

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

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.

Escape key closes environment editor modal in vim mode
3 participants