From 4a67723154ac01bc139a5a06e5c0ea229a6dae07 Mon Sep 17 00:00:00 2001 From: Aswin S Date: Sun, 22 Oct 2023 05:53:01 +0530 Subject: [PATCH 01/13] fix: append whitespace after emoji --- src/libs/ComposerUtils/index.ts | 5 +- .../ComposerWithSuggestions.js | 59 +++++++++++++------ 2 files changed, 44 insertions(+), 20 deletions(-) diff --git a/src/libs/ComposerUtils/index.ts b/src/libs/ComposerUtils/index.ts index 5e2a42fc65dd..987615f17695 100644 --- a/src/libs/ComposerUtils/index.ts +++ b/src/libs/ComposerUtils/index.ts @@ -32,7 +32,10 @@ function canSkipTriggerHotkeys(isSmallScreenWidth: boolean, isKeyboardShown: boo */ function getCommonSuffixLength(str1: string, str2: string): number { let i = 0; - while (str1[str1.length - 1 - i] === str2[str2.length - 1 - i]) { + if(str1.length===0||str2.length===0){ + return 0; + } + while (str1[str1.length - 1 - i] === str2[str2.length - 1 - i]) { i++; } return i; diff --git a/src/pages/home/report/ReportActionCompose/ComposerWithSuggestions.js b/src/pages/home/report/ReportActionCompose/ComposerWithSuggestions.js index e194d0870885..a1950ad2a96e 100644 --- a/src/pages/home/report/ReportActionCompose/ComposerWithSuggestions.js +++ b/src/pages/home/report/ReportActionCompose/ComposerWithSuggestions.js @@ -116,6 +116,7 @@ function ComposerWithSuggestions({ return draft; }); const commentRef = useRef(value); + const lastTextRef = useRef(value); const {isSmallScreenWidth} = useWindowDimensions(); const maxComposerLines = isSmallScreenWidth ? CONST.COMPOSER.MAX_LINES_SMALL_SCREEN : CONST.COMPOSER.MAX_LINES; @@ -194,6 +195,31 @@ function ComposerWithSuggestions({ RNTextInputReset.resetKeyboardInput(findNodeHandle(textInputRef.current)); }, [textInputRef]); + const findNewlyAddedChars = useCallback( + (prevText, newText) => { + const isTextReplace = selection.end - selection.start > 0; + const commonSuffixLength =ComposerUtils.getCommonSuffixLength(prevText, newText); + let startIndex = -1; + let endIndex = -1; + let i = 0; + + while (i < newText.length && prevText.charAt(i) === newText.charAt(i) && selection.start > i) { + i++; + } + + if (i < newText.length) { + startIndex = i; + // if text is getting pasted over find length of common suffix and subtract it from new text length + endIndex = isTextReplace ? newText.length-commonSuffixLength : i + (newText.length - prevText.length); + } + + return {startIndex, endIndex, diff: newText.substring(startIndex, endIndex)}; + }, + [selection.end, selection.start], + ); + + const insertWhiteSpace = (text, index) => `${text.slice(0, index)} ${text.slice(index)}`; + const debouncedSaveReportComment = useMemo( () => _.debounce((selectedReportID, newComment) => { @@ -211,7 +237,13 @@ function ComposerWithSuggestions({ const updateComment = useCallback( (commentValue, shouldDebounceSaveComment) => { raiseIsScrollLikelyLayoutTriggered(); - const {text: newComment, emojis} = EmojiUtils.replaceAndExtractEmojis(commentValue, preferredSkinTone, preferredLocale); + const {startIndex, endIndex, diff} = findNewlyAddedChars(lastTextRef.current, commentValue); + const isEmojiInserted = diff.length && endIndex > startIndex && EmojiUtils.containsOnlyEmojis(diff); + const {text: newComment, emojis} = EmojiUtils.replaceAndExtractEmojis( + isEmojiInserted ? insertWhiteSpace(commentValue, endIndex) : commentValue, + preferredSkinTone, + preferredLocale, + ); if (!_.isEmpty(emojis)) { const newEmojis = EmojiUtils.getAddedEmojis(emojis, emojisPresentBefore.current); @@ -255,16 +287,7 @@ function ComposerWithSuggestions({ debouncedBroadcastUserIsTyping(reportID); } }, - [ - debouncedUpdateFrequentlyUsedEmojis, - preferredLocale, - preferredSkinTone, - reportID, - setIsCommentEmpty, - suggestionsRef, - raiseIsScrollLikelyLayoutTriggered, - debouncedSaveReportComment, - ], + [raiseIsScrollLikelyLayoutTriggered, findNewlyAddedChars, preferredSkinTone, preferredLocale, setIsCommentEmpty, debouncedUpdateFrequentlyUsedEmojis, suggestionsRef, reportID, debouncedSaveReportComment], ); /** @@ -313,14 +336,8 @@ function ComposerWithSuggestions({ * @param {Boolean} shouldAddTrailSpace */ const replaceSelectionWithText = useCallback( - (text, shouldAddTrailSpace = true) => { - const updatedText = shouldAddTrailSpace ? `${text} ` : text; - const selectionSpaceLength = shouldAddTrailSpace ? CONST.SPACE_LENGTH : 0; - updateComment(ComposerUtils.insertText(commentRef.current, selection, updatedText)); - setSelection((prevSelection) => ({ - start: prevSelection.start + text.length + selectionSpaceLength, - end: prevSelection.start + text.length + selectionSpaceLength, - })); + (text) => { + updateComment(ComposerUtils.insertText(commentRef.current, selection, text)); }, [selection, updateComment], ); @@ -508,6 +525,10 @@ function ComposerWithSuggestions({ // eslint-disable-next-line react-hooks/exhaustive-deps }, []); + useEffect(() => { + lastTextRef.current = value; + }, [value]); + useImperativeHandle( forwardedRef, () => ({ From 6db20146c0d0d11ff350255c0ba8e6e02c5020b3 Mon Sep 17 00:00:00 2001 From: Aswin S Date: Sun, 22 Oct 2023 06:18:40 +0530 Subject: [PATCH 02/13] fix: prevent infinite loop --- src/libs/ComposerUtils/index.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/libs/ComposerUtils/index.ts b/src/libs/ComposerUtils/index.ts index 987615f17695..bf22fcb04a49 100644 --- a/src/libs/ComposerUtils/index.ts +++ b/src/libs/ComposerUtils/index.ts @@ -35,7 +35,8 @@ function getCommonSuffixLength(str1: string, str2: string): number { if(str1.length===0||str2.length===0){ return 0; } - while (str1[str1.length - 1 - i] === str2[str2.length - 1 - i]) { + const minLen = Math.min(str1.length, str2.length); + while (i>minLen && str1[str1.length - 1 - i] === str2[str2.length - 1 - i]) { i++; } return i; From a8bd00576b2efad25380615217eb9904203dcda4 Mon Sep 17 00:00:00 2001 From: Aswin S Date: Sun, 22 Oct 2023 06:24:05 +0530 Subject: [PATCH 03/13] fix: clean lint --- src/libs/ComposerUtils/index.ts | 4 ++-- .../ComposerWithSuggestions.js | 16 +++++++++++++--- 2 files changed, 15 insertions(+), 5 deletions(-) diff --git a/src/libs/ComposerUtils/index.ts b/src/libs/ComposerUtils/index.ts index bf22fcb04a49..3167ce851e60 100644 --- a/src/libs/ComposerUtils/index.ts +++ b/src/libs/ComposerUtils/index.ts @@ -32,11 +32,11 @@ function canSkipTriggerHotkeys(isSmallScreenWidth: boolean, isKeyboardShown: boo */ function getCommonSuffixLength(str1: string, str2: string): number { let i = 0; - if(str1.length===0||str2.length===0){ + if (str1.length === 0 || str2.length === 0) { return 0; } const minLen = Math.min(str1.length, str2.length); - while (i>minLen && str1[str1.length - 1 - i] === str2[str2.length - 1 - i]) { + while (i < minLen && str1[str1.length - 1 - i] === str2[str2.length - 1 - i]) { i++; } return i; diff --git a/src/pages/home/report/ReportActionCompose/ComposerWithSuggestions.js b/src/pages/home/report/ReportActionCompose/ComposerWithSuggestions.js index a1950ad2a96e..d8b3bc8f820a 100644 --- a/src/pages/home/report/ReportActionCompose/ComposerWithSuggestions.js +++ b/src/pages/home/report/ReportActionCompose/ComposerWithSuggestions.js @@ -198,7 +198,7 @@ function ComposerWithSuggestions({ const findNewlyAddedChars = useCallback( (prevText, newText) => { const isTextReplace = selection.end - selection.start > 0; - const commonSuffixLength =ComposerUtils.getCommonSuffixLength(prevText, newText); + const commonSuffixLength = ComposerUtils.getCommonSuffixLength(prevText, newText); let startIndex = -1; let endIndex = -1; let i = 0; @@ -210,7 +210,7 @@ function ComposerWithSuggestions({ if (i < newText.length) { startIndex = i; // if text is getting pasted over find length of common suffix and subtract it from new text length - endIndex = isTextReplace ? newText.length-commonSuffixLength : i + (newText.length - prevText.length); + endIndex = isTextReplace ? newText.length - commonSuffixLength : i + (newText.length - prevText.length); } return {startIndex, endIndex, diff: newText.substring(startIndex, endIndex)}; @@ -287,7 +287,17 @@ function ComposerWithSuggestions({ debouncedBroadcastUserIsTyping(reportID); } }, - [raiseIsScrollLikelyLayoutTriggered, findNewlyAddedChars, preferredSkinTone, preferredLocale, setIsCommentEmpty, debouncedUpdateFrequentlyUsedEmojis, suggestionsRef, reportID, debouncedSaveReportComment], + [ + raiseIsScrollLikelyLayoutTriggered, + findNewlyAddedChars, + preferredSkinTone, + preferredLocale, + setIsCommentEmpty, + debouncedUpdateFrequentlyUsedEmojis, + suggestionsRef, + reportID, + debouncedSaveReportComment, + ], ); /** From ec3284d9e078d05e72f23eee3ab0578be9643d56 Mon Sep 17 00:00:00 2001 From: Aswin S Date: Thu, 26 Oct 2023 07:27:32 +0530 Subject: [PATCH 04/13] fix: remove extra white space getting added --- src/libs/EmojiUtils.js | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/src/libs/EmojiUtils.js b/src/libs/EmojiUtils.js index 05ad1bd3c2ce..9f3931e59530 100644 --- a/src/libs/EmojiUtils.js +++ b/src/libs/EmojiUtils.js @@ -360,19 +360,13 @@ function replaceEmojis(text, preferredSkinTone = CONST.EMOJI_DEFAULT_SKIN_TONE, } } if (checkEmoji && checkEmoji.metaData.code) { - let emojiReplacement = getEmojiCodeWithSkinColor(checkEmoji.metaData, preferredSkinTone); + const emojiReplacement = getEmojiCodeWithSkinColor(checkEmoji.metaData, preferredSkinTone); emojis.push({ name, code: checkEmoji.metaData.code, types: checkEmoji.metaData.types, }); - // If this is the last emoji in the message and it's the end of the message so far, - // add a space after it so the user can keep typing easily. - if (i === emojiData.length - 1) { - emojiReplacement += ' '; - } - newText = newText.replace(emojiData[i], emojiReplacement); } } From 51406fcd2abd28cf9de862c072a8d7c1b5c7efad Mon Sep 17 00:00:00 2001 From: Aswin S Date: Thu, 26 Oct 2023 07:27:43 +0530 Subject: [PATCH 05/13] refactor and add comments --- .../ComposerWithSuggestions.js | 30 +++++++++++++------ 1 file changed, 21 insertions(+), 9 deletions(-) diff --git a/src/pages/home/report/ReportActionCompose/ComposerWithSuggestions.js b/src/pages/home/report/ReportActionCompose/ComposerWithSuggestions.js index 183152088f8d..bcfeb3d74cf9 100644 --- a/src/pages/home/report/ReportActionCompose/ComposerWithSuggestions.js +++ b/src/pages/home/report/ReportActionCompose/ComposerWithSuggestions.js @@ -196,25 +196,37 @@ function ComposerWithSuggestions({ RNTextInputReset.resetKeyboardInput(findNodeHandle(textInputRef.current)); }, [textInputRef]); + /** + * Find diff between text changes in composer + */ const findNewlyAddedChars = useCallback( (prevText, newText) => { - const isTextReplace = selection.end - selection.start > 0; - const commonSuffixLength = ComposerUtils.getCommonSuffixLength(prevText, newText); let startIndex = -1; let endIndex = -1; - let i = 0; + let currentIndex = 0; - while (i < newText.length && prevText.charAt(i) === newText.charAt(i) && selection.start > i) { - i++; + // Find the first character mismatch with newText + while (currentIndex < newText.length && prevText.charAt(currentIndex) === newText.charAt(currentIndex) && selection.start > currentIndex) { + currentIndex++; } - if (i < newText.length) { - startIndex = i; + if (currentIndex < newText.length) { + startIndex = currentIndex; + // if text is getting pasted over find length of common suffix and subtract it from new text length - endIndex = isTextReplace ? newText.length - commonSuffixLength : i + (newText.length - prevText.length); + if (selection.end - selection.start > 0) { + const commonSuffixLength = ComposerUtils.getCommonSuffixLength(prevText, newText); + endIndex = newText.length - commonSuffixLength; + } else { + endIndex = currentIndex + (newText.length - prevText.length); + } } - return {startIndex, endIndex, diff: newText.substring(startIndex, endIndex)}; + return { + startIndex, + endIndex, + diff: newText.substring(startIndex, endIndex), + }; }, [selection.end, selection.start], ); From 12273a3bae0be54175ccad1c8978592a16e73a0d Mon Sep 17 00:00:00 2001 From: Aswin S Date: Thu, 26 Oct 2023 09:57:14 +0530 Subject: [PATCH 06/13] fix: remove redundant test cases --- tests/unit/EmojiTest.js | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/tests/unit/EmojiTest.js b/tests/unit/EmojiTest.js index d10da618480e..d1f69b8c3384 100644 --- a/tests/unit/EmojiTest.js +++ b/tests/unit/EmojiTest.js @@ -101,16 +101,6 @@ describe('EmojiTest', () => { expect(EmojiUtils.containsOnlyEmojis('🅃🄴🅂🅃')).toBe(false); }); - it('replaces an emoji code with an emoji and a space', () => { - const text = 'Hi :smile:'; - expect(lodashGet(EmojiUtils.replaceEmojis(text), 'text')).toBe('Hi 😄 '); - }); - - it('will add a space after the last emoji if there is text after it', () => { - const text = 'Hi :smile::wave:space after last emoji'; - expect(lodashGet(EmojiUtils.replaceEmojis(text), 'text')).toBe('Hi 😄👋 space after last emoji'); - }); - it('suggests emojis when typing emojis prefix after colon', () => { const text = 'Hi :coffin'; expect(EmojiUtils.suggestEmojis(text, 'en')).toEqual([{code: '⚰️', name: 'coffin'}]); From bb6bd37d2ba450d56068bd3b608d8e74bd6a2476 Mon Sep 17 00:00:00 2001 From: Aswin S Date: Thu, 26 Oct 2023 10:00:00 +0530 Subject: [PATCH 07/13] remove unused import --- tests/unit/EmojiTest.js | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/unit/EmojiTest.js b/tests/unit/EmojiTest.js index d1f69b8c3384..2be35467a163 100644 --- a/tests/unit/EmojiTest.js +++ b/tests/unit/EmojiTest.js @@ -1,7 +1,6 @@ import _ from 'underscore'; import {getUnixTime} from 'date-fns'; import Onyx from 'react-native-onyx'; -import lodashGet from 'lodash/get'; import Emoji from '../../assets/emojis'; import * as EmojiUtils from '../../src/libs/EmojiUtils'; import ONYXKEYS from '../../src/ONYXKEYS'; From f00a383dcdcb9b017712cafa9fd2571a9d678e39 Mon Sep 17 00:00:00 2001 From: Aswin S Date: Fri, 27 Oct 2023 20:06:35 +0530 Subject: [PATCH 08/13] fix: clean lint --- .../home/report/ReportActionCompose/ComposerWithSuggestions.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/pages/home/report/ReportActionCompose/ComposerWithSuggestions.js b/src/pages/home/report/ReportActionCompose/ComposerWithSuggestions.js index f9a50fdb8c73..3b7690482a62 100644 --- a/src/pages/home/report/ReportActionCompose/ComposerWithSuggestions.js +++ b/src/pages/home/report/ReportActionCompose/ComposerWithSuggestions.js @@ -255,7 +255,8 @@ function ComposerWithSuggestions({ const isEmojiInserted = diff.length && endIndex > startIndex && EmojiUtils.containsOnlyEmojis(diff); const {text: newComment, emojis} = EmojiUtils.replaceAndExtractEmojis( isEmojiInserted ? insertWhiteSpace(commentValue, endIndex) : commentValue, - preferredSkinTone, preferredLocale); + preferredSkinTone, + preferredLocale, ); if (!_.isEmpty(emojis)) { From b1213cd23cc3f389abf1c4dcc57f8cf638280a49 Mon Sep 17 00:00:00 2001 From: Aswin S Date: Fri, 27 Oct 2023 20:13:47 +0530 Subject: [PATCH 09/13] fix: prettify code --- .imgbotconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.imgbotconfig b/.imgbotconfig index 9996ba809af1..ff5c3345cc4d 100644 --- a/.imgbotconfig +++ b/.imgbotconfig @@ -1,6 +1,6 @@ { "ignoredFiles": [ - "assets/images/empty-state_background-fade.png", // Caused an issue with colour gradients, https://github.com/Expensify/App/issues/30499 + "assets/images/empty-state_background-fade.png" // Caused an issue with colour gradients, https://github.com/Expensify/App/issues/30499 ], "aggressiveCompression": "false" } From 9f3653ce3963edec6d275b395632a321ea3751d7 Mon Sep 17 00:00:00 2001 From: Aswin S Date: Sat, 28 Oct 2023 14:25:50 +0530 Subject: [PATCH 10/13] fix: revert change to EmojiUtils --- src/libs/EmojiUtils.js | 8 +++++++- tests/unit/EmojiTest.js | 11 +++++++++++ 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/src/libs/EmojiUtils.js b/src/libs/EmojiUtils.js index 9f3931e59530..05ad1bd3c2ce 100644 --- a/src/libs/EmojiUtils.js +++ b/src/libs/EmojiUtils.js @@ -360,13 +360,19 @@ function replaceEmojis(text, preferredSkinTone = CONST.EMOJI_DEFAULT_SKIN_TONE, } } if (checkEmoji && checkEmoji.metaData.code) { - const emojiReplacement = getEmojiCodeWithSkinColor(checkEmoji.metaData, preferredSkinTone); + let emojiReplacement = getEmojiCodeWithSkinColor(checkEmoji.metaData, preferredSkinTone); emojis.push({ name, code: checkEmoji.metaData.code, types: checkEmoji.metaData.types, }); + // If this is the last emoji in the message and it's the end of the message so far, + // add a space after it so the user can keep typing easily. + if (i === emojiData.length - 1) { + emojiReplacement += ' '; + } + newText = newText.replace(emojiData[i], emojiReplacement); } } diff --git a/tests/unit/EmojiTest.js b/tests/unit/EmojiTest.js index 2be35467a163..d10da618480e 100644 --- a/tests/unit/EmojiTest.js +++ b/tests/unit/EmojiTest.js @@ -1,6 +1,7 @@ import _ from 'underscore'; import {getUnixTime} from 'date-fns'; import Onyx from 'react-native-onyx'; +import lodashGet from 'lodash/get'; import Emoji from '../../assets/emojis'; import * as EmojiUtils from '../../src/libs/EmojiUtils'; import ONYXKEYS from '../../src/ONYXKEYS'; @@ -100,6 +101,16 @@ describe('EmojiTest', () => { expect(EmojiUtils.containsOnlyEmojis('🅃🄴🅂🅃')).toBe(false); }); + it('replaces an emoji code with an emoji and a space', () => { + const text = 'Hi :smile:'; + expect(lodashGet(EmojiUtils.replaceEmojis(text), 'text')).toBe('Hi 😄 '); + }); + + it('will add a space after the last emoji if there is text after it', () => { + const text = 'Hi :smile::wave:space after last emoji'; + expect(lodashGet(EmojiUtils.replaceEmojis(text), 'text')).toBe('Hi 😄👋 space after last emoji'); + }); + it('suggests emojis when typing emojis prefix after colon', () => { const text = 'Hi :coffin'; expect(EmojiUtils.suggestEmojis(text, 'en')).toEqual([{code: '⚰️', name: 'coffin'}]); From 40a6f855bc79f3b7fbba2c60c1e122922743d526 Mon Sep 17 00:00:00 2001 From: Aswin S Date: Tue, 31 Oct 2023 11:00:08 +0530 Subject: [PATCH 11/13] fix: reset cursor to last position on focus --- .../report/ReportActionCompose/ComposerWithSuggestions.js | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/pages/home/report/ReportActionCompose/ComposerWithSuggestions.js b/src/pages/home/report/ReportActionCompose/ComposerWithSuggestions.js index b55927e56edd..ee287bda07c5 100644 --- a/src/pages/home/report/ReportActionCompose/ComposerWithSuggestions.js +++ b/src/pages/home/report/ReportActionCompose/ComposerWithSuggestions.js @@ -486,7 +486,12 @@ function ComposerWithSuggestions({ } focus(); - replaceSelectionWithText(e.key, false); + // Reset cursor to last known location + setSelection((prevSelection) => ({ + start: prevSelection.start+1 , + end: prevSelection.end+1, + })); + replaceSelectionWithText(e.key); }, [checkComposerVisibility, focus, replaceSelectionWithText], ); From c66bc8cb181aac505c4dd222d273587ddfbb5b86 Mon Sep 17 00:00:00 2001 From: Aswin S Date: Tue, 31 Oct 2023 11:02:59 +0530 Subject: [PATCH 12/13] prettify code --- .../report/ReportActionCompose/ComposerWithSuggestions.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/pages/home/report/ReportActionCompose/ComposerWithSuggestions.js b/src/pages/home/report/ReportActionCompose/ComposerWithSuggestions.js index ee287bda07c5..efc0b88fe5c2 100644 --- a/src/pages/home/report/ReportActionCompose/ComposerWithSuggestions.js +++ b/src/pages/home/report/ReportActionCompose/ComposerWithSuggestions.js @@ -488,8 +488,8 @@ function ComposerWithSuggestions({ focus(); // Reset cursor to last known location setSelection((prevSelection) => ({ - start: prevSelection.start+1 , - end: prevSelection.end+1, + start: prevSelection.start + 1, + end: prevSelection.end + 1, })); replaceSelectionWithText(e.key); }, From f2c1022794bd7d1bda6b80ce2297493e7c1b0386 Mon Sep 17 00:00:00 2001 From: Aswin S Date: Wed, 1 Nov 2023 20:39:47 +0530 Subject: [PATCH 13/13] Add params for jsDoc --- .../ReportActionCompose/ComposerWithSuggestions.js | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/pages/home/report/ReportActionCompose/ComposerWithSuggestions.js b/src/pages/home/report/ReportActionCompose/ComposerWithSuggestions.js index efc0b88fe5c2..b306676d476a 100644 --- a/src/pages/home/report/ReportActionCompose/ComposerWithSuggestions.js +++ b/src/pages/home/report/ReportActionCompose/ComposerWithSuggestions.js @@ -198,7 +198,14 @@ function ComposerWithSuggestions({ }, [textInputRef]); /** - * Find diff between text changes in composer + * Find the newly added characters between the previous text and the new text based on the selection. + * + * @param {string} prevText - The previous text. + * @param {string} newText - The new text. + * @returns {object} An object containing information about the newly added characters. + * @property {number} startIndex - The start index of the newly added characters in the new text. + * @property {number} endIndex - The end index of the newly added characters in the new text. + * @property {string} diff - The newly added characters. */ const findNewlyAddedChars = useCallback( (prevText, newText) => {