-
-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
Add keyboard shortcuts to change current conversation #3479
Conversation
This is related to some issues: signalapp#512, signalapp#1104, signalapp#3154. This work is based on an existing closed PR (signalapp#2042) by @colefranz.
This work is based on an existing closed PR (!2042) by @colefranz.
@scottnonnenberg-signal do you have any news on this MR? Is it relevant for me to rebase and fix the conflict, or will it simply be dropped? |
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.
MR looks good to me, but I'd want the keys to be configurable before merging it. ...not that I have the permissions to do so anyway 😉
At least the modifier keys should be configurable, but the "action keys" (currently arrow keys) might be nice to have as configurable buttons. E.g. to have keys change conversations like Firefox/Chrome changes tabs you'd use CTRL+PG_UP or CTRL+PG_DOWN.
This would also make it possible to set the default key combinations based on platform, e.g. ⌘+UP and ⌘+DOWN for Apple platforms.
...and if you're up for it I'd love to be able to use PG_UP and PG_DOWN to scroll up/down in my Signal conversations 😄
Hey, thanks for the review! I'll see what I can do to enable configuration of the shortcuts. Do you have any pointers on the configuration mechanisms available in Signal? Should it be in a file? Which one? In the "Preference" window? |
Sorry, can't give you any pointers. I just stumbled across this issue because I wanted some keyboard shortcuts in Signal too and I have no familiarity with the Signal-Desktop codebase... With that said I'd expect to be able to adjust such preferences using the Preference dialog, not having to manually edit a text file. I'd be fine with either, but the intended audience for this app should probably be a bit wider than that. |
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.
Looking good! 👍
This isn't fully working yet, I have a bug with the setting displayed by the "Preference" window: no modifier is checked by default when I open it, even if everything is working fine from a functional point of view. I'll try to fix that soon, and implement the setting for the action key. |
@Hyask I hate to be the bearer of bad news, but it is very unlikely that we'll merge this. We have a more comprehensive look at keyboard shortcuts planned, and this will be superseded. You can keep it open and continue to work on it, but I would recommend that you close it. |
Okay, thanks for letting me know that. Do you have an idea about the schedule of your work on the keyboard shortcuts? |
Keyboard shortcuts have been merged! |
That's awesome news, thanks a lot! |
First time contributor checklist:
Contributor checklist:
development
branchyarn ready
run passes successfully (more about tests here)Description
This PR adds some keyboard shortcuts to switch conversation:
Ctrl/Alt
+Up/Down
.This is mainly an update of this previous MR: #2042, and concerns at least the following issues: #512, #1104, #3154.
I tested those change locally on a development version, then on a local production release, both successfully. A first press on
tab
, or a click anywhere inside the app, is required on startup to give right focus for keyboard events.My conversation list include normal Signal conversation, empty ones (contacts that have Signal, but to who I never talked), and the
Note to self
contact (me). The shortcuts are of course able to travel the whole list, but sadly, it doesn't scroll on the conversation list once the focused conversation is out of the widget.Everything was made on Debian Buster.