[HOLD for payment 2022-11-29] [$250] Cursor moves to the end of the message when inserting an emoji in between the words reported by @adeel0202

[kavimuru]

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!

---

Action Performed:

1. Go to any chat
2. Type something in the composer e.g. "hello there"
3. Now type 👋 in between "hello" and "there"

Expected Result:

The cursor is positioned at the end of the emoji

Actual Result:

The cursor is positioned at the end of the message

Workaround:

unknown

Platform:

Where is this issue occurring?

• 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/199127521-33318600-d63e-4c04-a751-30c2dc7c57e3.mp4
https://user-images.githubusercontent.com/43996225/199127573-9bac7276-3186-4e2e-8401-5c148d47b9eb.mov

Expensify/Expensify Issue URL:
Issue reported by: @adeel0202
Slack conversation: https://expensify.slack.com/archives/C01GTK53T8Q/p1667226062750359

View all open jobs on GitHub

[melvin-bot]

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

[melvin-bot]

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

[melvin-bot]

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

[melvin-bot]

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

[laurenreidexpensify]

https://www.upwork.com/jobs/~01ee82e558b5ff525a

[hungvu193]

Proposal
Since we are using setState to replace current comment with new comment with emoji, this issue happen.
Solution
We need to get current position of latest emoji, so we need to move our cursor to that position + length of that emoji.
So in EmojiUtil.js update replaceEmojis function to:

function replaceEmojis(text) {
+  let latestIconPosition = -1;
    let newText = text;
    const emojiData = text.match(CONST.REGEX.EMOJI_NAME);
    if (!emojiData || emojiData.length === 0) {
-       return text
+        return {
+            newComment: newText,
+           latestIconPosition,
        };
    }
    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);
+          latestIconPosition = text.indexOf(emojiData[i]) + checkEmoji.metaData.code.length;
        }
    }
-    return newText
+    return {
+       newComment: newText,
+        latestIconPosition,
    };
}

After that update updateComment function inside ReportActionCompose.js

+  const {newComment, latestIconPosition} = EmojiUtils.replaceEmojis(comment);
        this.setState({
            isCommentEmpty: !!newComment.match(/^(\s|`)*$/),
            value: newComment,
-     }
+        }, () => {
+            if (latestIconPosition !== -1) {
+               this.textInput.selectionEnd = latestIconPosition;
+           }
        });

We are also using replaceEmojis inside ReportActionItemMessageEdit.js (updateDraft function), so we need to update it too:

    updateDraft(draft) {
-        const newDraft = EmojiUtils.replaceEmojis(draft);
+        const { newComment: newDraft, latestIconPosition } = EmojiUtils.replaceEmojis(draft);
-       this.setState({draft: newDraft});
+        this.setState({draft: newDraft}, () => {
+              if (latestIconPosition !== -1) {
+               this.textInput.selectionEnd = latestIconPosition;
+              }
+        })
        // This component is rendered only when draft is set to a non-empty string. In order to prevent component
        // unmount when user deletes content of textarea, we set previous message instead of empty string.
        if (newDraft.trim().length > 0) {
            this.debouncedSaveDraft(newDraft);
        } else {
            this.debouncedSaveDraft(this.props.action.message[0].html);
        }
    }

Updated
To handle paste event, we need to update cursor in case we paste text that had emoji, to do that we need to update paste function inside Composer/index.js:

    paste(text) {
        try {
            document.execCommand('insertText', false, text);
            this.updateNumberOfLines();

            // Pointer will go out of sight when a large paragraph is pasted on the web. Refocusing the input keeps the cursor in view.
            this.textInput.blur();
            this.textInput.focus();
+          this.textInput.selectionStart = text.length;
+          this.textInput.selectionEnd = text.length;
        } catch (e) {}
    }

Here's the result when texting:
https://user-images.githubusercontent.com/16502320/199280393-81a7d7c0-ebf5-4e6f-8bdb-06c5c7dded44.mov

Here's the result when editing:

Screen.Recording.2022-11-02.at.00.35.50.mov

Result for pasting:

Screen.Recording.2022-11-02.at.09.52.50.mov

[yanjankaf]

this should have been closed by now, the proposal @hungvu193 has provided works.

[AndreasBBS]

Proposal
It appears that the e.nativeEvent.selection values are being set to the end of the string in the onSelectionChange handler when the emoji is replaced.

My proposed solution is, when the emoji is replaced, to get the cursor position from previous state and set the selection value in the new state.

In the ReportActionCompose.js function updateComment(comment, shouldDebounceSaveComment) would look like this:

  const newComment = EmojiUtils.replaceEmojis(comment);
+ this.setState((prevState) => {
+     let newState = {
+         isCommentEmpty: !!newComment.match(/^(\s|`)*$/),
+         value: newComment
+     }
+ 
+     if(comment !== newComment) {
+         const remainder = prevState.value.slice(prevState.selection.end).length
+         newState.selection =  {
+             start: newComment.length - remainder,
+             end: newComment.length - remainder
+         }
+     }
+ 
+     return newState
  });

