From 44dfd280170ea6b1823be7f8f2d732a48baea57f Mon Sep 17 00:00:00 2001 From: Josh Perez <60019601+josh-signal@users.noreply.github.com> Date: Thu, 4 Mar 2021 14:34:04 -0500 Subject: [PATCH] Fix message retry and search results with mentions --- ts/components/ConversationList.stories.tsx | 2 + ts/components/LeftPane.stories.tsx | 2 + .../MessageBodyHighlight.stories.tsx | 18 ++++ .../conversationList/MessageBodyHighlight.tsx | 56 ++++++++--- .../MessageSearchResult.stories.tsx | 96 +++++++++++++++++++ .../conversationList/MessageSearchResult.tsx | 83 ++++++++++++++-- ts/models/messages.ts | 1 + ts/state/ducks/search.ts | 3 + ts/state/selectors/search.ts | 2 + ts/test-both/state/selectors/search_test.ts | 12 +++ 10 files changed, 254 insertions(+), 21 deletions(-) diff --git a/ts/components/ConversationList.stories.tsx b/ts/components/ConversationList.stories.tsx index 6f6d99ec40..6b6252513c 100644 --- a/ts/components/ConversationList.stories.tsx +++ b/ts/components/ConversationList.stories.tsx @@ -65,6 +65,8 @@ const createProps = (rows: ReadonlyArray): PropsType => ({ onClickContactCheckbox: action('onClickContactCheckbox'), renderMessageSearchResult: (id: string, style: React.CSSProperties) => ( = {}): PropsType => ({ renderMainHeader: () =>
, renderMessageSearchResult: (id: string, style: React.CSSProperties) => ( = {}): Props => ({ + bodyRanges: overrideProps.bodyRanges || [], i18n, text: text('text', overrideProps.text || ''), }); @@ -47,6 +48,23 @@ story.add('Two Replacements', () => { return ; }); +story.add('Two Replacements with an @mention', () => { + const props = createProps({ + bodyRanges: [ + { + length: 1, + mentionUuid: '0ca40892-7b1a-11eb-9439-0242ac130002', + replacementText: 'Jin Sakai', + start: 52, + }, + ], + text: + 'Begin <>Inside #1<> \uFFFC@52 This is between the two <>Inside #2<> End.', + }); + + return ; +}); + story.add('Emoji + Newlines + URLs', () => { const props = createProps({ text: diff --git a/ts/components/conversationList/MessageBodyHighlight.tsx b/ts/components/conversationList/MessageBodyHighlight.tsx index 43729cbcf5..af3d727eab 100644 --- a/ts/components/conversationList/MessageBodyHighlight.tsx +++ b/ts/components/conversationList/MessageBodyHighlight.tsx @@ -4,25 +4,27 @@ import React, { ReactNode } from 'react'; import { MESSAGE_TEXT_CLASS_NAME } from './BaseConversationListItem'; +import { AtMentionify } from '../conversation/AtMentionify'; import { MessageBody } from '../conversation/MessageBody'; import { Emojify } from '../conversation/Emojify'; import { AddNewLines } from '../conversation/AddNewLines'; import { SizeClassType } from '../emoji/lib'; -import { LocalizerType, RenderTextCallbackType } from '../../types/Util'; +import { + BodyRangesType, + LocalizerType, + RenderTextCallbackType, +} from '../../types/Util'; const CLASS_NAME = `${MESSAGE_TEXT_CLASS_NAME}__message-search-result-contents`; export type Props = { + bodyRanges: BodyRangesType; text: string; i18n: LocalizerType; }; -const renderNewLines: RenderTextCallbackType = ({ text, key }) => ( - -); - const renderEmoji = ({ text, key, @@ -44,18 +46,42 @@ const renderEmoji = ({ ); export class MessageBodyHighlight extends React.Component { + private readonly renderNewLines: RenderTextCallbackType = ({ + text: textWithNewLines, + key, + }) => { + const { bodyRanges } = this.props; + return ( + ( + + )} + /> + ); + }; + private renderContents(): ReactNode { - const { text, i18n } = this.props; + const { bodyRanges, text, i18n } = this.props; const results: Array = []; const FIND_BEGIN_END = /<>(.+?)<>/g; - let match = FIND_BEGIN_END.exec(text); + const processedText = AtMentionify.preprocessMentions(text, bodyRanges); + + let match = FIND_BEGIN_END.exec(processedText); let last = 0; let count = 1; if (!match) { return ( - + ); } @@ -63,7 +89,7 @@ export class MessageBodyHighlight extends React.Component { while (match) { if (last < match.index) { - const beforeText = text.slice(last, match.index); + const beforeText = processedText.slice(last, match.index); count += 1; results.push( renderEmoji({ @@ -71,7 +97,7 @@ export class MessageBodyHighlight extends React.Component { sizeClass, key: count, i18n, - renderNonEmoji: renderNewLines, + renderNonEmoji: this.renderNewLines, }) ); } @@ -85,24 +111,24 @@ export class MessageBodyHighlight extends React.Component { sizeClass, key: count, i18n, - renderNonEmoji: renderNewLines, + renderNonEmoji: this.renderNewLines, })} ); last = FIND_BEGIN_END.lastIndex; - match = FIND_BEGIN_END.exec(text); + match = FIND_BEGIN_END.exec(processedText); } - if (last < text.length) { + if (last < processedText.length) { count += 1; results.push( renderEmoji({ - text: text.slice(last), + text: processedText.slice(last), sizeClass, key: count, i18n, - renderNonEmoji: renderNewLines, + renderNonEmoji: this.renderNewLines, }) ); } diff --git a/ts/components/conversationList/MessageSearchResult.stories.tsx b/ts/components/conversationList/MessageSearchResult.stories.tsx index cde32a194a..5144a5cf11 100644 --- a/ts/components/conversationList/MessageSearchResult.stories.tsx +++ b/ts/components/conversationList/MessageSearchResult.stories.tsx @@ -43,6 +43,8 @@ const createProps = (overrideProps: Partial = {}): PropsType => ({ 'snippet', overrideProps.snippet || "What's <>going<> on?" ), + body: text('body', overrideProps.body || "What's going on?"), + bodyRanges: overrideProps.bodyRanges || [], from: overrideProps.from as PropsType['from'], to: overrideProps.to as PropsType['to'], isSelected: boolean('isSelected', overrideProps.isSelected || false), @@ -141,3 +143,97 @@ story.add('Empty (should be invalid)', () => { return ; }); + +story.add('@mention', () => { + const props = createProps({ + body: + 'moss banana twine sound lake zoo brain count vacuum work stairs try power forget hair dry diary years no results \uFFFC elephant sorry umbrella potato igloo kangaroo home Georgia bayonet vector orange forge diary zebra turtle rise front \uFFFC', + bodyRanges: [ + { + length: 1, + mentionUuid: '7d007e95-771d-43ad-9191-eaa86c773cb8', + replacementText: 'Shoe', + start: 113, + }, + { + length: 1, + mentionUuid: '7d007e95-771d-43ad-9191-eaa86c773cb8', + replacementText: 'Shoe', + start: 237, + }, + ], + from: someone, + to: me, + snippet: + '...forget hair dry diary years no <>results<> \uFFFC <>elephant<> sorry umbrella potato igloo kangaroo home Georgia...', + }); + + return ; +}); + +story.add('@mention regexp', () => { + const props = createProps({ + body: + '\uFFFC This is a (long) /text/ ^$ that is ... specially **crafted** to (test) our regexp escaping mechanism! Making sure that the code we write works in all sorts of scenarios', + bodyRanges: [ + { + length: 1, + mentionUuid: '7d007e95-771d-43ad-9191-eaa86c773cb8', + replacementText: 'RegExp', + start: 0, + }, + ], + from: someone, + to: me, + snippet: + '\uFFFC This is a (long) /text/ ^$ that is ... <>specially<> **crafted** to (test) our regexp escaping mechanism...', + }); + + return ; +}); + +story.add('@mention no-matches', () => { + const props = createProps({ + body: '\uFFFC hello', + bodyRanges: [ + { + length: 1, + mentionUuid: '7d007e95-771d-43ad-9191-eaa86c773cb8', + replacementText: 'Neo', + start: 0, + }, + ], + from: someone, + to: me, + snippet: '\uFFFC hello', + }); + + return ; +}); + +story.add('@mention no-matches', () => { + const props = createProps({ + body: + 'moss banana twine sound lake zoo brain count vacuum work stairs try power forget hair dry diary years no results \uFFFC elephant sorry umbrella potato igloo kangaroo home Georgia bayonet vector orange forge diary zebra turtle rise front \uFFFC', + bodyRanges: [ + { + length: 1, + mentionUuid: '7d007e95-771d-43ad-9191-eaa86c773cb8', + replacementText: 'Shoe', + start: 113, + }, + { + length: 1, + mentionUuid: '7d007e95-771d-43ad-9191-eaa86c773cb8', + replacementText: 'Shoe', + start: 237, + }, + ], + from: someone, + to: me, + snippet: + '...forget hair dry diary years no results \uFFFC elephant sorry umbrella potato igloo kangaroo home Georgia...', + }); + + return ; +}); diff --git a/ts/components/conversationList/MessageSearchResult.tsx b/ts/components/conversationList/MessageSearchResult.tsx index 4b603bdd46..ba98edb1f3 100644 --- a/ts/components/conversationList/MessageSearchResult.tsx +++ b/ts/components/conversationList/MessageSearchResult.tsx @@ -7,11 +7,13 @@ import React, { FunctionComponent, ReactNode, } from 'react'; +import { escapeRegExp } from 'lodash'; import { MessageBodyHighlight } from './MessageBodyHighlight'; import { ContactName } from '../conversation/ContactName'; -import { LocalizerType } from '../../types/Util'; +import { assert } from '../../util/assert'; +import { BodyRangesType, LocalizerType } from '../../types/Util'; import { ColorType } from '../../types/Colors'; import { BaseConversationListItem } from './BaseConversationListItem'; @@ -24,6 +26,8 @@ export type PropsDataType = { sentAt?: number; snippet: string; + body: string; + bodyRanges: BodyRangesType; from: { phoneNumber?: string; @@ -78,17 +82,77 @@ const renderPerson = ( /> ); +// This function exists because bodyRanges tells us the character position +// where the at-mention starts at according to the full body text. The snippet +// we get back is a portion of the text and we don't know where it starts. This +// function will find the relevant bodyRanges that apply to the snippet and +// then update the proper start position of each body range. +function getFilteredBodyRanges( + snippet: string, + body: string, + bodyRanges: BodyRangesType +): BodyRangesType { + // Find where the snippet starts in the full text + const stripped = snippet + .replace(/<>/g, '') + .replace(/<>/g, '') + .replace(/^.../, '') + .replace(/...$/, ''); + const rx = new RegExp(escapeRegExp(stripped)); + const match = rx.exec(body); + + assert(Boolean(match), `No match found for "${snippet}" inside "${body}"`); + + const delta = match ? match.index + snippet.length : 0; + + // Filters out the @mentions that are present inside the snippet + const filteredBodyRanges = bodyRanges.filter(bodyRange => { + return bodyRange.start < delta; + }); + + const snippetBodyRanges = []; + const MENTIONS_REGEX = /\uFFFC/g; + + let bodyRangeMatch = MENTIONS_REGEX.exec(snippet); + let i = 0; + + // Find the start position within the snippet so these can later be + // encoded and rendered correctly. + while (bodyRangeMatch) { + const bodyRange = filteredBodyRanges[i]; + + if (bodyRange) { + snippetBodyRanges.push({ + ...bodyRange, + start: bodyRangeMatch.index, + }); + } else { + assert( + false, + `Body range does not exist? Count: ${i}, Length: ${filteredBodyRanges.length}` + ); + } + + bodyRangeMatch = MENTIONS_REGEX.exec(snippet); + i += 1; + } + + return snippetBodyRanges; +} + export const MessageSearchResult: FunctionComponent = React.memo( ({ - id, + body, + bodyRanges, conversationId, from, - to, - sentAt, i18n, + id, openConversationInternal, - style, + sentAt, snippet, + style, + to, }) => { const onClickItem = useCallback(() => { openConversationInternal({ conversationId, messageId: id }); @@ -113,7 +177,14 @@ export const MessageSearchResult: FunctionComponent = React.memo( ); } - const messageText = ; + const snippetBodyRanges = getFilteredBodyRanges(snippet, body, bodyRanges); + const messageText = ( + + ); return ( { preview: previewWithData, sticker: stickerWithData, expireTimer: this.get('expireTimer'), + mentions: this.get('bodyRanges'), profileKey, groupV2, group: groupV2 diff --git a/ts/state/ducks/search.ts b/ts/state/ducks/search.ts index b6073483ac..3a27d9c283 100644 --- a/ts/state/ducks/search.ts +++ b/ts/state/ducks/search.ts @@ -7,6 +7,7 @@ import { normalize } from '../../types/PhoneNumber'; import { cleanSearchTerm } from '../../util/cleanSearchTerm'; import dataInterface from '../../sql/Client'; import { makeLookup } from '../../util/makeLookup'; +import { BodyRangesType } from '../../types/Util'; import { ConversationUnloadedActionType, @@ -28,6 +29,8 @@ const { export type MessageSearchResultType = MessageType & { snippet: string; + body: string; + bodyRanges: BodyRangesType; }; export type MessageSearchResultLookupType = { diff --git a/ts/state/selectors/search.ts b/ts/state/selectors/search.ts index 51085eb797..ba6cf03374 100644 --- a/ts/state/selectors/search.ts +++ b/ts/state/selectors/search.ts @@ -125,6 +125,8 @@ export function _messageSearchResultSelector( conversationId: message.conversationId, sentAt: message.sent_at, snippet: message.snippet, + bodyRanges: message.bodyRanges, + body: message.body, isSelected: Boolean(selectedMessageId && message.id === selectedMessageId), isSearchingInConversation: Boolean(searchConversationId), diff --git a/ts/test-both/state/selectors/search_test.ts b/ts/test-both/state/selectors/search_test.ts index 07a4bf1a74..ac3a7fe11b 100644 --- a/ts/test-both/state/selectors/search_test.ts +++ b/ts/test-both/state/selectors/search_test.ts @@ -44,6 +44,8 @@ describe('both/state/selectors/search', () => { function getDefaultSearchMessage(id: string): MessageSearchResultType { return { ...getDefaultMessage(id), + body: 'foo bar', + bodyRanges: [], snippet: 'foo bar', }; } @@ -77,6 +79,8 @@ describe('both/state/selectors/search', () => { ...getDefaultMessage(id), type: 'keychange' as const, snippet: 'snippet', + body: 'snippet', + bodyRanges: [], }, }, }, @@ -114,6 +118,8 @@ describe('both/state/selectors/search', () => { sourceUuid: fromId, conversationId: toId, snippet: 'snippet', + body: 'snippet', + bodyRanges: [], }, }, }, @@ -129,6 +135,8 @@ describe('both/state/selectors/search', () => { conversationId: toId, sentAt: undefined, snippet: 'snippet', + body: 'snippet', + bodyRanges: [], isSelected: false, isSearchingInConversation: false, @@ -165,6 +173,8 @@ describe('both/state/selectors/search', () => { type: 'outgoing' as const, conversationId: toId, snippet: 'snippet', + body: 'snippet', + bodyRanges: [], }, }, }, @@ -180,6 +190,8 @@ describe('both/state/selectors/search', () => { conversationId: toId, sentAt: undefined, snippet: 'snippet', + body: 'snippet', + bodyRanges: [], isSelected: false, isSearchingInConversation: false,