[HOLD setSelection refactor] [$2000] Android - Cursor is placed in wrong position after write/paste the emoji between the word Hello and nice - Reported by: @AndreasBBS

[kbecciv]

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

---

Issue found when executing PR #12632

Action Performed:

1. Launch the app
2. Log in with any account
3. Go to any chat
4. Start with a string Hello nice to meet you
5. Write the emoji 👋 in the string Hello nice to meet you between the word Hello and nice
6. Start with a string Hello nice to meet you
7. Paste the string 👋 friend in the string Hello nice to meet you between the word Hello and nice

Expected Result:

Describe what you think should've happened

Actual Result:

The resulting string has the emoji converted and the cursor is placed after the word friend/ or after the emoji

Workaround:

Unknown

Platform:

Where is this issue occurring?

• Android

Version Number: 1.2.29.2

Reproducible in staging?: Yes

Reproducible in production?: No

Email or phone of affected tester (no customers):

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

Notes/Photos/Videos: Any additional supporting documentation

Screen_Recording_20221117-143136_New.Expensify.mp4

Expensify/Expensify Issue URL:

Issue reported by: Applause - Internal Team

Slack conversation:

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~0134ac93ef23b19fbf
• Upwork Job ID: 1594665437635485696
• Last Price Increase: 2022-12-16

[melvin-bot]

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

[rushatgabhane]

This is a known bug and reported by @AndreasBBS over here

@dylanexpensify could you please add them as the reporter so that they get reporting bonus, thank you!

[roryabraham]

Can we please add some automated UI tests to cover this behavior with the next PR to fix this?

[rushatgabhane]

@roryabraham if i remb correctly, this issue is due to poor performance and not correctness

Can we please add some automated UI tests

sure!

[AndreasBBS]

@roryabraham You can check my analysis of this problem here. In my opinion is something that needs to be solved at the level of the react-native library, the onSelectionChange has erratic behavior on Android.

[melvin-bot]

@dylanexpensify Whoops! This issue is 2 days overdue. Let's get this updated quick!

[dylanexpensify]

reviewing today!

[dylanexpensify]

@AndreasBBS added you to title for reporting.

Upwork job: https://www.upwork.com/jobs/~013f07f27493ef4401

[melvin-bot]

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

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~0134ac93ef23b19fbf

[melvin-bot]

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

[melvin-bot]

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

[Puneet-here]

Hi, I had reported this bug here.

[s77rt]

Proposal

