-
-
Notifications
You must be signed in to change notification settings - Fork 2.4k
Add keybind to cancel confirmation with 'n' #2082
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
Conversation
|
Coincidentally #2097 touches on the same thing. I think we should remove the 'y' keybinding to remove this confusion, and to free up more space for menu-specific keybindings |
|
CC @mark2185 |
|
That's why I opened the PR, actually. Even though I never use |
I just want to express my preference for being able to use y/n. |
jesseduffield
left a comment
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.
You've convinced me! One thing :)
|
I’ll solve the merge conflict tomorrow. |
|
Rebased on main and resolved the merge conflicts. One small hiccup is that you can also dismiss non-question dialogues with ‘n’. For example: “You cannot delete the checked out branch!” can be dismissed with ‘n’ now, too. Semantically this is odd, but in terms of muscle memory it might be a good thing. |
|
Maybe the |
|
This relates to the argument I was making in #2097 i.e. the muscle memory can cause confusion. I don't mind having y/n work in an alert panel (each keybinding would just close it) but if you're used to using 'n' to close a popup panel and then a menu panel appears where 'n' is tied to an actual menu item (e.g. 'create _n_ew branch') that will cause confusion. So rather than build up the muscle memory of using enter/escape for everything, the user could get themselves in trouble with using y/n sometimes and enter/escape other times. |
Just to recap my suggestion:
To be frank, I'm also getting the feeling this complicates things, but I don't think it's due to muscle memory, but to the |
|
@mark2185 I know that some CLI tools take that approach but I have some issues with it:
I'm not aware of any confirmation popups where [y/N] would be preferable to [Y/n]. If we come across a confirmation popup where [y/N] makes more sense, I think a menu with two items would likely be better. |
|
Actually that might be a way to solve this problem: literally use a menu in place of a confirmation panel: but allow for menus to have prompt text at the top. We can have 'y' and 'n' be assigned to the yes and no menu items, and enter/escape will have the same effect as before. |
That's no longer true since #2495 , now one can confirm only with |
|
Closing as we now only allow enter/esc for confirmation panels and so it's consistent (and we get more keybindings freed up for other things). |
In confirmation boxes, you're able to confirm with either Enter or y. Cancelling is done with Esc, but not with n, as you might expect.
I noticed myself trying to dismiss this prompt with n a lot of times and failing, so I added the option to do so.
In my opinion, this doesn't warrant any tests because it only adds a shortcut for confirmation dialogues.