Skip to content

Fix search result keyboard navigation #7913

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.

Sign up for GitHub

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

Open
Ap4sh wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Fix search result keyboard navigation #7913

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion ts/components/conversationList/MessageSearchResult.dom.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ export const MessageSearchResult: FunctionComponent<PropsType> = memo(
getPreferredBadge,
i18n,
id,
isSelected,
sentAt,
showConversation,
snippet,
Expand Down Expand Up @@ -193,7 +194,7 @@ export const MessageSearchResult: FunctionComponent<PropsType> = memo(
id={id}
isMe={from.isMe}
isNoteToSelf={isNoteToSelf}
isSelected={false}
isSelected={Boolean(isSelected)}
messageText={messageText}
onClick={onClickItem}
phoneNumber={from.phoneNumber}
Expand Down
6 changes: 3 additions & 3 deletions ts/components/leftPane/LeftPaneArchiveHelper.dom.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,7 @@ export class LeftPaneArchiveHelper extends LeftPaneHelper<LeftPaneArchivePropsTy

getConversationAndMessageAtIndex(
conversationIndex: number
): undefined | { conversationId: string } {
): undefined | { conversationId: string; messageId?: string } {
const searchHelper = this.#searchHelper;
const archivedConversations = this.#archivedConversations;

Expand All @@ -192,8 +192,8 @@ export class LeftPaneArchiveHelper extends LeftPaneHelper<LeftPaneArchivePropsTy
getConversationAndMessageInDirection(
toFind: Readonly<ToFindType>,
selectedConversationId: undefined | string,
targetedMessageId: unknown
): undefined | { conversationId: string } {
targetedMessageId: undefined | string
): undefined | { conversationId: string; messageId?: string } {
if (this.#searchHelper) {
return this.#searchHelper.getConversationAndMessageInDirection(
toFind,
Expand Down
74 changes: 67 additions & 7 deletions ts/components/leftPane/LeftPaneSearchHelper.dom.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@

import type { ReactNode } from 'react';

import type { ToFindType } from './LeftPaneHelper.dom.tsx';
import { FindDirection, type ToFindType } from './LeftPaneHelper.dom.tsx';
import { LeftPaneHelper } from './LeftPaneHelper.dom.tsx';
import type { LocalizerType } from '../../types/Util.std.ts';
import type { Row } from '../ConversationList.dom.tsx';
Expand All @@ -27,6 +27,11 @@ import { UserText } from '../UserText.dom.tsx';
// fairly big number.
const SEARCH_RESULTS_FAKE_ROW_COUNT = 99;

type ConversationAndMessageType = {
conversationId: string;
messageId?: string;
};

type MaybeLoadedSearchResultsType<T> =
| { isLoading: true }
| { isLoading: false; results: Array<T> };
Expand Down Expand Up @@ -394,13 +399,45 @@ export class LeftPaneSearchHelper extends LeftPaneHelper<LeftPaneSearchPropsType
return undefined;
}

// This is currently unimplemented. See DESKTOP-1170.
getConversationAndMessageInDirection(
_toFind: Readonly<ToFindType>,
_selectedConversationId: undefined | string,
_targetedMessageId: unknown
): undefined | { conversationId: string } {
return undefined;
toFind: Readonly<ToFindType>,
selectedConversationId: undefined | string,
targetedMessageId: undefined | string
): undefined | ConversationAndMessageType {
if (toFind.unreadOnly) {
return undefined;
}

const results = this.#getConversationAndMessageResults();
if (!results || !results.length) {
return undefined;
}

let selectedIndex = -1;
if (targetedMessageId) {
selectedIndex = results.findIndex(
result => result.messageId === targetedMessageId
);
}
if (selectedIndex < 0 && selectedConversationId) {
selectedIndex = results.findIndex(
result =>
result.messageId == null &&
result.conversationId === selectedConversationId
);
}

let nextIndex: number;
if (selectedIndex < 0) {
nextIndex =
toFind.direction === FindDirection.Up ? results.length - 1 : 0;
} else {
const step = toFind.direction === FindDirection.Up ? -1 : 1;
nextIndex = (selectedIndex + step + results.length) % results.length;
}
const result = results[nextIndex];
strictAssert(result, 'Missing search result');
return result;
}

override onKeyDown(
Expand All @@ -426,6 +463,29 @@ export class LeftPaneSearchHelper extends LeftPaneHelper<LeftPaneSearchPropsType
return this.#allResults().some(results => results.isLoading);
}

#getConversationAndMessageResults():
| undefined
| Array<ConversationAndMessageType> {
const results: Array<ConversationAndMessageType> = [];
for (const list of this.#allResults()) {
if (list.isLoading) {
return undefined;
}

for (const result of list.results) {
results.push(
result.type === 'incoming' || result.type === 'outgoing'
? {
conversationId: result.conversationId,
messageId: result.id,
}
: { conversationId: result.id }
);
}
}
return results;
}

readonly #onEnterKeyDown = (
clearSearchQuery: () => unknown,
showConversation: ShowConversationType
Expand Down
226 changes: 225 additions & 1 deletion ts/test-node/components/leftPane/LeftPaneSearchHelper_test.dom.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import {
} from '../../../components/ConversationList.dom.tsx';
import { getDefaultConversation } from '../../../test-helpers/getDefaultConversation.std.ts';

import { FindDirection } from '../../../components/leftPane/LeftPaneHelper.dom.tsx';
import { LeftPaneSearchHelper } from '../../../components/leftPane/LeftPaneSearchHelper.dom.tsx';

const baseSearchHelperArgs = {
Expand All @@ -24,10 +25,17 @@ const baseSearchHelperArgs = {
startSearchCounter: 0,
};
describe('LeftPaneSearchHelper', () => {
const fakeMessage = () => ({
const fakeMessage = (
overrideProps: Partial<{
id: string;
type: string;
conversationId: string;
}> = {}
) => ({
id: uuid(),
type: 'outgoing',
conversationId: uuid(),
...overrideProps,
});

describe('getBackAction', () => {
Expand Down Expand Up @@ -635,4 +643,220 @@ describe('LeftPaneSearchHelper', () => {
assert.isUndefined(helper.getConversationAndMessageAtIndex(2));
});
});

describe('getConversationAndMessageInDirection', () => {
it('returns the next result across conversations, contacts, and messages', () => {
const conversations = [getDefaultConversation()] as const;
const contacts = [getDefaultConversation()] as const;
const messages = [fakeMessage()] as const;
const helper = new LeftPaneSearchHelper({
...baseSearchHelperArgs,
conversationResults: {
isLoading: false,
results: [...conversations],
},
contactResults: { isLoading: false, results: [...contacts] },
messageResults: { isLoading: false, results: [...messages] },
});

assert.deepEqual(
helper.getConversationAndMessageInDirection(
{ direction: FindDirection.Down, unreadOnly: false },
conversations[0].id,
undefined
),
{ conversationId: contacts[0].id }
);

assert.deepEqual(
helper.getConversationAndMessageInDirection(
{ direction: FindDirection.Down, unreadOnly: false },
contacts[0].id,
undefined
),
{
conversationId: messages[0].conversationId,
messageId: messages[0].id,
}
);
});

it('uses the targeted message when moving through message results', () => {
const conversationId = uuid();
const messages = [
fakeMessage({ conversationId }),
fakeMessage({ conversationId }),
] as const;
const helper = new LeftPaneSearchHelper({
...baseSearchHelperArgs,
messageResults: { isLoading: false, results: [...messages] },
});

assert.deepEqual(
helper.getConversationAndMessageInDirection(
{ direction: FindDirection.Down, unreadOnly: false },
conversationId,
messages[0].id
),
{
conversationId,
messageId: messages[1].id,
}
);

assert.deepEqual(
helper.getConversationAndMessageInDirection(
{ direction: FindDirection.Up, unreadOnly: false },
conversationId,
messages[1].id
),
{
conversationId,
messageId: messages[0].id,
}
);
});

it('does not treat message results as selected without a targeted message', () => {
const conversationId = uuid();
const messages = [
fakeMessage({ conversationId }),
fakeMessage({ conversationId }),
] as const;
const helper = new LeftPaneSearchHelper({
...baseSearchHelperArgs,
messageResults: { isLoading: false, results: [...messages] },
});

assert.deepEqual(
helper.getConversationAndMessageInDirection(
{ direction: FindDirection.Down, unreadOnly: false },
conversationId,
undefined
),
{
conversationId,
messageId: messages[0].id,
}
);

assert.deepEqual(
helper.getConversationAndMessageInDirection(
{ direction: FindDirection.Up, unreadOnly: false },
conversationId,
undefined
),
{
conversationId,
messageId: messages[1].id,
}
);
});

it('wraps when moving past the first or last result', () => {
const conversations = [getDefaultConversation()] as const;
const messages = [fakeMessage()] as const;
const helper = new LeftPaneSearchHelper({
...baseSearchHelperArgs,
conversationResults: {
isLoading: false,
results: [...conversations],
},
messageResults: { isLoading: false, results: [...messages] },
});

assert.deepEqual(
helper.getConversationAndMessageInDirection(
{ direction: FindDirection.Up, unreadOnly: false },
conversations[0].id,
undefined
),
{
conversationId: messages[0].conversationId,
messageId: messages[0].id,
}
);

assert.deepEqual(
helper.getConversationAndMessageInDirection(
{ direction: FindDirection.Down, unreadOnly: false },
messages[0].conversationId,
messages[0].id
),
{ conversationId: conversations[0].id }
);
});

it('returns the first or last result if no conversation is selected', () => {
const conversations = [getDefaultConversation()] as const;
const messages = [fakeMessage()] as const;
const helper = new LeftPaneSearchHelper({
...baseSearchHelperArgs,
conversationResults: {
isLoading: false,
results: [...conversations],
},
messageResults: { isLoading: false, results: [...messages] },
});

assert.deepEqual(
helper.getConversationAndMessageInDirection(
{ direction: FindDirection.Down, unreadOnly: false },
undefined,
undefined
),
{ conversationId: conversations[0].id }
);

assert.deepEqual(
helper.getConversationAndMessageInDirection(
{ direction: FindDirection.Up, unreadOnly: false },
undefined,
undefined
),
{
conversationId: messages[0].conversationId,
messageId: messages[0].id,
}
);
});

it('returns undefined while results are loading', () => {
const helper = new LeftPaneSearchHelper({
...baseSearchHelperArgs,
conversationResults: { isLoading: true },
contactResults: {
isLoading: false,
results: [getDefaultConversation()],
},
messageResults: { isLoading: false, results: [fakeMessage()] },
});

assert.isUndefined(
helper.getConversationAndMessageInDirection(
{ direction: FindDirection.Down, unreadOnly: false },
undefined,
undefined
)
);
});

it('returns undefined for unread-only navigation', () => {
const helper = new LeftPaneSearchHelper({
...baseSearchHelperArgs,
conversationResults: {
isLoading: false,
results: [getDefaultConversation({ markedUnread: true })],
},
});

assert.isUndefined(
helper.getConversationAndMessageInDirection(
{ direction: FindDirection.Down, unreadOnly: true },
undefined,
undefined
)
);
});
});
});
Loading