Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[PAYMENT 3/4][$500] iOS - IOU - The cursor appears at random locations after pasting #35420

Closed
1 of 6 tasks
kbecciv opened this issue Jan 30, 2024 · 20 comments
Closed
1 of 6 tasks
Assignees
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Bug Something is broken. Auto assigns a BugZero manager. External Added to denote the issue can be worked on by a contributor Weekly KSv2

Comments

@kbecciv
Copy link

kbecciv commented Jan 30, 2024

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


Version Number: 1.4.33.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
Expensify/Expensify Issue URL:
Issue reported by: Applause- Internal Team
Slack conversation:

Issue found when executing PR #35119

Action Performed:

  1. Tap FAB - Request money
  2. Type "22222"
  3. Copy any single digit number "7"
  4. Drag, select & try to replace last number in step 3 by pasting "7"
  5. Drag, select & try to replace first number in step 3 by pasting "7"
  6. Drag, select & try to replace middle number in step 3 by pasting "7"
  7. Now drag, select & try to replace remaining digit by pasting "7"

Expected Result:

It should be appearing right after the paster number.

Actual Result:

The cursor appears at random locations after pasting.

Workaround:

Unknown

Platforms:

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

  • Android: Native
  • Android: mWeb Chrome
  • iOS: Native
  • iOS: mWeb Safari
  • MacOS: Chrome / Safari
  • MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

Bug6361115_1706634381199.JSTP7179.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~0180563495c94b9556
  • Upwork Job ID: 1752383457329037312
  • Last Price Increase: 2024-01-30
  • Automatic offers:
    • bernhardoj | Contributor | 0
@kbecciv kbecciv added External Added to denote the issue can be worked on by a contributor Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Jan 30, 2024
@melvin-bot melvin-bot bot changed the title iOS - IOU - The cursor appears at random locations after pasting [$500] iOS - IOU - The cursor appears at random locations after pasting Jan 30, 2024
Copy link

melvin-bot bot commented Jan 30, 2024

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

Copy link

melvin-bot bot commented Jan 30, 2024

Job added to Upwork: https://www.upwork.com/jobs/~0180563495c94b9556

@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Jan 30, 2024
Copy link

melvin-bot bot commented Jan 30, 2024

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

@kbecciv
Copy link
Author

kbecciv commented Jan 30, 2024

We think that this bug might be related to #vip-vsb
CC @quinthar

@bernhardoj
Copy link
Contributor

bernhardoj commented Jan 31, 2024

Proposal

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

Pasting a number on the money request input moves the selection to the left.

What is the root cause of that problem?

When we set a selection that is larger than the input value length, the selection will be put on the left. You can test this by having a button that will set the selection to a number that is larger than the current input value length.

In our case, when we paste something to the amount input that has a value, a whitespace is also added. This behavior is also found on this old issue. So, the total length of the input is added by 1 because of the whitespace, but we strip the space from the input.

