[$250] Login - Clicking the eye icon moves the cursor to the beginning

[kbecciv]

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 URL https://staging.new.expensify.com/
2. Enter an email address
3. Press continue
4. Enter the password
5. Click to the eye icon

Expected Result:

The cursor should not moved

Actual Result:

The cursor moved to the beginning

Workaround:

Unknown

Platform:

Where is this issue occurring?

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

Version Number: 1.2.18.2

Reproducible in staging?: Yes

Reproducible in production?: Yes

Email or phone of affected tester (no customers):

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

Notes/Photos/Videos: Any additional supporting documentation

https://user-images.githubusercontent.com/93399543/196955840-36e347cf-f392-43c0-8b63-822710a9d4d6.mp4

@Julesssss removed the capture as it was an unrelated recording

Expensify/Expensify Issue URL:

Issue reported by: Applause - Internal Team

Slack conversation:

View all open jobs on GitHub

[melvin-bot[bot]]

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

[Julesssss]

Yep, I can reproduce. @kbecciv would you mind re-uploading a video? I think you uploaded a different bug, so I removed it.

[melvin-bot[bot]]

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

[melvin-bot[bot]]

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

[melvin-bot[bot]]

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

[aimane-chnaif]

I don't think it's worthy enough to fix. slack discussion: https://expensify.slack.com/archives/C01GTK53T8Q/p1663012828257749 related issue and comments: https://github.com/Expensify/App/issues/10555

[Julesssss]

Oh, thanks for linking! While I agree with most of that discussion we've recently decided to try fixing each and every bug we encounter. For that reason, I think we should reopen this and see what sort of proposals we receive.

If the solution is very convoluted, then maybe it would be preferable to just default to the end, given that mobile and some web browsers do this by default.

[kbecciv]

@Julesssss Video is attached, sorry for missing it.

[Julesssss]

Thanks, no worries

[melvin-bot[bot]]

@eVoloshchak, @Julesssss, @flaviadefaria Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[melvin-bot[bot]]

@eVoloshchak, @Julesssss, @flaviadefaria Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[Julesssss]

Awaiting proposals

[flaviadefaria]

link to the internal job posting: posting https://www.upwork.com/ab/applicants/1584804132817670144/job-details link external job: https://www.upwork.com/jobs/~017777eb694223c27d

[esh-g]

PROPOSAL

---

I looked at the code to update the secured state of the password and this is the method that does that:

Problem Changing the state seems to reset the cursor and brings it to the start.

Solution When the hide button is clicked, and the above method is called, it should also call another method that sets the cursor position to the end of the input:

This method just gets the length of the current value and send the cursor to that position. This is the cleanest and lowest complexity implementation for this situation. The only problem was that calling this function along side the setState did not work. So I had to queue it in the microtasks so that it gets executed after calling setState.

I successfully solved the issue and now this works with complete reliability on web.

https://user-images.githubusercontent.com/77237602/198126708-1f1c5cab-a961-47f8-bb2d-496f70ee9d06.mp4

Now only testing on other platforms is left, which I plan to do upon hire. I look forward to contribute to this wonderful project!

[Julesssss]

Thanks for the proposal. But I'm curious whether there's a better solution that doesn't require us to manually set the cursor.

[esh-g]

Apparently the cursor resetting is just react-native internal behaviour, I tested on snack and got the cursor going to the beginning as well. I tried to fiddle around with other textInput components which was unsuccessful but I think that would be unnecessary and just add extra code. This was the solution that was the simplest and most importantly added the least amount of code. And is much more reliable than relying on a third-party component.

[melvin-bot[bot]]

@eVoloshchak, @Julesssss, @flaviadefaria Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[melvin-bot[bot]]

@eVoloshchak, @Julesssss, @flaviadefaria Whoops! This issue is 2 days overdue. Let's get this updated quick!

[Julesssss]

I agree we don't want to depend on 3rd party component for this. I'm coming around to the manual setting of cursor, but want to leave the issue open a little longer.

[Julesssss]

Holding for any last proposals

[melvin-bot[bot]]

@eVoloshchak, @Julesssss, @flaviadefaria Whoops! This issue is 2 days overdue. Let's get this updated quick!

[flaviadefaria]

Posted in slack re next steps.

[esh-g]

Hey @Julesssss now could we move ahead with that solution?

[Julesssss]

Yep, seems like this is the best solution we have.

[melvin-bot[bot]]

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

[aimane-chnaif]

I don't think this proposal will work. This approach always moves the cursor to the end no matter current cursor position and text selection. We should keep previous cursor position and text selection after tapping eye icon. cc: @parasharrajat

I already posted solution for that in the past, but that doesn't work on iOS native. https://github.com/Expensify/App/issues/10555#issuecomment-1244922998

[esh-g]

@Julesssss Thank you for assigning me the job! I will do the proposed solution within this week and will also do some extensive research on all possible solutions to this.

[Julesssss]

Oh, thanks for mentioning @aimane-chnaif. Would you mind explaining further?

As far as I can tell the proposal would only set the cursor on toggle, but maybe I'm missing something?

[aimane-chnaif]

Here's android video

Current version:

https://user-images.githubusercontent.com/96077027/200573981-c46a4b09-35fa-4aa7-8b08-8a51331d7957.mp4

After applying this proposal:

https://user-images.githubusercontent.com/96077027/200574036-79625048-63b0-47b6-a33a-78a847496209.mp4

Current android app is perfect. Always keeps cursor position or selection range. I think this GH is just to make all platforms work like android.

[Julesssss]

Actually @Someone-here, sorry to halt hire this but there are a couple of issues:

and will also do some extensive research on all possible solutions to this.

1. We need to approve the proposal before sending you off to investigate. Ideally the investigation occurs beforehand, so that we can properly compare solutions.

2. I missed this previously, but the video shows a delay in the cursor being set (at 11-12 seconds). I'd say it's better to have the current bug than to show a laggy movement of the cursor.

3. As @aimane-chnaif mentions, the solution relies on setTimeout AND we lose the current position of the cursor.

I'm going to unassign for now, but we're open to any alternative solutions.