[$1000] The cursor resets to the beginning for the edit message in draft

[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 report
2. Send any comment
3. Long press the comment
4. Press Edit comment
5. Set cursor position anywhere except at the beginning
6. Go to any other report
7. Come back to the report that has edit draft

Expected Result:

The cursor position remains unchanged

Actual Result:

The cursor position resets to the beginning

Workaround:

Can the user still use Expensify without this being fixed? Have you informed them of the workaround?

Platforms:

Which of our officially supported platforms is this issue occurring on?

• Android / native
• Android / Chrome
• iOS / native
• iOS / Safari
• MacOS / Chrome / Safari
• MacOS / Desktop

Version Number: 1.3.29-0
Reproducible in staging?: y
Reproducible in production?: y
If this was caught during regression testing, add the test name, ID and link from TestRail:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos: Any additional supporting documentation

23-06-12-23-47-59.mp4

Record_2023-06-20-16-59-37_4f9154176b47c00da84e32064abf1c48.mp4

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

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~014a2eb8967fd1486f
• Upwork Job ID: 1671935895187853312
• Last Price Increase: 2023-06-29

[melvin-bot]

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

[melvin-bot]

Bug0 Triage Checklist (Main S/O)

• This "bug" occurs on a supported platform (ensure Platforms in OP are ✅)
• This bug is not a duplicate report (check E/App issues and #expensify-bugs)
  • If it is, comment with a link to the original report, close the issue and add any novel details to the original issue instead
• This bug is reproducible using the reproduction steps in the OP. S/O
  • If the reproduction steps are clear and you're unable to reproduce the bug, check with the reporter and QA first, then close the issue.
  • If the reproduction steps aren't clear and you determine the correct steps, please update the OP.
• This issue is filled out as thoroughly and clearly as possible
  • Pay special attention to the title, results, platforms where the bug occurs, and if the bug happens on staging/production.
• I have reviewed and subscribed to the linked Slack conversation to ensure Slack/Github stay in sync

[slafortune]

Looks good!

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~014a2eb8967fd1486f

[melvin-bot]

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

[melvin-bot]

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

[dukenv0307]

Proposal

Please re-state the problem that we are trying to solve in this issue.

The cursor position resets to the beginning

What is the root cause of that problem?

We currently do not store the cursor position of the Composer in Onyx, so when we go back to the screen, the composer will mount again and the cursor will be in its default position.

We need to support saving the "draft" cursor position, Slack also does it.

What changes do you think we should make in order to solve the problem?

We can:

1. Add a method to store the "draft" cursor position in Onyx, similar to the saving of the report comment here and number of lines here
2. In ReportActionItemMessageEdit, save the "draft" cursor position onSelectionChange (we can do this in debounce as well, similar to saving the draft)
3. When ReportActionItemMessageEdit is mounted, set the initial selection to the one that we get from Onyx

We need to do the same for ReportActionCompose

What alternative solutions did you explore? (Optional)

NA

[melvin-bot]

📣 @ginsuma! 📣
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. 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.
2. 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.
3. 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>

[kameshwarnayak]

Proposal

Please re-state the problem that we are trying to solve in this issue.

The cursor resets to the beginning for the edit message in draft

What is the root cause of that problem?

The selection values in the react native TextInput are not being set in few scenarios. There are multiple open bugs relating TextInput selection option (29063, 35005) in react native GH. However they don't point to the exact scenario that we are facing.

One of the workaround mentioned is to use setNativeProps to set the selection values.

What changes do you think we should make in order to solve the problem?

Using setNativeProps to set the selection in the use effects of the following file will solve the issue

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

Lines 98 to 101 in 820fa93

	useEffect(() => {
	// required for keeping last state of isFocused variable
	isFocusedRef.current = isFocused;
	}, [isFocused]);

becomes

    useEffect(() => {
        // required for keeping last state of isFocused variable
        isFocusedRef.current = isFocused;
        textInputRef.current.setNativeProps({
            selection: selection
        });
    }, [isFocused]);

Result :

screen-recording-2023-06-24-at-65550-pm_uDg1UmIh.mp4

What alternative solutions did you explore? (Optional)

None

[slafortune]

@thesahindia - thoughts?

[thesahindia]

@kameshwarnayak's proposal looks good to me but I wanna test it on a physical device first. I will do it today.

[kameshwarnayak]

@kameshwarnayak's proposal looks good to me but I wanna test it on a physical device first. I will do it today.

@thesahindia Tested this on the physical device. It is working on the physical device too. Please find the attached screenshot.

record-2023-06-27-19-02-57_kOEMFPSx.mp4

[melvin-bot]

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

[thesahindia]

@kameshwarnayak's proposal looks good.

🎀 👀 🎀 C+ reviewed

[melvin-bot]

Triggered auto assignment to @aldo-expensify, see https://stackoverflow.com/c/expensify/questions/7972 for more details.

[kameshwarnayak]

@kameshwarnayak's proposal looks good.

🎀 👀 🎀 C+ reviewed

@thesahindia Thanks for the confirmation. I have submitted the proposal on Upwork. Will create the PR once I get the approval on Upwork.

[melvin-bot]

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

[melvin-bot]

Bug0 Triage Checklist (Main S/O)

• This "bug" occurs on a supported platform (ensure Platforms in OP are ✅)
• This bug is not a duplicate report (check E/App issues and #expensify-bugs)
  • If it is, comment with a link to the original report, close the issue and add any novel details to the original issue instead
• This bug is reproducible using the reproduction steps in the OP. S/O
  • If the reproduction steps are clear and you're unable to reproduce the bug, check with the reporter and QA first, then close the issue.
  • If the reproduction steps aren't clear and you determine the correct steps, please update the OP.
• This issue is filled out as thoroughly and clearly as possible
  • Pay special attention to the title, results, platforms where the bug occurs, and if the bug happens on staging/production.
• I have reviewed and subscribed to the linked Slack conversation to ensure Slack/Github stay in sync

[bfitzexpensify]

@thesahindia bump on this question, so I can get payments sorted out:

I'm having a hard time understanding what is complete in what timeframe on this GH and trying to set up payment and reassign this. @thesahindia are you able to summarize this for us?

[kameshwarnayak]

There is a merge conflict in the PR since it has been long. I will fix the conflict and push the code tomorrow.

[thesahindia]

@bfitzexpensify, ahh this one got a little bit complicated. The original solution caused a regression and we reverted the PR and we have another PR that need to be reviewed. I missed it because the issue still has "HOLD for payment" in the title. Let's remove that.

[bfitzexpensify]

Sure, title updated.

So, my understanding is that we're waiting on the 2nd PR to be reviewed, merged and deployed, and then we can follow the typical close-out process (BZ checklist and payments issued) after that's done — is that correct?

[thesahindia]

Yes, that is correct!

[bfitzexpensify]

Awesome, and #24552 is the new PR, right?

[kameshwarnayak]

@thesahindia Was a bit held up with a personal commitment. I will have a look at the comment in a couple of days.

[thesahindia]

Awesome, and #24552 is the new PR, right?

Correct!

[kameshwarnayak]

Have we changed the behaviour of the focus on returning to a report. The focus is not on the draft when we open a report in the current main. Is it intended or a regression?

screen-recording-2023-09-24-at-31118-pm_PpBeWYNO.mp4

[aldo-expensify]

cc @Beamanator since you were talking the other day about documenting this kind of things (like focus behaviour) ^

The focus is not on the draft when we open a report in the current main. Is it intended or a regression?

No idea. For me it makes sense that it should be focused automatically, but I don't know if someone thought differently and removed it on purpose or if it is a regression.

[kameshwarnayak]

@aldo-expensify My personal choice would be to focus automatically.

Should we proceed with that change or find if the change is on purpose? If there is a document with these changes, it would be great to refer.

[thesahindia]

We should definitely check if it was changed intentionally.

[bfitzexpensify]

OK, I think it might be related to this broader issue - #15992 - that's dealing with general Composer Component Focus Issues. Does that sound right to everyone else?

[aldo-expensify]

I think it would make a lot of sense to define the focus behaviour we want and write it down so we don't end up changing it after because of someone personal preference.

[aldo-expensify]

I'm going OOO today, please reassign if you think you need an engineer. I'm not sure about where we are with this issue... it has the label "Awaiting Payment"

[bfitzexpensify]

I'm not entirely sure, either. @thesahindia, what are your thoughts on where this is at? #24552 was opened, but not merged, and my understanding is that the intent was to finalise that and then action payment on that PR.

#15992 is open, set to monthly, and unassigned, so I don't think anything will happen there anytime soon.

So, should we move forward with #24552, and then wrap this up, or should we leave this entirely?

[kameshwarnayak]

@bfitzexpensify #24552 this will not work because the behaviour has changed. With the current behaviour, we may not need a fix.

I am confused on the behaviour of the app when the user goes back to a report with a draft. Let me check the latest main and let you know the behaviour

[bfitzexpensify]

Any update on this one @kameshwarnayak?

[bfitzexpensify]

Friendly bump @kameshwarnayak

[kameshwarnayak]

@bfitzexpensify - will look into it today. Was but held up and but under the weather

[kameshwarnayak]

@bfitzexpensify Tested this on the latest master. The behaviour has changed and the keyboard doesn't appear by default. Please find the video below.

Screen.Recording.2023-10-22.at.12.31.48.AM.mov

If this is the new behaviour, then the bug becomes irrelevant.

[bfitzexpensify]

Agreed @kameshwarnayak. Closing this out.