const setNewAmount = useCallback(
(newAmount) => {
// Remove spaces from the newAmount value because Safari on iOS adds spaces when pasting a copied value
// More info: https://github.com/Expensify/App/issues/16974
const newAmountWithoutSpaces = MoneyRequestUtils.stripSpacesFromAmount(newAmount);

So, if the input is "2(whitespace)5", the new input state will be "25" and the value is updated before onSelectionChange is called, so when the selection change is called, it will try to update the selection to be 3, but the length is only 2.

Btw, to reproduce it easier, you can just type a number to the input then paste anything afterwards.

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

We can clamp the selection value so it doesn't set a value larger than the current input length.

onSelectionChange={(e) => {
if (!shouldUpdateSelection) {
return;
}
setSelection(e.nativeEvent.selection);
}}

const maxSelection = formattedAmount.length;
const start = Math.min(e.nativeEvent.selection.start, maxSelection);
const end = Math.min(e.nativeEvent.selection.end, maxSelection);
setSelection({start, end});

What alternative solutions did you explore? (Optional)

We can ignore the selection change if there is any update with the amount (because we already handle it manually). There is a shouldUpdateSelection but it's also being used for long press, so I think it's better to create a new state to temporarily ignore the selection change event and re-enable it after we ignore it.

const setNewAmount = useCallback(
    (newAmount) => {
        setShouldTemporaryIgnoreSelection(true);
...
onSelectionChange={() => {
    if (shouldTemporaryIgnoreSelection) {
        setShouldTemporaryIgnoreSelection(false);
        return;
    }

I guess ref would be more performant

@melvin-bot melvin-bot bot added the Overdue label Feb 2, 2024
@CortneyOfstad
Copy link
Contributor

@rushatgabhane any thoughts on the proposal above?

@melvin-bot melvin-bot bot removed the Overdue label Feb 2, 2024
@rushatgabhane
Copy link
Member

@bernhardoj's proposal fixes the root cause #35420 (comment)
🎀 👀 🎀

Copy link

melvin-bot bot commented Feb 5, 2024

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

@melvin-bot melvin-bot bot added the Overdue label Feb 7, 2024
@CortneyOfstad
Copy link
Contributor

@bernhardoj any update on the PR? Thank you!

@melvin-bot melvin-bot bot removed the Overdue label Feb 7, 2024
@bernhardoj
Copy link
Contributor

@CortneyOfstad I'm waiting to get assigned first

@melvin-bot melvin-bot bot added the Overdue label Feb 9, 2024
@melvin-bot melvin-bot bot removed the Help Wanted Apply this label when an issue is open to proposals by contributors label Feb 9, 2024
Copy link

melvin-bot bot commented Feb 9, 2024

📣 @bernhardoj 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job
Please accept the offer 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 📖

@melvin-bot melvin-bot bot added Weekly KSv2 and removed Daily KSv2 Overdue labels Feb 12, 2024
@bernhardoj
Copy link
Contributor

PR is ready

cc: @rushatgabhane

@CortneyOfstad
Copy link
Contributor

@rushatgabhane @bernhardoj any update on the PR/review? Thanks!

@CortneyOfstad
Copy link
Contributor

PR shows it is in staging as of 3 days ago. Will continue to keep an eye on this.

@CortneyOfstad
Copy link
Contributor

PR in production as of 2 days ago 👍

@CortneyOfstad CortneyOfstad added Awaiting Payment Auto-added when associated PR is deployed to production and removed Reviewing Has a PR in review labels Feb 29, 2024
@CortneyOfstad CortneyOfstad changed the title [$500] iOS - IOU - The cursor appears at random locations after pasting [PAYMENT 3/4][$500] iOS - IOU - The cursor appears at random locations after pasting Feb 29, 2024
@CortneyOfstad CortneyOfstad removed the Bug Something is broken. Auto assigns a BugZero manager. label Feb 29, 2024
@CortneyOfstad CortneyOfstad removed their assignment Feb 29, 2024
@CortneyOfstad CortneyOfstad added the Bug Something is broken. Auto assigns a BugZero manager. label Feb 29, 2024
Copy link

melvin-bot bot commented Feb 29, 2024

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

@melvin-bot melvin-bot bot added Daily KSv2 and removed Weekly KSv2 labels Feb 29, 2024
@CortneyOfstad CortneyOfstad added Weekly KSv2 and removed Daily KSv2 labels Feb 29, 2024
@CortneyOfstad
Copy link
Contributor

Hey @trjExpensify! I am heading OoO and will be back March 11th, so reassigned to have someone keep this moving until I am back. At this stage, the PR here went into production 3 days ago. If there are no regressions, the payment is set for Monday, March 4. Payment breakdown is below:

Payment Summary

@bernhardoj — $500 to be paid in Upwork
@rushatgabhane — $500 to be paid in NewDot

@CortneyOfstad CortneyOfstad self-assigned this Feb 29, 2024
@rushatgabhane
Copy link
Member

https://staging.new.expensify.com/r/4222149771653350 money request here

@JmillsExpensify
Copy link

$500 approved for @rushatgabhane based on summary above.

@trjExpensify
Copy link
Contributor

Paid $500 out @bernhardoj based on the payment summary here. @rushatgabhane confirmed to have requested, closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Bug Something is broken. Auto assigns a BugZero manager. External Added to denote the issue can be worked on by a contributor Weekly KSv2
Projects
None yet
Development

No branches or pull requests

7 participants