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

Add keyboard shortcuts to change current conversation #3479

Closed
wants to merge 3 commits into from

Conversation

Hyask
Copy link

@Hyask Hyask commented Jul 26, 2019

First time contributor checklist:

Contributor checklist:

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.

Hyask added 2 commits July 26, 2019 15:34
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.
@Hyask
Copy link
Author

Hyask commented Aug 26, 2019

@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?

Copy link

@runejuhl runejuhl left a 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 😄

@Hyask
Copy link
Author

Hyask commented Sep 25, 2019

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?

@runejuhl
Copy link

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.

Copy link

@runejuhl runejuhl left a comment

Choose a reason for hiding this comment

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

Looking good! 👍

@Hyask
Copy link
Author

Hyask commented Oct 8, 2019

Looking good! +1

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.

@scottnonnenberg-signal
Copy link
Contributor

@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.

@Hyask
Copy link
Author

Hyask commented Oct 9, 2019

@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?

@scottnonnenberg-signal
Copy link
Contributor

Keyboard shortcuts have been merged!

@Hyask
Copy link
Author

Hyask commented Jan 7, 2020

Keyboard shortcuts have been merged!

That's awesome news, thanks a lot!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants