Skip to content

enhance: close all windows with ESC #1509

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

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

NathanBaulch
Copy link
Contributor

My muscle memory expects dialogs to be easily dismissed with ESC, so it's quite jarring when it doesn't work.
This PR ensures all chromeless windows close when escape is pressed.

@love-linger
Copy link
Collaborator

I do not like this PR. ESC should only close toolbox windows (normally a modal dialog). Windows like File Histories, Blame, Branch Compare and Dir Histories should not use ESC to close.

@NathanBaulch
Copy link
Contributor Author

AFAIK none of these windows have unsaved state that the user needs to commit, so what's the risk of providing a quick dismiss option? Nothing is lost if the user accidentally hits ESC.

@love-linger
Copy link
Collaborator

This has nothing to do with the content displayed within the window, but only with the window's own displaying mode.

Usually, if a window is independent of another window, has Minimize and Maximize buttons, and has Actived and Deactived states, the ESC key will not be used to close the window.

For example: on macOS, we use system caption buttons instead of the custom CaptionButtons view. When pressed the green button, the window will be changed to full-screen mode. And then user restore it using ESC key.

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