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

electron: fix electron global shortcuts #10869

Merged
merged 1 commit into from
Mar 10, 2022
Merged

Conversation

vince-fugnitto
Copy link
Member

What it does

The pull-request restores the change #10704 which was re-introduced after #10600.

The goal of the pull-request is to remove the problematic attachGlobalShortcuts method which caused the application to always interpret ctrl+r as a hard reload of the application despite having either a different context (ex: terminals), or updating keybinding.

The change should now properly use the reload command which uses a proper context, and can be customized by end-users (keymaps.json).

How to test

Same as #10704:

  1. Run the Electron application
  2. Open a bash terminal
  3. Use the ctrl+r shortcut to access 'reverse-i-search'
  4. Put focus on some element other than the terminal.
  5. Use the ctrl+rshortcut to refresh the window - it should refresh
  6. Dirty an editor and use the ctrl+r shortcut - you should be prompted to confirm the refresh
  7. If you'd like, rebind the Refresh Window command to some other keybinding and confirm that it behaves correctly.

Review checklist

Reminder for reviewers

Signed-off-by: vince-fugnitto [email protected]
Co-authored-by: Colin Grant [email protected]

@vince-fugnitto vince-fugnitto added the electron issues related to the electron target label Mar 10, 2022
The commit restores a fix that would previously remove the problematic
`attachGlobalShortcuts` refactoring. The changes should now properly
reload the application when necessary, and support commands such as
recursive-search in the terminal.

Signed-off-by: vince-fugnitto <[email protected]>
Co-authored-by: Colin Grant <[email protected]>
@vince-fugnitto vince-fugnitto force-pushed the vf/fix-electron-reload branch from e6a8c82 to 03e6efd Compare March 10, 2022 13:26
Copy link
Contributor

@colin-grant-work colin-grant-work left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Behavior post #10704 as been restored. Global shortcuts do not interfere with in-app shortcuts. 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
electron issues related to the electron target
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants