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

Keyboard shortcuts: Fix Ctrl+G and other intermittently-firing shortcuts on Win/Linux #2705

Merged
merged 1 commit into from
Mar 10, 2021

Conversation

codebykat
Copy link
Member

@codebykat codebykat commented Mar 2, 2021

Fix

Fixes #2420

Some keyboard shortcuts were silently failing on Win/Linux builds. After adding logging and debugging this I was able to determine that the if focusedWindow check was failing. I still don't know exactly why, as this works on OSX.

Changing it to explicitly check if the focusedWindow is undefined seems to fix the issue in my testing. I have only tested this on Windows, not Linux.

After some research, I think it would be safe to remove this guard entirely. The documentation does state that it can be undefined, but only in the case where no window is open, which only applies to OSX. (confirmed by a comment here)

Test

  1. Download the Windows and/or Linux builds from CircleCI
  2. Try all keyboard shortcuts, including Ctrl+G, and verify they all work with no errors in the console

Release

Fixed a bug that caused Ctrl+G and some other shortcuts to fail on Windows/Linux builds

@codebykat codebykat changed the title [WIP] fix ctrl+G on Windows Keyboard shortcuts: Fix Ctrl+G and other intermittently-firing shortcuts on Win/Linux Mar 2, 2021
@codebykat codebykat requested a review from a team March 2, 2021 23:32
@codebykat codebykat self-assigned this Mar 2, 2021
@codebykat codebykat added this to the 2.8.0 milestone Mar 2, 2021
@codebykat codebykat marked this pull request as ready for review March 2, 2021 23:32
Copy link
Contributor

@sandymcfadden sandymcfadden left a comment

Choose a reason for hiding this comment

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

I've been testing on Linux and the only one that doesn't seem to be working as expected is the open tag list Ctrl + Shift + U. If you are focused on the editor this doesn't fire while all others seem to.

It does work on the Mac build as expected. Should this be moved into the handleBrowserShortcut function?

@codebykat
Copy link
Member Author

I've been testing on Linux and the only one that doesn't seem to be working as expected is the open tag list Ctrl + Shift + U. If you are focused on the editor this doesn't fire while all others seem to.

Hmm. It works on Windows for me, regardless of what has focus.

Are you on Ubuntu? Is it possible Ubuntu is eating it? https://superuser.com/questions/358749/how-to-disable-ctrlshiftu-in-ubuntu-linux

Also, how does this compare to the functionality in the 2.7.0 and/or 2.6.0 releases? I don't know if this shortcut gets much use, so maybe it's never worked properly..

@sandymcfadden
Copy link
Contributor

You're right this seems to be the case that it is a global keyboard shortcut on some versions of Linux. I tried on 2.6.0 and had the same issue. Should we look at changing this shortcut, or be OK with it not working on Linux?

@codebykat
Copy link
Member Author

I'm going to merge this for 2.8.0 to get the Windows shortcuts fixed, and open a follow-up issue. Thanks @sandymcfadden!

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.

Search highlight isn't working with Ctrl + G (Find Next/Again)
2 participants