[hold for app #12854] [$2000] Shows incorrect emoji while combining the manual and emoji picker selected emoji within the text

[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. Load new.expensify.com.
2. Open a chat.
3. type Te:smile :st in the text bar.
4. remove the space so the smile emoji renders inside of the word test. leave your cursor flush with the emoji.
5. click the emoji picker.
6. select an emoji.
7. confirm that the new emoji loads in between the two question mark characters. This is the bug.

Expected Result:

Should show both emoji adjacently after selecting the second one.

Actual Result:

Shows a weird emoji, and removes the original emoji, incorrectly.

Workaround:

Select emojis via the emoji picker.

Platform:

Where is this issue occurring?

• Web

Version Number: v1.2.32-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:
7c67d298-274a-4ea8-a781-e28384f08007.webm
https://user-images.githubusercontent.com/43996225/204145304-bf1f2078-3b3f-46c5-9ac0-26b4d8bcf01e.mp4

Expensify/Expensify Issue URL:
Issue reported by: @Pujan92
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1669391872652859

View all open jobs on GitHub

Recording.1012.mp4

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~016d120322c0f448f0
• Upwork Job ID: 1597336781605519360
• Last Price Increase: 2023-04-17

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~016d120322c0f448f0
• Upwork Job ID: 1597336781605519360
• Last Price Increase: 2023-04-17

[melvin-bot]

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

[Pujan92]

Proposal

As we are removing the character between the emoji string(: smile:) I think prevState selection logic won't work in the following code.

App/src/pages/home/report/ReportActionCompose.js

Lines 392 to 396 in 17c34e3

	const remainder = prevState.value.slice(prevState.selection.end).length;
	newState.selection = {
	start: newComment.length - remainder,
	end: newComment.length - remainder,
	};

const index = this.getLastUnmatchedCharacterIndex(comment, newComment) + 1;
newState.selection = {
    start: index,
    end: index,
};

getLastUnmatchedCharacterIndex(text, modifiedText) {
    for (let i = text.length - 1, j = modifiedText.length - 1; i >= 0; i--, j--) {
        if (text[i] !== modifiedText[j]) {
             return j;
         }
     }
}

The above solution will fix 12325 too which was earlier fixed with prevState selection logic.

[s77rt]

Proposal

diff --git a/src/pages/home/report/ReportActionCompose.js b/src/pages/home/report/ReportActionCompose.js
index 2335864f91..fc7b5fad51 100644
--- a/src/pages/home/report/ReportActionCompose.js
+++ b/src/pages/home/report/ReportActionCompose.js
@@ -7,6 +7,7 @@ import {
 } from 'react-native';
 import _ from 'underscore';
 import lodashGet from 'lodash/get';
+import lodashClamp from 'lodash/clamp';
 import {withOnyx} from 'react-native-onyx';
 import lodashIntersection from 'lodash/intersection';
 import styles from '../../../styles/styles';
@@ -389,10 +390,11 @@ class ReportActionCompose extends React.Component {
                 value: newComment,
             };
             if (comment !== newComment) {
-                const remainder = prevState.value.slice(prevState.selection.end).length;
+                const offset = newComment.length - comment.length;
+                const compensation = Math.max(comment.length - prevState.value.length, 0);
                 newState.selection = {
-                    start: newComment.length - remainder,
-                    end: newComment.length - remainder,
+                    start: lodashClamp(prevState.selection.start + offset + compensation, 0, newComment.length),
+                    end: lodashClamp(prevState.selection.end + offset + compensation, 0, newComment.length),
                 };
             }
             return newState;

Details:

The current implemented code does not take into account compensation and overflow.
In my proposal I added a compensation variable to fix the current issue and used lodashClamp to cover other cases (e.g. write : wave:hi then remove the space before wave, the calculated position is negative (invalid), this has been fixed as well)

[joekaufmanexpensify]

Triaging this now:

From this SO: https://stackoverflow.com/c/expensify/questions/14418

• The "bug" is actually a bug:
• The bug is not a duplicate report of an existing GH (close the GH and add any novel details to the original GH instead):
• The bug is reproducible, following the reproduction steps:
• If you're unable to reproduce the bug, add the Needs reproduction label. Comment on the issue outlining the steps you took to try to reproduce the bug, your results and tag the issue reporter and the Applause QA member who created the issue. Ask them to clarify reproduction steps and/or to check the reproduction steps again. Close issue.:
• The GH template is filled out as fully as possible -- this means the GH body and title are clear (ie. could an external contributor understand it and work on it?):
• The GH was created by an Expensify employee or a QA tester:
• If the bug is an OldDot issue, you should decide whether it is widespread enough to justify fixing or it is better to wait for reunification. If it's better to wait, close the GH & provide this justification:
• If there's a link to Slack, check the discussion to see if we decided not to fix it:
• Decide if the GH should be resolved by an External contributor or Internal engineer, add the appropriate label:

[joekaufmanexpensify]

I can reproduce this on web. Added some more detail to reproduction steps. This is a niche bug, however it can be reproduced, so I think we proceed with fixing it.

[joekaufmanexpensify]

My thinking is this should be external. Assigning to engineering to confirm!

[melvin-bot]

Triggered auto assignment to @johnmlee101 (Engineering), see https://stackoverflow.com/c/expensify/questions/4319 for more details.

[johnmlee101]

Yep, confirmed. At first I thought it was the skin-tone modifiers for emoji but it seems to be injecting it in a way that splits the unicode incorrectly.

[melvin-bot]

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