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

Removed the default keybind that bound esc to cancel query #1717

Merged
merged 1 commit into from Sep 29, 2023

Conversation

austinwilcox
Copy link
Contributor

This contains the fix for Issue 1716. Esc was by default set to still cancel query instead of being handled later by checking to see if the user was using vim keybindings. This is a one liner change.

@rathboma
Copy link
Collaborator

In your PR - does Esc still cancel the query, but it's bound elsewhere?

@austinwilcox
Copy link
Contributor Author

          if(this.userKeymap === "vim") {
            runQueryKeyMap["Ctrl-Esc"] = this.cancelQuery
          } else {
            runQueryKeyMap.Esc = this.cancelQuery
          }

This is the code that handles adding esc to cancel the query for regular users, it's just a couple lines below the longer list of keybindings. I will run a couple of quick tests and make sure it will still cancel the query for users not using vim.

@austinwilcox
Copy link
Contributor Author

I can confirm that it does still cancel the query for default keybindings, and for vim keybindings it works with Ctrl+Esc.

@austinwilcox
Copy link
Contributor Author

Peek 2023-09-29 08-27
Here is a quick gif showing that, I still need to get something like screenkey running on my box so you can see the keypresses.

@rathboma rathboma merged commit 1d92b24 into beekeeper-studio:master Sep 29, 2023
4 checks passed
@rathboma
Copy link
Collaborator

Thanks Austin!

@austinwilcox
Copy link
Contributor Author

You're welcome. Happy to support the project where I can.

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.

None yet

2 participants