From 1bc33e2378b73c1325d2ba031ac2841fb0ee4f6b Mon Sep 17 00:00:00 2001 From: jprask Date: Sun, 2 May 2021 00:46:43 -0300 Subject: [PATCH] Add getConversationAndMessageInDirection implementation to LeftPaneSearchHelper This allows user to navigate trough search results using the keyboard. Closes #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. --- ts/components/LeftPane.tsx | 5 +- .../leftPane/LeftPaneSearchHelper.tsx | 158 +++++++- .../leftPane/LeftPaneSearchHelper_test.ts | 355 +++++++++++++++++- 3 files changed, 501 insertions(+), 17 deletions(-) diff --git a/ts/components/LeftPane.tsx b/ts/components/LeftPane.tsx index bc210d5416..296096e2ca 100644 --- a/ts/components/LeftPane.tsx +++ b/ts/components/LeftPane.tsx @@ -146,6 +146,8 @@ export const LeftPane: React.FC = ({ modeSpecificProps ); + const previousMessageId = usePrevious(selectedMessageId, selectedMessageId); + // The left pane can be in various modes: the inbox, the archive, the composer, etc. // Ideally, this would render subcomponents such as `` or // `` (and if there's a way to do that cleanly, we should refactor @@ -286,7 +288,7 @@ export const LeftPane: React.FC = ({ conversationToOpen = helper.getConversationAndMessageInDirection( toFind, selectedConversationId, - selectedMessageId + selectedMessageId || previousMessageId ); } } @@ -309,6 +311,7 @@ export const LeftPane: React.FC = ({ selectedConversationId, selectedMessageId, startComposing, + previousMessageId, ]); const preRowsNode = helper.getPreRowsNode({ diff --git a/ts/components/leftPane/LeftPaneSearchHelper.tsx b/ts/components/leftPane/LeftPaneSearchHelper.tsx index 9a38fe8c48..3fcc6fe5f7 100644 --- a/ts/components/leftPane/LeftPaneSearchHelper.tsx +++ b/ts/components/leftPane/LeftPaneSearchHelper.tsx @@ -3,7 +3,9 @@ import React, { ReactChild } from 'react'; -import { LeftPaneHelper, ToFindType } from './LeftPaneHelper'; +import { first, last, flatten, isEmpty } from 'lodash'; + +import { LeftPaneHelper, ToFindType, FindDirection } from './LeftPaneHelper'; import { LocalizerType } from '../../types/Util'; import { Row, RowType } from '../ConversationList'; import { PropsData as ConversationListItemPropsType } from '../conversationList/ConversationListItem'; @@ -11,6 +13,7 @@ import { PropsData as ConversationListItemPropsType } from '../conversationList/ import { Intl } from '../Intl'; import { Emojify } from '../conversation/Emojify'; import { assert } from '../../util/assert'; +import { getConversationInDirection } from './getConversationInDirection'; // The "correct" thing to do is to measure the size of the left pane and render enough // search results for the container height. But (1) that's slow (2) the list is @@ -23,15 +26,17 @@ type MaybeLoadedSearchResultsType = | { isLoading: true } | { isLoading: false; results: Array }; +type MessageResultsType = { + id: string; + conversationId: string; +}; + export type LeftPaneSearchPropsType = { conversationResults: MaybeLoadedSearchResultsType< ConversationListItemPropsType >; contactResults: MaybeLoadedSearchResultsType; - messageResults: MaybeLoadedSearchResultsType<{ - id: string; - conversationId: string; - }>; + messageResults: MaybeLoadedSearchResultsType; searchConversationName?: string; searchTerm: string; }; @@ -51,10 +56,9 @@ export class LeftPaneSearchHelper extends LeftPaneHelper< ConversationListItemPropsType >; - private readonly messageResults: MaybeLoadedSearchResultsType<{ - id: string; - conversationId: string; - }>; + private readonly messageResults: MaybeLoadedSearchResultsType< + MessageResultsType + >; private readonly searchConversationName?: string; @@ -246,13 +250,80 @@ export class LeftPaneSearchHelper extends LeftPaneHelper< return undefined; } - // This is currently unimplemented. See DESKTOP-1170. - // eslint-disable-next-line class-methods-use-this getConversationAndMessageInDirection( - _toFind: Readonly, - _selectedConversationId: undefined | string, - _selectedMessageId: unknown - ): undefined | { conversationId: string } { + toFind: Readonly, + selectedConversationId: undefined | string, + selectedMessageId: undefined | string + ): undefined | { conversationId: string; messageId?: string } { + if (this.isLoading()) { + return undefined; + } + + return ( + this.getMessageInDirectionIfNeeded( + toFind, + selectedMessageId, + selectedConversationId + ) ?? + getConversationInDirection( + this.loadedConversationResults(), + toFind, + selectedMessageId ? undefined : selectedConversationId + ) + ); + } + + private shouldLookupMessages( + toFind: Readonly, + selectedConversationId: undefined | string, + selectedMessageId: undefined | string + ): boolean { + const messageResults = this.loadedMessageResults(); + const conversationResults = this.loadedConversationResults(); + const firstConversation = first(conversationResults); + const lastConversation = last(conversationResults); + + if (toFind.unreadOnly || isEmpty(messageResults)) { + return false; + } + + const goingUpToMessages = + toFind.direction === FindDirection.Up && + (!selectedConversationId || + firstConversation?.id === selectedConversationId || + conversationResults.every(({ id }) => id !== selectedConversationId)); + + const goingDownToMessages = + toFind.direction === FindDirection.Down && + (isEmpty(conversationResults) || + lastConversation?.id === selectedConversationId); + + const browsingMessages = messageResults.some( + ({ id }) => id === selectedMessageId + ); + + return goingUpToMessages || goingDownToMessages || browsingMessages; + } + + private getMessageInDirectionIfNeeded( + toFind: Readonly, + selectedMessageId: undefined | string, + selectedConversationId: undefined | string + ): { conversationId: string; messageId: string } | undefined { + const shouldLookupMessages = this.shouldLookupMessages( + toFind, + selectedConversationId, + selectedMessageId + ); + + if (shouldLookupMessages) { + return getMessageInDirection( + this.loadedMessageResults(), + toFind, + selectedMessageId + ); + } + return undefined; } @@ -263,6 +334,27 @@ export class LeftPaneSearchHelper extends LeftPaneHelper< private isLoading(): boolean { return this.allResults().some(results => results.isLoading); } + + private loadedConversationResults(): Array { + // can't use `this.isLoading` because typescript does not follow along + if (this.conversationResults.isLoading || this.contactResults.isLoading) { + return []; + } + + return flatten( + [this.conversationResults, this.contactResults].map( + ({ results }) => results + ) + ); + } + + private loadedMessageResults(): Array { + if (this.messageResults.isLoading) { + return []; + } + + return this.messageResults.results; + } } function getRowCountForLoadedSearchResults( @@ -283,3 +375,39 @@ function getRowCountForLoadedSearchResults( const hasHeader = Boolean(resultRows); return (hasHeader ? 1 : 0) + resultRows; } + +function formatMessageResult( + message: MessageResultsType +): { conversationId: string; messageId: string } { + return { + conversationId: message.conversationId, + messageId: message.id, + }; +} + +function getMessageInDirection( + messages: ReadonlyArray, + toFind: Readonly, + selectedMessageId: undefined | string +): { conversationId: string; messageId: string } | undefined { + if (messages.length === 0) { + return undefined; + } + + if (selectedMessageId === undefined) { + return formatMessageResult( + toFind.direction === FindDirection.Down + ? messages[0] + : messages[messages.length - 1] + ); + } + + const currentIdx = messages.findIndex(({ id }) => id === selectedMessageId); + + const resultIdx = + toFind.direction === FindDirection.Down ? currentIdx + 1 : currentIdx - 1; + + const result = messages[resultIdx]; + + return result ? formatMessageResult(result) : undefined; +} diff --git a/ts/test-node/components/leftPane/LeftPaneSearchHelper_test.ts b/ts/test-node/components/leftPane/LeftPaneSearchHelper_test.ts index 37be98cf39..056cce945e 100644 --- a/ts/test-node/components/leftPane/LeftPaneSearchHelper_test.ts +++ b/ts/test-node/components/leftPane/LeftPaneSearchHelper_test.ts @@ -5,14 +5,16 @@ import { assert } from 'chai'; import * as sinon from 'sinon'; import { v4 as uuid } from 'uuid'; import { RowType } from '../../../components/ConversationList'; +import { FindDirection } from '../../../components/leftPane/LeftPaneHelper'; import { LeftPaneSearchHelper } from '../../../components/leftPane/LeftPaneSearchHelper'; describe('LeftPaneSearchHelper', () => { - const fakeConversation = () => ({ + const fakeConversation = (markedUnread = false) => ({ id: uuid(), title: uuid(), type: 'direct' as const, + markedUnread, }); const fakeMessage = () => ({ @@ -446,4 +448,355 @@ describe('LeftPaneSearchHelper', () => { ); }); }); + + describe('getConversationAndMessageInDirection', () => { + it('returns undefined when loading', () => { + const helper = new LeftPaneSearchHelper({ + conversationResults: { + isLoading: true, + }, + contactResults: { isLoading: true }, + messageResults: { + isLoading: true, + }, + searchTerm: 'foo', + }); + + assert.deepEqual( + helper.getConversationAndMessageInDirection( + { direction: FindDirection.Down, unreadOnly: false }, + undefined, + undefined + ), + undefined + ); + }); + + it('skips messages if looking for unreads', () => { + const conversations = [fakeConversation(true), fakeConversation()]; + const messages = [fakeMessage(), fakeMessage(), fakeMessage()]; + + const helper = new LeftPaneSearchHelper({ + conversationResults: { + isLoading: false, + results: conversations, + }, + contactResults: { isLoading: false, results: [] }, + messageResults: { + isLoading: false, + results: messages, + }, + searchTerm: 'foo', + }); + + assert.deepEqual( + helper.getConversationAndMessageInDirection( + { direction: FindDirection.Up, unreadOnly: true }, + undefined, + messages[2].id + ), + { + conversationId: conversations[0].id, + } + ); + }); + + describe('when searching for messages', () => { + it('returns the first message going from last conversation down to messages', () => { + const conversations = [fakeConversation()]; + const messages = [fakeMessage(), fakeMessage(), fakeMessage()]; + + const helper = new LeftPaneSearchHelper({ + conversationResults: { + isLoading: false, + results: conversations, + }, + contactResults: { isLoading: false, results: [] }, + messageResults: { + isLoading: false, + results: messages, + }, + searchTerm: 'foo', + }); + + assert.deepEqual( + helper.getConversationAndMessageInDirection( + { direction: FindDirection.Down, unreadOnly: false }, + conversations[0].id, + undefined + ), + { + conversationId: messages[0].conversationId, + messageId: messages[0].id, + } + ); + }); + + it('returns the first message going from last contact down to messages', () => { + const conversations = [fakeConversation()]; + const contacts = [fakeConversation(), fakeConversation()]; + const messages = [fakeMessage(), fakeMessage(), fakeMessage()]; + + const helper = new LeftPaneSearchHelper({ + conversationResults: { + isLoading: false, + results: conversations, + }, + contactResults: { + isLoading: false, + results: contacts, + }, + messageResults: { + isLoading: false, + results: messages, + }, + searchTerm: 'oh hi mark', + }); + + assert.deepEqual( + helper.getConversationAndMessageInDirection( + { direction: FindDirection.Down, unreadOnly: false }, + contacts[1].id, + undefined + ), + { + conversationId: messages[0].conversationId, + messageId: messages[0].id, + } + ); + }); + + it('returns first message when going down and there are no conversation results', () => { + const messages = [fakeMessage(), fakeMessage(), fakeMessage()]; + + const helper = new LeftPaneSearchHelper({ + conversationResults: { + isLoading: false, + results: [], + }, + contactResults: { + isLoading: false, + results: [], + }, + messageResults: { + isLoading: false, + results: messages, + }, + searchTerm: 'foo', + }); + + assert.deepEqual( + helper.getConversationAndMessageInDirection( + { direction: FindDirection.Down, unreadOnly: false }, + uuid(), + undefined + ), + { + conversationId: messages[0].conversationId, + messageId: messages[0].id, + } + ); + }); + + it('returns the last message going from first conversation up to messages', () => { + const conversations = [fakeConversation()]; + const contacts = [fakeConversation()]; + const messages = [fakeMessage(), fakeMessage(), fakeMessage()]; + + const helper = new LeftPaneSearchHelper({ + conversationResults: { + isLoading: false, + results: conversations, + }, + contactResults: { isLoading: false, results: contacts }, + messageResults: { + isLoading: false, + results: messages, + }, + searchTerm: 'foo', + }); + + assert.deepEqual( + helper.getConversationAndMessageInDirection( + { direction: FindDirection.Up, unreadOnly: false }, + conversations[0].id, + undefined + ), + { + conversationId: messages[2].conversationId, + messageId: messages[2].id, + } + ); + }); + + it('returns the last message going from first contact up to messages', () => { + const contacts = [fakeConversation(), fakeConversation()]; + const messages = [fakeMessage(), fakeMessage()]; + + const helper = new LeftPaneSearchHelper({ + conversationResults: { + isLoading: false, + results: [], + }, + contactResults: { + isLoading: false, + results: contacts, + }, + messageResults: { + isLoading: false, + results: messages, + }, + searchTerm: 'foo', + }); + + assert.deepEqual( + helper.getConversationAndMessageInDirection( + { direction: FindDirection.Up, unreadOnly: false }, + contacts[0].id, + undefined + ), + { + conversationId: messages[1].conversationId, + messageId: messages[1].id, + } + ); + }); + + it('returns last message when going up and no result is selected', () => { + const conversations = [fakeConversation(), fakeConversation()]; + const contacts = [fakeConversation(), fakeConversation()]; + const messages = [fakeMessage(), fakeMessage()]; + + const helper = new LeftPaneSearchHelper({ + conversationResults: { + isLoading: false, + results: conversations, + }, + contactResults: { + isLoading: false, + results: contacts, + }, + messageResults: { + isLoading: false, + results: messages, + }, + searchTerm: 'foo', + }); + + assert.deepEqual( + helper.getConversationAndMessageInDirection( + { direction: FindDirection.Up, unreadOnly: false }, + uuid(), + undefined + ), + { + conversationId: messages[1].conversationId, + messageId: messages[1].id, + } + ); + }); + + it('returns next message when going trough message list', () => { + const conversations = [fakeConversation(), fakeConversation()]; + const messages = [fakeMessage(), fakeMessage(), fakeMessage()]; + + const helper = new LeftPaneSearchHelper({ + conversationResults: { + isLoading: false, + results: conversations, + }, + contactResults: { + isLoading: false, + results: [], + }, + messageResults: { + isLoading: false, + results: messages, + }, + searchTerm: 'foo', + }); + + assert.deepEqual( + helper.getConversationAndMessageInDirection( + { direction: FindDirection.Down, unreadOnly: false }, + undefined, + messages[0].id + ), + { + conversationId: messages[1].conversationId, + messageId: messages[1].id, + } + ); + }); + + it('returns previous message when going trough message list', () => { + const contacts = [fakeConversation(), fakeConversation()]; + const messages = [fakeMessage(), fakeMessage()]; + + const helper = new LeftPaneSearchHelper({ + conversationResults: { + isLoading: false, + results: [], + }, + contactResults: { + isLoading: false, + results: contacts, + }, + messageResults: { + isLoading: false, + results: messages, + }, + searchTerm: 'foo', + }); + + assert.deepEqual( + helper.getConversationAndMessageInDirection( + { direction: FindDirection.Up, unreadOnly: false }, + undefined, + messages[1].id + ), + { + conversationId: messages[0].conversationId, + messageId: messages[0].id, + } + ); + }); + }); + + describe('when searching for conversations', () => { + it('returns the next conversation when searching downward', () => { + const conversations = [ + fakeConversation(), + fakeConversation(), + fakeConversation(), + ]; + + const messages = [fakeMessage(), fakeMessage(), fakeMessage()]; + + const helper = new LeftPaneSearchHelper({ + conversationResults: { + isLoading: false, + results: conversations, + }, + contactResults: { isLoading: false, results: [] }, + messageResults: { + isLoading: false, + results: messages, + }, + searchTerm: 'foo', + }); + + assert.deepEqual( + helper.getConversationAndMessageInDirection( + { direction: FindDirection.Down, unreadOnly: false }, + conversations[1].id, + undefined + ), + { conversationId: conversations[2].id } + ); + }); + + // Additional tests are found with `getConversationInDirection`. + }); + }); });