In the ReportActionItemMessageEdit.js function updateDraft(draft) would look like this:

  const newDraft = EmojiUtils.replaceEmojis(draft);
+ this.setState((prevState) => {
+     let newState = {draft: newDraft}
+
+     if(draft !== newDraft) {
+         const remainder = prevState.draft.slice(prevState.selection.end).length
+         newState.selection =  {
+             start: newDraft.length - remainder,
+             end: newDraft.length - remainder
+         }
+     }
+
+     return newState
+ });

Here's a video of this code taking effect:

bugfix.mov

[laurenreidexpensify]

@rushatgabhane bump ^^

[AndreasBBS]

I'm quite confused maybe someone from the engineering team might enlighten me.
I accidentally checked out the branch master out of habit and noticed this particular bug was solved. This made me quite confused because in the staging environment the bug still persists. I then realized that there's also a branch main and that's the one I originally had tested for the bug.
I'm left wondering why, in the first place, is there both a master and a main branch? Why do they have different histories? Should I test for the bugs against the master branch? What is the purpose of the master branch?
Maybe this is not the best place to be placing this questions but I thought it was relevant to mention that this is not a bug in the master branch, whether this means anything I'm not sure as I do not understand the purpose of that branch.

[rushatgabhane]

this.textInput.selectionEnd = latestIconPosition

@hungvu193 question: does modifying the selection using it's ref work on all platforms? (especially iOS and iOS safari)

suggestion: we already are passing selection as a prop to TextInput. Maybe we should use that?

[rushatgabhane]

We're adding selection logic to EmojiUtils.replaceEmojis(). I'm not sure if that's the right place to do it.
What if we want a space in some places and not in others? Can we seperate the concerns better

[hungvu193]

@rushatgabhane yeah, it worked on all platforms, I tested them all.
Let me see if we can use selection as a state.

[rushatgabhane]

@hungvu193 If I'm not wrong, @AndreasBBS's proposal uses selection prop, and also addresses seperation of concerns.

[hungvu193]

yeah, that's right. If I use selection props, it will look like:

 this.setState(() => {
            const newState = {
                isCommentEmpty: !!newComment.match(/^(\s|`)*$/),
                value: newComment,
            };
            if (latestIconPosition !== -1) {
                newState.selection = {
                    start: latestIconPosition,
                    end: latestIconPosition,
                };
            }
            return newState;
        });

[rushatgabhane]

Really appreciate your proposal! @hungvu193

---

I'm gonna recommend @AndreasBBS' proposal to @ctkochan22 because it was using the existing selection prop, and had better separation of concerns.

🎀 👀 🎀 C+ reviewed

[AndreasBBS]

@hungvu193 Both our approaches work the same when writing an emoji, the big difference is how the paste is handled.
Let's say we're pasting the string :wave: friend in the string Hello nice to meet you between the word "Hello" and "nice": Hello |nice to meet you (representing the cursor with the char "|")

• Using your approach, after you paste; the cursor would be left at the end of the string: Hello 👋 friend nice to meet you|
• Using your approach before your update, after you paste; the cursor would be left after the emoji: Hello 👋| friend nice to meet you
• Using my approach the cursor would be left where it was before the paste, behind the word nice: Hello 👋 friend |nice to meet you

[melvin-bot]

@ctkochan22, @rushatgabhane, @laurenreidexpensify Whoops! This issue is 2 days overdue. Let's get this updated quick!

[laurenreidexpensify]

@ctkochan22 bump here 👍 for this solution Thanks

[AndreasBBS]

Hi. I'm writing here trying to get an update. I already submitted the proposal on Upwork. Is there something on my side holding up? It's my first time contributing to this project, if there's anything on my side that needs to be done just let me know 😃

[rushatgabhane]

@ctkochan22 gentle bump to review @AndreasBBS's proposal #12325 (comment)

[ctkochan22]

Man, hard to keep track of so many of these. Good work here everyone! @rushatgabhane I'm good with your recommendation. @AndreasBBS 's solution looks good.

@laurenreidexpensify lets hire and move forward

I then realized that there's also a branch main and that's the one I originally had tested for the bug.

Second @AndreasBBS , we no longer use the master branch. We moved over to using main. So its now, main -> staging -> production

[melvin-bot]

📣 @AndreasBBS You have been assigned to this job by @laurenreidexpensify!
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 📖

[laurenreidexpensify]

@AndreasBBS I've hired you in Upwork now, if you can let us know when you're aiming to have the PR up? Feel free to ask any questions in Slack as you go : )

[AndreasBBS]

@laurenreidexpensify I just pushed the fix to my branch. I'm gonna now record the videos of it working on the different platforms. I'm aiming to submit the PR still today.

[AndreasBBS]

I just discovered a strange behavior in the Android app that I'm trying to understand if it was causes by my changes. It's why it's taking me a while to make the PR. I want to make sure I don't create another problem trying to solve. Thanks for your patience.

[adeel0202]

@laurenreidexpensify this got closed without issuing payments.

[adeel0202]

@laurenreidexpensify friendly bump

[laurenreidexpensify]

Everyone has been paid now, apols!