diff --git a/src/components/Composer/index.android.js b/src/components/Composer/index.android.js
index 603bdf829..83dc6be58 100644
--- a/src/components/Composer/index.android.js
+++ b/src/components/Composer/index.android.js
@@ -24,12 +24,6 @@ const propTypes = {
     /** Prevent edits and interactions like focus for this input. */
     isDisabled: PropTypes.bool,
 
-    /** Selection Object */
-    selection: PropTypes.shape({
-        start: PropTypes.number,
-        end: PropTypes.number,
-    }),
-
     /** Whether the full composer can be opened */
     isFullComposerAvailable: PropTypes.bool,
 
@@ -51,10 +45,6 @@ const defaultProps = {
     autoFocus: false,
     isDisabled: false,
     forwardedRef: null,
-    selection: {
-        start: 0,
-        end: 0,
-    },
     isFullComposerAvailable: false,
     setIsFullComposerAvailable: () => {},
     isComposerFullSize: false,
diff --git a/src/components/Composer/index.ios.js b/src/components/Composer/index.ios.js
index b31a5b462..ee3abf32b 100644
--- a/src/components/Composer/index.ios.js
+++ b/src/components/Composer/index.ios.js
@@ -24,12 +24,6 @@ const propTypes = {
     /** Prevent edits and interactions like focus for this input. */
     isDisabled: PropTypes.bool,
 
-    /** Selection Object */
-    selection: PropTypes.shape({
-        start: PropTypes.number,
-        end: PropTypes.number,
-    }),
-
     /** Whether the full composer can be opened */
     isFullComposerAvailable: PropTypes.bool,
 
@@ -51,10 +45,6 @@ const defaultProps = {
     autoFocus: false,
     isDisabled: false,
     forwardedRef: null,
-    selection: {
-        start: 0,
-        end: 0,
-    },
     isFullComposerAvailable: false,
     setIsFullComposerAvailable: () => {},
     isComposerFullSize: false,
@@ -94,9 +84,6 @@ class Composer extends React.Component {
     render() {
         // On native layers we like to have the Text Input not focused so the
         // user can read new chats without the keyboard in the way of the view.
-        // On Android, the selection prop is required on the TextInput but this prop has issues on IOS
-        // https://github.com/facebook/react-native/issues/29063
-        const propsToPass = _.omit(this.props, 'selection');
         return (
             <RNTextInput
                 autoComplete="off"
@@ -108,7 +95,7 @@ class Composer extends React.Component {
                 textAlignVertical="center"
                 style={this.state.propStyles}
                 /* eslint-disable-next-line react/jsx-props-no-spreading */
-                {...propsToPass}
+                {...this.props}
                 editable={!this.props.isDisabled}
             />
         );
diff --git a/src/pages/home/report/ReportActionCompose.js b/src/pages/home/report/ReportActionCompose.js
index 2335864f9..fbc7ca93a 100644
--- a/src/pages/home/report/ReportActionCompose.js
+++ b/src/pages/home/report/ReportActionCompose.js
@@ -47,6 +47,7 @@ import ExceededCommentLength from '../../../components/ExceededCommentLength';
 import withNavigationFocus from '../../../components/withNavigationFocus';
 import * as EmojiUtils from '../../../libs/EmojiUtils';
 import reportPropTypes from '../../reportPropTypes';
+import setSelection from '../../../libs/setSelection';
 
 const propTypes = {
     /** Beta features list */
@@ -323,13 +324,11 @@ class ReportActionCompose extends React.Component {
         const emojiWithSpace = `${emoji} `;
         const newComment = this.comment.slice(0, this.state.selection.start)
             + emojiWithSpace + this.comment.slice(this.state.selection.end, this.comment.length);
-        this.setState(prevState => ({
-            selection: {
-                start: prevState.selection.start + emojiWithSpace.length,
-                end: prevState.selection.start + emojiWithSpace.length,
-            },
-        }));
-        this.updateComment(newComment);
+        const newSelection = {
+            start: this.state.selection.start + emojiWithSpace.length,
+            end: this.state.selection.start + emojiWithSpace.length,
+        };
+        this.updateComment(newComment, false, newSelection);
     }
 
     /**
@@ -381,21 +380,19 @@ class ReportActionCompose extends React.Component {
      * @param {String} comment
      * @param {Boolean} shouldDebounceSaveComment
      */
-    updateComment(comment, shouldDebounceSaveComment) {
+    updateComment(comment, shouldDebounceSaveComment, selection) {
+        const oldComment = this.state.value;
         const newComment = EmojiUtils.replaceEmojis(comment);
-        this.setState((prevState) => {
-            const 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,
-                };
+        this.setState({
+            isCommentEmpty: !!newComment.match(/^(\s|`)*$/),
+            value: newComment,
+        }, () => {
+            if (selection) {
+                setSelection(this.textInput, selection.start, selection.end);
+            } else if (newComment !== comment) {
+                const remainder = oldComment.slice(this.state.selection.end).length;
+                setSelection(this.textInput, newComment.length - remainder, newComment.length - remainder);
             }
-            return newState;
         });
 
         // Indicate that draft has been created.
@@ -669,7 +666,6 @@ class ReportActionCompose extends React.Component {
                                         shouldClear={this.state.textInputShouldClear}
                                         onClear={() => this.setTextInputShouldClear(false)}
                                         isDisabled={isComposeDisabled || isBlockedFromConcierge}
-                                        selection={this.state.selection}
                                         onSelectionChange={this.onSelectionChange}
                                         isFullComposerAvailable={this.state.isFullComposerAvailable}
                                         setIsFullComposerAvailable={this.setIsFullComposerAvailable}
diff --git a/src/pages/home/report/ReportActionItemMessageEdit.js b/src/pages/home/report/ReportActionItemMessageEdit.js
index 2f7fbc29d..92a53bee8 100644
--- a/src/pages/home/report/ReportActionItemMessageEdit.js
+++ b/src/pages/home/report/ReportActionItemMessageEdit.js
@@ -22,6 +22,7 @@ import * as EmojiUtils from '../../../libs/EmojiUtils';
 import reportPropTypes from '../../reportPropTypes';
 import ExceededCommentLength from '../../../components/ExceededCommentLength';
 import CONST from '../../../CONST';
+import setSelection from '../../../libs/setSelection';
 
 const propTypes = {
     /** All the data of the action */
@@ -100,18 +101,18 @@ class ReportActionItemMessageEdit extends React.Component {
      *
      * @param {String} draft
      */
-    updateDraft(draft) {
+    updateDraft(draft, selection) {
+        const oldDraft = this.state.draft;
         const newDraft = EmojiUtils.replaceEmojis(draft);
-        this.setState((prevState) => {
-            const 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,
-                };
+        this.setState({
+            draft: newDraft,
+        }, () => {
+            if (selection) {
+                setSelection(this.textInput, selection.start, selection.end);
+            } else if (newDraft !== draft) {
+                const remainder = oldDraft.slice(this.state.selection.end).length;
+                setSelection(this.textInput, newDraft.length - remainder, newDraft.length - remainder);
             }
-            return newState;
         });
 
         // This component is rendered only when draft is set to a non-empty string. In order to prevent component
@@ -180,13 +181,11 @@ class ReportActionItemMessageEdit extends React.Component {
         const emojiWithSpace = `${emoji} `;
         const newComment = this.state.draft.slice(0, this.state.selection.start)
             + emojiWithSpace + this.state.draft.slice(this.state.selection.end, this.state.draft.length);
-        this.setState(prevState => ({
-            selection: {
-                start: prevState.selection.start + emojiWithSpace.length,
-                end: prevState.selection.start + emojiWithSpace.length,
-            },
-        }));
-        this.updateDraft(newComment);
+        const newSelection = {
+            start: this.state.selection.start + emojiWithSpace.length,
+            end: this.state.selection.start + emojiWithSpace.length,
+        };
+        this.updateDraft(newComment, newSelection);
     }
 
     /**
@@ -243,7 +242,6 @@ class ReportActionItemMessageEdit extends React.Component {
                             this.setState({isFocused: false});
                             toggleReportActionComposeView(true, VirtualKeyboard.shouldAssumeIsOpen());
                         }}
-                        selection={this.state.selection}
                         onSelectionChange={this.onSelectionChange}
                     />
                     <View style={styles.editChatItemEmojiWrapper}>

Details

1. Removed selection prop since it's not needed and it's also the main cause. We don't need to constantly control the selection just on updateComment, and in my proposal the update is done via setSelection lib.

[melvin-bot]

Looks like something related to react-navigation may have been mentioned in this issue discussion.

As a reminder, please make sure that all proposals are not workarounds and that any and all attempt to fix the issue holistically have been made before proceeding with a solution. Proposals to change our DeprecatedCustomActions.js files should not be accepted.

Feel free to drop a note in #expensify-open-source with any questions.

[mollfpr]

@s77rt Thanks for the proposal!

Could you attach the result of your proposal?

[s77rt]

@mollfpr

Kooha-2022-11-21-17-27-16.mp4

[dylanexpensify]

Mind giving us an idea on what seems fair for this, friends? 🙇‍♂️ @s77rt @mollfpr

[mollfpr]

Sorry got miss the notification.

I just tested the issue again on the latest main, and couldn't reproduce it. @s77rt Could you confirm it?

Mind giving us an idea on what seems fair for this, friends? 🙇‍♂️ @s77rt @mollfpr

@dylanexpensify 50% of the issue amount is fair to me.

[s77rt]

@mollfpr The bug seems to be fixed by facebook/react-native#37424 #12854 (comment)

@dylanexpensify Since no PR ended up being merged I think 25% is fair for my case.

[hannojg]

The app is now updated to react native 0.72+ which includes the native fix for that android issue, so it should be fixed 🥳

[dylanexpensify]

got it! @s77rt mind bringing this up please to C+ channel and cc'ing me?

[hannojg]

@s77rt did you had any chance to double check? I think this is fixed and can get closed 😊

[s77rt]

@hannojg Yes it's fixed! 🎉

[hannojg]

@dylanexpensify I think we can then close this issue, as it was fixed as part of our work to fix the selection prop in react native:

• Fix copy / paste menu and simplify controlled text selection on Android facebook/react-native#37424

[MariaHCD]

Oh, missed the latest updates on this but it sounds like we are good to close!

[dylanexpensify]

@MariaHCD mind confirming thoughts on above compensation?

[MariaHCD]

Ah, apologies, I missed that we need to pay out the compensations as I suggested before.

After refreshing my memory on the work on this issue:

• @mollfpr handled a lot of investigation, reviewing proposals and testing.
• @s77rt also did spend time in investigation and worked on a RN PR: Switch order of onSelectionChange and onChange events on iOS facebook/react-native#35603

So the compensation that both suggested sounds good to me too!

[dylanexpensify]

Payment Summary:

• Reporter: @AndreasBBS $250
• Contributor: @mollfpr $1,000
• Contributor+: @s77rt $500

Upwork job here! Please apply!

[s77rt]

@dylanexpensify Applied! Thanks!

[mollfpr]

Applied too, thanks @dylanexpensify

[dylanexpensify]

@s77rt @mollfpr sent offers!

[mollfpr]

Accepted, thank you!

[s77rt]

Same ^

[Puneet-here]

Hey @dylanexpensify, I am the original reporter of this issue. It was confirmed here earlier.

[AndreasBBS]

Hey @dylanexpensify, I am the original reporter of this issue. It was confirmed here earlier.

I can confirm this. I haven't claimed the bounty. I think the confusion comes from the title of the issue. I did report it unaware it had already been reported. Go get what's rightfully yours my friend 😁👍

[melvin-bot]

📣 @AndreasBBS! 📣
Hey, it seems we don’t have your contributor details yet! You'll only have to do this once, and this is how we'll hire you on Upwork.
Please follow these steps:

1. Make sure you've read and understood the contributing guidelines.
2. Get the email address used to login to your Expensify account. If you don't already have an Expensify account, create one here. If you have multiple accounts (e.g. one for testing), please use your main account email.
3. Get the link to your Upwork profile. It's necessary because we only pay via Upwork. You can access it by logging in, and then clicking on your name. It'll look like this. If you don't already have an account, sign up for one here.
4. Copy the format below and paste it in a comment on this issue. Replace the placeholder text with your actual details.
   
   Format:

Contributor details
Your Expensify account email: <REPLACE EMAIL HERE>
Upwork Profile Link: <REPLACE LINK HERE>

[dylanexpensify]

@Puneet-here offer sent!
@mollfpr @s77rt payments sent!

[Puneet-here]

@Puneet-here offer sent!

Accepted!

[dylanexpensify]

We're all done! Thanks y'all!