[HOLD for payment 2022-11-18] [$250] Space is not added after rendering emoji via text reported by @adeel0202

[kavimuru]

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!

---

Action Performed:

1. Go to any chat
2. Type :wave: in composer

Expected Result:

Space is added after the emoji

Actual Result:

Space is not added after the emoji

Workaround:

unknown

Platform:

Where is this issue occurring?

• Web
• iOS
• Android
• Desktop App
• Mobile Web

Version Number: 1.2.22-1

Reproducible in staging?: y

Reproducible in production?: y

Email or phone of affected tester (no customers):

Logs: https://stackoverflow.com/c/expensify/questions/4856

Notes/Photos/Videos:

https://user-images.githubusercontent.com/43996225/199114556-cfce4d7d-56d7-4cc3-b96e-a6365ff640e6.mp4

https://user-images.githubusercontent.com/43996225/199114573-ad97238d-9e19-43e1-8603-e9eb89ec1838.mov

Expensify/Expensify Issue URL:

Issue reported by: @adeel0202

Slack conversation: https://expensify.slack.com/archives/C01GTK53T8Q/p1667208968300839

View all open jobs on GitHub

[melvin-bot[bot]]

Triggered auto assignment to @dylanexpensify (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

[Puneet-here]

Proposal

We just have to return

`${newText} `

https://github.com/Expensify/App/blob/2174ef50cd16fb3da074ef5dce880ac1653e7d23/src/libs/EmojiUtils.js#L201

[adeel0202]

@Puneet-here you should've waited for the Help Wanted label before posting your proposal.

[Puneet-here]

@adeel0202, proposals can be posted any time after the issue gets created. Please refer here.

[melvin-bot[bot]]

Current assignee @dylanexpensify is eligible for the External assigner, not assigning anyone new.

[melvin-bot[bot]]

Triggered auto assignment to Contributor-plus team member for initial proposal review - @rushatgabhane (External)

[melvin-bot[bot]]

Triggered auto assignment to @neil-marcellini (External), see https://stackoverflow.com/c/expensify/questions/7972 for more details.

[dylanexpensify]

Upwork: https://www.upwork.com/jobs/~01b305b99cbec3f000

[StefanNemeth]

I'd like to work on it. Should be quite straightforward to change by adding a space when replacing the emojis in EmojiUtils. It might make sense to not do it in case the text is pasted in as this might cause unwanted spaces.

[officialksolomon]

MY PROPOSAL FOR [$250] Space is not added after rendering emoji via text reported by @adeel0202 #12314 @dylanexpensify

Situation: Space is not added after rendering emoji via text.

Solution: Use regular expression to detect emoji inputs, separate emojis from normal text and perform necessary string manipulation to add space to the beginning of the separated text.

Steps:

1. Create a function that detects the emoji text.
2. Create a function for the string manipulations
3. Store manipulated text with emoji in state variable.
4. And update input value.

Benefits: My solution accounts for bot typed and pasted text.

[neil-marcellini]

This is a feature not a bug. The implementation comes from this commit from this PR to implement this feature request issue.

https://github.com/Expensify/App/blob/064eeea9e36bc4cb8cacb3b92ad02acf8ea6b35d/src/pages/home/report/ReportActionCompose.js#L315-L321

[adeel0202]

@neil-marcellini I think you misunderstood the bug here. The bug report states that the space is NOT added when we render an emoji via text. After that feature implementation you mentioned, a space is now added when we choose emoji from the emojis picker, however, when we add an emoji via text then there is no space after the emoji so it is a bug in this case.

[neil-marcellini]

Oh ok, thanks for clearing that up @adeel0202! You are right, I'll try to read more carefully next time.

[neil-marcellini]

At first glance I like @Puneet-here's proposal https://github.com/Expensify/App/issues/12314#issuecomment-1297727461, but @adeel0202 brings up a good point that it doesn't work properly when pasting in text with emojis.

When I type :apple: in the composer box it should replace that with an apple and a space following it, e.x "🍎 ", so that I can keep typing with ease. However, if I past in some text like this :smile::apple::hamburger:no space after emojis then I want to see the emojis right next to each other and text immediately following the last emoji, e.x "😄🍎🍔no space after emojis". In other words, I think we should only manually append a space after an emoji if it is the last character in the text. Do you guys agree?

If so, please submit a proposal that handles these cases.

[StefanNemeth]

When I type :apple: in the composer box it should replace that with an apple and a space following it, e.x "🍎 ", so that I can keep typing with ease. However, if I past in some text like this :smile::apple::hamburger:no space after emojis then I want to see the emojis right next to each other and text immediately following the last emoji, e.x "😄🍎🍔no space after emojis". In other words, I think we should only manually append a space after an emoji if it is the last character in the text. Do you guys agree?

I definitely agree, seems better to add a space if the last character was changed to an emoji instead of checking whether the user is pasting something and adding unnecessary logic. I would not change the EmojiUtils function but rather the updateComment function due to separation of concerns.

Following proposal should do the trick.

updateComment(comment, shouldDebounceSaveComment) {
        let newComment = EmojiUtils.replaceEmojis(comment);

        // If last character changed, then we know it was an emoji
        const emojiAtEnd = newComment.length > 0 && comment.charAt(comment.length - 1) !== newComment.charAt(newComment.length - 1);

        // If emoji is at the end of the comment, add a space for better typing experience
        if (emojiAtEnd) {
            newComment += ' ';
        }

[Puneet-here]

Updated proposal

Check the emojiData.length

    if (emojiData.length > 1) {
        return newText;
    }
    return `${newText} `;

https://github.com/Expensify/App/blob/2174ef50cd16fb3da074ef5dce880ac1653e7d23/src/libs/EmojiUtils.js#L201

[Karim-30]

in src/libs/EmojiUtils.js :

function replaceEmojis(text) {
    let newText = text;
    const emojiData = text.match(CONST.REGEX.EMOJI_NAME);
    if (!emojiData || emojiData.length === 0) {
        return text;
    }
    for (let i = 0; i < emojiData.length; i++) {
        const checkEmoji = emojisTrie.search(emojiData[i].slice(1, -1));
        if (checkEmoji && checkEmoji.metaData.code) {
-           newText = newText.replace(emojiData[i], checkEmoji.metaData.code);
+           newText = newText.replace(emojiData[i], `${checkEmoji.metaData.code} `);
        }
    }
    return newText;
}

https://user-images.githubusercontent.com/108357004/199337922-d2fc8cd0-6c2f-455a-aabd-45b1eeb71174.mp4

I will not get paid for it.

[rushatgabhane]

@StefanNemeth i like your idea to update updateComment instead of EmojiUtils. Separation of concerns is a good thing because we're gonna completely revamp EmojiUtils.

// If last character changed, then we know it was an emoji const emojiAtEnd = newComment.length > 0 && comment.charAt(comment.length - 1) !== newComment.charAt(newComment.length - 1);

In future, a comment may change due to other reasons. I'm not sure if this is how we should do it.

[rushatgabhane]

In other words, I think we should only manually append a space after an emoji if it is the last character in the text. Do you guys agree

@neil-marcellini what if we're inserting an emoji in the middle of a comment

https://user-images.githubusercontent.com/29673073/199894284-52e84104-31cf-4103-91b8-daea115044fc.mov

[neil-marcellini]

@neil-marcellini what if we're inserting an emoji in the middle of a comment

It's important that if the user pastes in text it shows up as they expect, so we don't want to insert spaces after emojis in the middle of a comment when pasting text, e.x :smile::apple::hamburger:no space after emojis. Maybe we could replace the emojis without adding spaces when we handlePaste?

Alternatively we could let the user add the space themselves when inserting an emoji in the middle of a comment.

[rushatgabhane]

Okayy

Alternatively we could let the user add the space themselves when inserting an emoji in the middle of a comment.

I like this because the user is editing the comment anyway and won't be continuing to type.

and because it's a simpler solution

[neil-marcellini]

Proposal

Insert a space when replacing emojis if the emoji is at the end of the current message.

If @rushatgabhane likes this proposal too then I would like @Puneet-here to go ahead an implement it for me, because he had the first proposal.

diff --git a/src/libs/EmojiUtils.js b/src/libs/EmojiUtils.js
index ed67a2aed..f70413d68 100644
--- a/src/libs/EmojiUtils.js
+++ b/src/libs/EmojiUtils.js
@@ -195,7 +195,13 @@ function replaceEmojis(text) {
     for (let i = 0; i < emojiData.length; i++) {
         const checkEmoji = emojisTrie.search(emojiData[i].slice(1, -1));
         if (checkEmoji && checkEmoji.metaData.code) {
-            newText = newText.replace(emojiData[i], checkEmoji.metaData.code);
+            let emojiReplacement = checkEmoji.metaData.code;
+
+            // 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);
         }
     }
     return newText;
diff --git a/tests/unit/EmojiTest.js b/tests/unit/EmojiTest.js
index 5a66a57e9..387d716a1 100644
--- a/tests/unit/EmojiTest.js
+++ b/tests/unit/EmojiTest.js
@@ -78,9 +78,14 @@ describe('EmojiTest', () => {
         expect(EmojiUtils.containsOnlyEmojis('😄  👋')).toBe(true);
     });

-    it('replaces emoji codes with emojis inside a text', () => {
-        const text = 'Hi :smile::wave:';
-        expect(EmojiUtils.replaceEmojis(text)).toBe('Hi 😄👋');
+    it('replaces an emoji code with an emoji and a space', () => {
+        const text = 'Hi :smile:';
+        expect(EmojiUtils.replaceEmojis(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(EmojiUtils.replaceEmojis(text)).toBe('Hi 😄👋no space after last emoji');
     });

     it('suggests emojis when typing emojis prefix after colon', () => {

[rushatgabhane]

@neil-marcellini

nitpick: an edgecase with this proposal is that it won't add a space for a multiline comment.

[neil-marcellini]

nitpick: an edgecase with this proposal is that it won't add a space for a multiline comment.

That's true, but I think it's just another case where the user is inserting an emoji in the middle of a comment and we already decided that the user can add a space manually.

[rushatgabhane]

yeah, forgot about that 😂

let's get this done then. cc @Puneet-here @neil-marcellini

[neil-marcellini]

@Puneet-here I'm assigning you to implement this proposal https://github.com/Expensify/App/issues/12314#issuecomment-1304016775. Please let me know within a day or so if you can implement this, otherwise I'll pick someone else so we can move this forward quickly. Thanks!

[melvin-bot[bot]]

📣 @Puneet-here You have been assigned to this job by @neil-marcellini! Please apply to this job in Upwork and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻 Keep in mind: Code of Conduct | Contributing 📖

[Puneet-here]

Thanks @neil-marcellini, I have raised the PR.

[melvin-bot[bot]]

BugZero Checklist: The PR fixing this issue has been merged! The following checklist will need to be completed before the issue can be closed:

• [ ] A regression test has been added or updated so that the same bug will not reach production again. Link to the updated test here:
• [x] The PR that introduced the bug has been identified. Link to the PR: N/A missed during initial implementation
• [x] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment: N/A missed during initial implementation
• [x] A discussion in #contributor-plus has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion: N/A missed during initial implementation
• [x] Payment has been made to the issue reporter (if applicable)
• [x] Payment has been made to the contributor that fixed the issue (if applicable)
• [x] Payment has been made to the contributor+ that helped on the issue (if applicable)

[dylanexpensify]

@Puneet-here @rushatgabhane would you mind applying on the Upwork job above please? 🙇