Allow keyboard navigation in search results #5234
Conversation
…archHelper This allows user to navigate trough search results using the keyboard. Closes signalapp#5176. The feature is more complex than one might initially think, due to the data structure for the messages being different of that from the conversations. This commit goes with the approach of figuring out if the next item in the navigation flow could be a message, then trying to find the next message and finally falling back to looking for the next conversation. Another thing about the message data structure is that currently we can't check if it's unread, so in this commit when browsing trough unreads only the message results will be ignored.
There was a problem hiding this comment.
Thanks for doing this.
I haven't looked at the code yet, but I think these two things must be addressed before we can merge (good catches, by the way):
- When a message is selected, the focus changes to that message for about a second before coming back to the left panel, breaking the browsing flow if they press the navigation button too quickly while the message is still outlined. I think it would be preferable to still have the message highlighted somehow but keeping the focus on the results list, so the user can quickly browse trough messages but wanted to keep it simple for now but open to suggestions if/how that should be achieved.
It feels like we need a way for the message to be highlighted without being focused. I think it's probably worth discussing the best way to do this. At a glance, it seems like it might be worth replacing selectedMessageId with something like this:
enum MessageSelectionMode {
HighlightedButNotFocused,
HighlightedAndFocused,
}
// ...
selectedMessage?: {
mode: MessageSelectionMode;
id: string;
};
- There is no visual indication of which item is active in the search left bar, so adding the selected state to the result items is probably the next step for the UX to be complete.
I think we need to do this. Could we compute this from the selectedConversation and selectedMessageId (or whatever the latter gets changed to)?
| ); | ||
| }); | ||
|
|
||
| it('returns next message when going trough message list', () => { |
|
Thanks for looking at this @EvanHahn-Signal,
so I have a WIP commit that does that, but in some cases it runs into issues because the
That looks like a nice api. For the implementation, I was initially thinking we would change the |
EvanHahn-Signal
changed the title
|
Does this implementation also cover the "new conversation" list (that can be opened via CTRL-N / pencil icon on the right of the search text area)? I was about to open a new issue, but maybe your PR fixes this already. |
@hiqua I don't think so, I think for that we would implement However when we implement the selected item highlight here I believe it should work regardless of the helper being used so would be worth linking this PR if you create the new issue |
First time contributor checklist:
Contributor checklist:
developmentbranchyarn readyrun passes successfully (more about tests here)Description
Closes #5176.
Implement
getConversationAndMessageInDirectionfor the search helper to allow users to browse search results with keyboard.Couple things to note:
When browsing trough unreads (alt + shift + arrow), messages will be ignored, because given the message search result data structure it's not possible to tell if it's unread or not. I went for the simple solution but suppose we could update the data, just let me know!
When a message is selected, the focus changes to that message for about a second before coming back to the left panel, breaking the browsing flow if they press the navigation button too quickly while the message is still outlined. I think it would be preferable to still have the message highlighted somehow but keeping the focus on the results list, so the user can quickly browse trough messages but wanted to keep it simple for now but open to suggestions if/how that should be achieved.
There is no visual indication of which item is active in the search left bar, so adding the selected state to the result items is probably the next step for the UX to be complete.