From 861c2c72ac631e9ce9d863b5a68e060c4121eaeb Mon Sep 17 00:00:00 2001 From: Getabalew Tesfaye Date: Tue, 27 Jun 2023 23:48:52 +0300 Subject: [PATCH 01/10] fix space inconsistency inserting emoji --- src/libs/EmojiUtils.js | 5 ----- src/pages/home/report/ReportActionCompose.js | 6 +++--- 2 files changed, 3 insertions(+), 8 deletions(-) diff --git a/src/libs/EmojiUtils.js b/src/libs/EmojiUtils.js index a4325af27c9c..ffdbc9077b25 100644 --- a/src/libs/EmojiUtils.js +++ b/src/libs/EmojiUtils.js @@ -242,11 +242,6 @@ function replaceEmojis(text, isSmallScreenWidth = false, preferredSkinTone = CON 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 (isSmallScreenWidth && i === emojiData.length - 1 && text.endsWith(emojiData[i])) { - emojiReplacement += ' '; - } newText = newText.replace(emojiData[i], emojiReplacement); } } diff --git a/src/pages/home/report/ReportActionCompose.js b/src/pages/home/report/ReportActionCompose.js index 5d83dcdf285a..e1645dde6504 100644 --- a/src/pages/home/report/ReportActionCompose.js +++ b/src/pages/home/report/ReportActionCompose.js @@ -614,7 +614,7 @@ class ReportActionCompose extends React.Component { const emojiCode = emojiObject.types && emojiObject.types[this.props.preferredSkinTone] ? emojiObject.types[this.props.preferredSkinTone] : emojiObject.code; const commentAfterColonWithEmojiNameRemoved = this.state.value.slice(this.state.selection.end); - this.updateComment(`${commentBeforeColon}${emojiCode} ${this.trimLeadingSpace(commentAfterColonWithEmojiNameRemoved)}`, true); + this.updateComment(`${commentBeforeColon}${emojiCode}${this.trimLeadingSpace(commentAfterColonWithEmojiNameRemoved)}`, true); // In some Android phones keyboard, the text to search for the emoji is not cleared // will be added after the user starts typing again on the keyboard. This package is // a workaround to reset the keyboard natively. @@ -623,8 +623,8 @@ class ReportActionCompose extends React.Component { } this.setState((prevState) => ({ selection: { - start: prevState.colonIndex + emojiCode.length + CONST.SPACE_LENGTH, - end: prevState.colonIndex + emojiCode.length + CONST.SPACE_LENGTH, + start: prevState.colonIndex + emojiCode.length, + end: prevState.colonIndex + emojiCode.length, }, suggestedEmojis: [], })); From ff45b46d793a7b030380d5397a388a738057211f Mon Sep 17 00:00:00 2001 From: Getabalew Tesfaye Date: Wed, 28 Jun 2023 00:33:14 +0300 Subject: [PATCH 02/10] remove isSmallScreenWidth params --- src/libs/EmojiUtils.js | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/libs/EmojiUtils.js b/src/libs/EmojiUtils.js index ffdbc9077b25..030e4a1f1eb6 100644 --- a/src/libs/EmojiUtils.js +++ b/src/libs/EmojiUtils.js @@ -220,11 +220,10 @@ const getEmojiCodeWithSkinColor = (item, preferredSkinToneIndex) => { * If we're on mobile, we also add a space after the emoji granted there's no text after it. * * @param {String} text - * @param {Boolean} isSmallScreenWidth * @param {Number} preferredSkinTone * @returns {Object} */ -function replaceEmojis(text, isSmallScreenWidth = false, preferredSkinTone = CONST.EMOJI_DEFAULT_SKIN_TONE) { +function replaceEmojis(text, preferredSkinTone = CONST.EMOJI_DEFAULT_SKIN_TONE) { let newText = text; const emojis = []; const emojiData = text.match(CONST.REGEX.EMOJI_NAME); @@ -235,13 +234,12 @@ function replaceEmojis(text, isSmallScreenWidth = false, preferredSkinTone = CON const name = emojiData[i].slice(1, -1); const checkEmoji = emojisTrie.search(name); 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, }); - newText = newText.replace(emojiData[i], emojiReplacement); } } From 7a3cc95b91c2f79a31e1c92bdac836d4bd259331 Mon Sep 17 00:00:00 2001 From: Getabalew Tesfaye Date: Wed, 28 Jun 2023 00:34:15 +0300 Subject: [PATCH 03/10] remove isSmallScreenWidth args --- src/pages/home/report/ReportActionCompose.js | 2 +- src/pages/home/report/ReportActionItemMessageEdit.js | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/pages/home/report/ReportActionCompose.js b/src/pages/home/report/ReportActionCompose.js index e1645dde6504..2662b461d37e 100644 --- a/src/pages/home/report/ReportActionCompose.js +++ b/src/pages/home/report/ReportActionCompose.js @@ -721,7 +721,7 @@ class ReportActionCompose extends React.Component { * @param {Boolean} shouldDebounceSaveComment */ updateComment(comment, shouldDebounceSaveComment) { - const {text: newComment = '', emojis = []} = EmojiUtils.replaceEmojis(comment, this.props.isSmallScreenWidth, this.props.preferredSkinTone); + const {text: newComment = '', emojis = []} = EmojiUtils.replaceEmojis(comment, this.props.preferredSkinTone); if (!_.isEmpty(emojis)) { User.updateFrequentlyUsedEmojis(EmojiUtils.getFrequentlyUsedEmojis(emojis)); diff --git a/src/pages/home/report/ReportActionItemMessageEdit.js b/src/pages/home/report/ReportActionItemMessageEdit.js index 56b78a73ae50..1637a72cf927 100644 --- a/src/pages/home/report/ReportActionItemMessageEdit.js +++ b/src/pages/home/report/ReportActionItemMessageEdit.js @@ -144,7 +144,7 @@ function ReportActionItemMessageEdit(props) { */ const updateDraft = useCallback( (newDraftInput) => { - const {text: newDraft = '', emojis = []} = EmojiUtils.replaceEmojis(newDraftInput, isSmallScreenWidth, props.preferredSkinTone); + const {text: newDraft = '', emojis = []} = EmojiUtils.replaceEmojis(newDraftInput, props.preferredSkinTone); if (!_.isEmpty(emojis)) { User.updateFrequentlyUsedEmojis(EmojiUtils.getFrequentlyUsedEmojis(emojis)); @@ -171,7 +171,7 @@ function ReportActionItemMessageEdit(props) { debouncedSaveDraft(props.action.message[0].html); } }, - [props.action.message, debouncedSaveDraft, isSmallScreenWidth, props.preferredSkinTone], + [props.action.message, debouncedSaveDraft, props.preferredSkinTone], ); /** From edcf92790e7db5aa4b99ad98e7a0b953304a0214 Mon Sep 17 00:00:00 2001 From: Getabalew Tesfaye Date: Wed, 28 Jun 2023 00:34:38 +0300 Subject: [PATCH 04/10] remove unecessary tests --- tests/unit/EmojiTest.js | 14 ++------------ 1 file changed, 2 insertions(+), 12 deletions(-) diff --git a/tests/unit/EmojiTest.js b/tests/unit/EmojiTest.js index 8fc592ab0fc4..2d2fa2f4981f 100644 --- a/tests/unit/EmojiTest.js +++ b/tests/unit/EmojiTest.js @@ -101,9 +101,9 @@ describe('EmojiTest', () => { expect(EmojiUtils.containsOnlyEmojis('🅃🄴🅂🅃')).toBe(false); }); - it('replaces an emoji code with an emoji and a space on mobile', () => { + it('replaces an emoji code with an emoji and a no space', () => { const text = 'Hi :smile:'; - expect(lodashGet(EmojiUtils.replaceEmojis(text, true), 'text')).toBe('Hi 😄 '); + expect(lodashGet(EmojiUtils.replaceEmojis(text), 'text')).toBe('Hi 😄'); }); it('will not add a space after the last emoji if there is text after it', () => { @@ -111,16 +111,6 @@ describe('EmojiTest', () => { expect(lodashGet(EmojiUtils.replaceEmojis(text), 'text')).toBe('Hi 😄👋no space after last emoji'); }); - it('will not add a space after the last emoji when there is text after it on mobile', () => { - const text = 'Hi :smile::wave:no space after last emoji'; - expect(lodashGet(EmojiUtils.replaceEmojis(text, true), 'text')).toBe('Hi 😄👋no space after last emoji'); - }); - - it("will not add a space after the last emoji if we're not on mobile", () => { - const text = 'Hi :smile:'; - expect(lodashGet(EmojiUtils.replaceEmojis(text), 'text')).toBe('Hi 😄'); - }); - it('suggests emojis when typing emojis prefix after colon', () => { const text = 'Hi :coffin'; expect(EmojiUtils.suggestEmojis(text)).toEqual([{code: '⚰️', name: 'coffin'}]); From 8b0ea26c0625a7946c80a5b1a1becb469224b903 Mon Sep 17 00:00:00 2001 From: Getabalew Tesfaye Date: Wed, 28 Jun 2023 09:58:18 +0300 Subject: [PATCH 05/10] add space on every platform on emoji insertion --- src/libs/EmojiUtils.js | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/libs/EmojiUtils.js b/src/libs/EmojiUtils.js index 030e4a1f1eb6..14c5d1222d85 100644 --- a/src/libs/EmojiUtils.js +++ b/src/libs/EmojiUtils.js @@ -234,12 +234,19 @@ function replaceEmojis(text, preferredSkinTone = CONST.EMOJI_DEFAULT_SKIN_TONE) const name = emojiData[i].slice(1, -1); const checkEmoji = emojisTrie.search(name); 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 && text.endsWith(emojiData[i])) { + emojiReplacement += ' '; + } + newText = newText.replace(emojiData[i], emojiReplacement); } } From 6ed4d5a9463d29c150a52492dc5d30ce6d24f712 Mon Sep 17 00:00:00 2001 From: Getabalew Tesfaye Date: Wed, 28 Jun 2023 09:59:13 +0300 Subject: [PATCH 06/10] make sure to add emoji after insertion --- src/pages/home/report/ReportActionCompose.js | 6 +++--- tests/unit/EmojiTest.js | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/pages/home/report/ReportActionCompose.js b/src/pages/home/report/ReportActionCompose.js index 2662b461d37e..dcb79c06ad57 100644 --- a/src/pages/home/report/ReportActionCompose.js +++ b/src/pages/home/report/ReportActionCompose.js @@ -614,7 +614,7 @@ class ReportActionCompose extends React.Component { const emojiCode = emojiObject.types && emojiObject.types[this.props.preferredSkinTone] ? emojiObject.types[this.props.preferredSkinTone] : emojiObject.code; const commentAfterColonWithEmojiNameRemoved = this.state.value.slice(this.state.selection.end); - this.updateComment(`${commentBeforeColon}${emojiCode}${this.trimLeadingSpace(commentAfterColonWithEmojiNameRemoved)}`, true); + this.updateComment(`${commentBeforeColon}${emojiCode} ${this.trimLeadingSpace(commentAfterColonWithEmojiNameRemoved)}`, true); // In some Android phones keyboard, the text to search for the emoji is not cleared // will be added after the user starts typing again on the keyboard. This package is // a workaround to reset the keyboard natively. @@ -623,8 +623,8 @@ class ReportActionCompose extends React.Component { } this.setState((prevState) => ({ selection: { - start: prevState.colonIndex + emojiCode.length, - end: prevState.colonIndex + emojiCode.length, + start: prevState.colonIndex + emojiCode.length + CONST.SPACE_LENGTH, + end: prevState.colonIndex + emojiCode.length + CONST.SPACE_LENGTH, }, suggestedEmojis: [], })); diff --git a/tests/unit/EmojiTest.js b/tests/unit/EmojiTest.js index 2d2fa2f4981f..e054bc627467 100644 --- a/tests/unit/EmojiTest.js +++ b/tests/unit/EmojiTest.js @@ -101,9 +101,9 @@ describe('EmojiTest', () => { expect(EmojiUtils.containsOnlyEmojis('🅃🄴🅂🅃')).toBe(false); }); - it('replaces an emoji code with an emoji and a no space', () => { + it('replaces an emoji code with an emoji and a space', () => { const text = 'Hi :smile:'; - expect(lodashGet(EmojiUtils.replaceEmojis(text), 'text')).toBe('Hi 😄'); + expect(lodashGet(EmojiUtils.replaceEmojis(text), 'text')).toBe('Hi 😄 '); }); it('will not add a space after the last emoji if there is text after it', () => { From a5f2f2c92f8f6615682e2e93c77013de3f3e24d6 Mon Sep 17 00:00:00 2001 From: Getabalew Tesfaye Date: Wed, 28 Jun 2023 11:06:07 +0300 Subject: [PATCH 07/10] add space if the emoji is at end selecting from picker --- src/pages/home/report/ReportActionCompose.js | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/pages/home/report/ReportActionCompose.js b/src/pages/home/report/ReportActionCompose.js index dcb79c06ad57..37837c12c8c3 100644 --- a/src/pages/home/report/ReportActionCompose.js +++ b/src/pages/home/report/ReportActionCompose.js @@ -662,11 +662,13 @@ class ReportActionCompose extends React.Component { * @param {String} emoji */ addEmojiToTextBox(emoji) { - this.updateComment(ComposerUtils.insertText(this.comment, this.state.selection, emoji)); + const isEmojiAtEnd = this.state.selection.start === this.comment.length; + this.updateComment(ComposerUtils.insertText(this.comment, this.state.selection, isEmojiAtEnd ? `${emoji} ` : emoji)); + this.setState((prevState) => ({ selection: { - start: prevState.selection.start + emoji.length, - end: prevState.selection.start + emoji.length, + start: prevState.selection.start + emoji.length + (isEmojiAtEnd ? CONST.SPACE_LENGTH : 0), + end: prevState.selection.start + emoji.length + (isEmojiAtEnd ? CONST.SPACE_LENGTH : 0), }, })); } From a2b13f8b1bc86182b0aa74c5df60cfe53358779e Mon Sep 17 00:00:00 2001 From: Getabalew Tesfaye Date: Wed, 28 Jun 2023 11:06:37 +0300 Subject: [PATCH 08/10] add space if the emoji is at end for editing message --- src/pages/home/report/ReportActionItemMessageEdit.js | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/pages/home/report/ReportActionItemMessageEdit.js b/src/pages/home/report/ReportActionItemMessageEdit.js index 1637a72cf927..f45049345e62 100644 --- a/src/pages/home/report/ReportActionItemMessageEdit.js +++ b/src/pages/home/report/ReportActionItemMessageEdit.js @@ -225,11 +225,13 @@ function ReportActionItemMessageEdit(props) { * @param {String} emoji */ const addEmojiToTextBox = (emoji) => { + const isEmojiAtEnd = selection.start === draft.length; + setSelection((prevSelection) => ({ - start: prevSelection.start + emoji.length, - end: prevSelection.start + emoji.length, + start: prevSelection.start + emoji.length + (isEmojiAtEnd ? CONST.SPACE_LENGTH : 0), + end: prevSelection.start + emoji.length + (isEmojiAtEnd ? CONST.SPACE_LENGTH : 0), })); - updateDraft(ComposerUtils.insertText(draft, selection, emoji)); + updateDraft(ComposerUtils.insertText(draft, selection, isEmojiAtEnd ? `${emoji} ` : emoji)); }; /** From 45f02e05679cbe060549a63db0792a5c4220bd6b Mon Sep 17 00:00:00 2001 From: Getabalew Tesfaye Date: Wed, 28 Jun 2023 15:19:46 +0300 Subject: [PATCH 09/10] remove a condition to add a space after emoji insertion --- src/libs/EmojiUtils.js | 2 +- src/pages/home/report/ReportActionCompose.js | 8 +++----- src/pages/home/report/ReportActionItemMessageEdit.js | 8 +++----- 3 files changed, 7 insertions(+), 11 deletions(-) diff --git a/src/libs/EmojiUtils.js b/src/libs/EmojiUtils.js index 14c5d1222d85..9d2ed855cebf 100644 --- a/src/libs/EmojiUtils.js +++ b/src/libs/EmojiUtils.js @@ -243,7 +243,7 @@ function replaceEmojis(text, preferredSkinTone = CONST.EMOJI_DEFAULT_SKIN_TONE) // 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 && text.endsWith(emojiData[i])) { + if (i === emojiData.length - 1) { emojiReplacement += ' '; } diff --git a/src/pages/home/report/ReportActionCompose.js b/src/pages/home/report/ReportActionCompose.js index 37837c12c8c3..4e86086aac30 100644 --- a/src/pages/home/report/ReportActionCompose.js +++ b/src/pages/home/report/ReportActionCompose.js @@ -662,13 +662,11 @@ class ReportActionCompose extends React.Component { * @param {String} emoji */ addEmojiToTextBox(emoji) { - const isEmojiAtEnd = this.state.selection.start === this.comment.length; - this.updateComment(ComposerUtils.insertText(this.comment, this.state.selection, isEmojiAtEnd ? `${emoji} ` : emoji)); - + this.updateComment(ComposerUtils.insertText(this.comment, this.state.selection, `${emoji} `)); this.setState((prevState) => ({ selection: { - start: prevState.selection.start + emoji.length + (isEmojiAtEnd ? CONST.SPACE_LENGTH : 0), - end: prevState.selection.start + emoji.length + (isEmojiAtEnd ? CONST.SPACE_LENGTH : 0), + start: prevState.selection.start + emoji.length + CONST.SPACE_LENGTH, + end: prevState.selection.start + emoji.length + CONST.SPACE_LENGTH, }, })); } diff --git a/src/pages/home/report/ReportActionItemMessageEdit.js b/src/pages/home/report/ReportActionItemMessageEdit.js index f45049345e62..e8414aa969d7 100644 --- a/src/pages/home/report/ReportActionItemMessageEdit.js +++ b/src/pages/home/report/ReportActionItemMessageEdit.js @@ -225,13 +225,11 @@ function ReportActionItemMessageEdit(props) { * @param {String} emoji */ const addEmojiToTextBox = (emoji) => { - const isEmojiAtEnd = selection.start === draft.length; - setSelection((prevSelection) => ({ - start: prevSelection.start + emoji.length + (isEmojiAtEnd ? CONST.SPACE_LENGTH : 0), - end: prevSelection.start + emoji.length + (isEmojiAtEnd ? CONST.SPACE_LENGTH : 0), + start: prevSelection.start + emoji.length + CONST.SPACE_LENGTH, + end: prevSelection.start + emoji.length + CONST.SPACE_LENGTH, })); - updateDraft(ComposerUtils.insertText(draft, selection, isEmojiAtEnd ? `${emoji} ` : emoji)); + updateDraft(ComposerUtils.insertText(draft, selection, `${emoji} `)); }; /** From 2a1caf96e4a29961f2b0019a1dc7c539fc4e1b25 Mon Sep 17 00:00:00 2001 From: Getabalew Tesfaye Date: Wed, 28 Jun 2023 15:29:21 +0300 Subject: [PATCH 10/10] modify tests to match the change --- tests/unit/EmojiTest.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/unit/EmojiTest.js b/tests/unit/EmojiTest.js index e054bc627467..dacd5ec1da9f 100644 --- a/tests/unit/EmojiTest.js +++ b/tests/unit/EmojiTest.js @@ -106,9 +106,9 @@ describe('EmojiTest', () => { expect(lodashGet(EmojiUtils.replaceEmojis(text), 'text')).toBe('Hi 😄 '); }); - it('will not add a space after the last emoji if there is text after it', () => { - const text = 'Hi :smile::wave:no space after last emoji'; - expect(lodashGet(EmojiUtils.replaceEmojis(text), 'text')).toBe('Hi 😄👋no space after last emoji'); + 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', () => {