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

[HOLD react-native 25643] [$1000] IOU - Correct digit is not getting deleted in IOU request reported by @daraksha-dk #12298

Closed
kavimuru opened this issue Oct 31, 2022 · 29 comments
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. Engineering External Added to denote the issue can be worked on by a contributor Help Wanted Apply this label when an issue is open to proposals by contributors Weekly KSv2

Comments

@kavimuru
Copy link

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. Request Money > type 1234
  2. Click between 3 & 4 and hold the cursor and move to any other position (b/w 1&2 or 2&3 etc)
  3. Now hit delete

Expected Result:

Deletion should happen from where the cursor is

Actual Result:

only the initial position is getting lost (it will delete 3)

Workaround:

unknown

Platform:

Where is this issue occurring?

  • Mobile Web - Android FF

Version Number: 1.2.21-4

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:

az_recorder_20221031_100651.1.mp4
cursor_issue.mp4

Expensify/Expensify Issue URL:

Issue reported by: @daraksha-dk

Slack conversation: https://expensify.slack.com/archives/C01GTK53T8Q/p1667200193320029

View all open jobs on GitHub

@kavimuru kavimuru added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Oct 31, 2022
@melvin-bot
Copy link

melvin-bot bot commented Oct 31, 2022

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

@kavimuru kavimuru changed the title When we click on any random position on the amount input and then hold & drag the cursor to another position then it will only remember the initial position and delete from there only reported by @daraksha-dk IOU - Correct digit is not getting deleted in IOU requrest reported by @daraksha-dk Oct 31, 2022
@greg-schroeder greg-schroeder removed their assignment Nov 1, 2022
@melvin-bot
Copy link

melvin-bot bot commented Nov 1, 2022

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

@tgolen tgolen added the External Added to denote the issue can be worked on by a contributor label Nov 1, 2022
@melvin-bot
Copy link

melvin-bot bot commented Nov 1, 2022

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

@melvin-bot
Copy link

melvin-bot bot commented Nov 1, 2022

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

@tgolen
Copy link
Contributor

tgolen commented Nov 1, 2022

Sounds like a legit bug. Pushing this to external to get proposals for it.

@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Nov 1, 2022
@melvin-bot
Copy link

melvin-bot bot commented Nov 1, 2022

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

@melvin-bot melvin-bot bot changed the title IOU - Correct digit is not getting deleted in IOU requrest reported by @daraksha-dk [$250] IOU - Correct digit is not getting deleted in IOU requrest reported by @daraksha-dk Nov 1, 2022
@lschurr lschurr changed the title [$250] IOU - Correct digit is not getting deleted in IOU requrest reported by @daraksha-dk [$250] IOU - Correct digit is not getting deleted in IOU request reported by @daraksha-dk Nov 1, 2022
@lschurr
Copy link
Contributor

lschurr commented Nov 1, 2022

@lschurr lschurr added Weekly KSv2 and removed Daily KSv2 labels Nov 1, 2022
@AndreasBBS
Copy link
Contributor

@kavimuru I'm trying to tackle this but I'm not able to reproduce this. I tried reproducing with the Brave and Chrome browsers. What browser are you using?

@kavimuru
Copy link
Author

kavimuru commented Nov 2, 2022

@AndreasBBS It happens only in Android - Firefox browser

@AndreasBBS
Copy link
Contributor

AndreasBBS commented Nov 4, 2022

I've investigated the issue a bit and here are my findings so far:

  • On Chrome the nativeEvent that react-native sends to onSelectionChange is of type: "selectionchange"
  • On Firefox the nativeEvent that react-native sends to onSelectionChange is of type: "mouseup"
  • Both in Chrome and Firefox the event is triggered when the user presses somewhere in the text, so the component is able to update the selection values correctly on both browsers in this case
  • On Chrome when the cursor is "dragged" the event is fired every time the cursor moves a character position, so the component is able to update the selection values correctly
  • On Firefox when the cursor is "dragged" the event is not fired, so the component does not know it has to update the selection values. This causes the component to "think" the cursor is still in the position where the user originally pressed. It not only causes the wrong digit to be deleted (as it is mentioned here in this post) but also if you introduce a new digit it will not place the digit in the cursor position but instead it will place it in the original cursor position (before dragging). I've verified that this variant of the bug also happens.

Proposal

My proposal is that this bug needs to be solved at the react-native library level. React-native should trigger the events in the same way regardless if it's Firefox or Chrome and I defend that this should be the approach to fix instead of changing the code of this App. The App code does the correct thing and when the events are triggered correctly, as is the case in Chrome, the correct behavior is observed.

@lschurr
Copy link
Contributor

lschurr commented Nov 4, 2022

@Santhosh-Sellavel - could you review this proposal?

@Santhosh-Sellavel
Copy link
Collaborator

@AndreasBBS
Thanks for investigating!

Have you checked on the react-native issues is there any related bugs?
Do you have a solution for the bug, if so please share it so I can review it.

@mallenexpensify mallenexpensify added Daily KSv2 and removed Weekly KSv2 labels Nov 5, 2022
@mallenexpensify
Copy link
Contributor

Bumped back to daily per our new BugZero process, internal Slack thread with deets

@AndreasBBS
Copy link
Contributor

AndreasBBS commented Nov 6, 2022

@Santhosh-Sellavel I've checked both the react-native and react-native-web issues and didn't find anything relating to this bug.

Because this is a browser bug I opened an issue in the react-native-web repo, you can find it here necolas/react-native-web#2422

You can also find a codesandbox there that illustrates the bug.

I looked into the react-native-web source and I couldn't really come up with a solution for this bug. I'm hoping that dialogue in the issue I created there might lead to some better solutions. I also think that this possible needs to be solved at the Firefox engine level because the native event in react-native-web is just the events that come from the browser and Firefox does not seem to fire the events correctly in this dragging cursor case. I'll wait for some replies in the react-native-web issue I created but I suspect this might prove to be a symptom of Firefox engine...

I'd love if you could try the codebox example I left on the react-native-web issue and give me your thoughts. This seems to me to be an issue outside of the scope of this app and fixes at lower lever libraries probably will require a higher insensitive than $250.

@Santhosh-Sellavel
Copy link
Collaborator

Thanks again, I've checked out codesandbox. The issue is occurring at my end as well on the Firefox browser.

Screen_Recording_20221107-011158_Firefox.mp4

Let's wait for some response from react-native-web libraries as well, but the issue seems to be at from firefox engine level.

Regarding incentives, if we have a clear path toward a solution, we could definitely discuss it.

Read this as we stated,

Please be aware that compensation for any support in solving an issue is provided entirely at Expensify’s discretion. Personal time or resources applied towards investigating a proposal will not guarantee compensation. Compensation is only guaranteed to those who propose a solution and get hired for that job.

@AndreasBBS
Copy link
Contributor

AndreasBBS commented Nov 6, 2022

I've found this as well and I'm gonna drop it here cause it might be useful to somebody trying to tackle this issue: https://developer.mozilla.org/en-US/docs/Web/API/HTMLInputElement/selectionchange_event

The example in the link works on Firefox for Android but it does not work in Chrome for Android. (In fact doesn't work in Chrome for desktop either, only Firefox)

It seems to me that Chrome and Firefox deal with this selection feature differently and react-native-web is defaulting to Chrome's solution...

@AndreasBBS
Copy link
Contributor

Update: I've investigated further and I discovered that this bug is something that extends to the react package. react-native-web uses react's input onSelect for the TextInput onSelectionChange handler. I closed the issue on react-native-web and opened a new one on react's repository: facebook/react#25643

@lschurr
Copy link
Contributor

lschurr commented Nov 7, 2022

Just clarifying @AndreasBBS - this GH needs to be on hold until the other you mentioned is fixed right?

@AndreasBBS
Copy link
Contributor

Just clarifying @AndreasBBS - this GH needs to be on hold until the other you mentioned is fixed right?

Yes, right. I'm not getting a lot of feedback on that issue and it seems to me that issues in that repo take a while to be addressed.

I've been trying to fix it myself but react source is dense and I rarely have to work at this level, I'm enjoying the challenge though. If you know anyone that's familiar with working on the react source code that can give me some guidance I'd highly appreciate if you could connect me with.

@Santhosh-Sellavel
Copy link
Collaborator

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

Join our slack channel, and post help anyone can offer help!

@lschurr lschurr changed the title [$250] IOU - Correct digit is not getting deleted in IOU request reported by @daraksha-dk [$500] IOU - Correct digit is not getting deleted in IOU request reported by @daraksha-dk Nov 9, 2022
@lschurr
Copy link
Contributor

lschurr commented Nov 9, 2022

Price doubled - https://www.upwork.com/jobs/~01cdae73aeb040edd9

@lschurr lschurr added Weekly KSv2 and removed Daily KSv2 labels Nov 11, 2022
@mvtglobally
Copy link

Issue not reproducible during KI retests. (First week)

@lschurr
Copy link
Contributor

lschurr commented Nov 16, 2022

Hey @Santhosh-Sellavel - Does this have relevant proposals? Is the PR that @AndreasBBS put up related?

@Santhosh-Sellavel
Copy link
Collaborator

Is #12632 that @AndreasBBS put up related?

No Its not related!

@Santhosh-Sellavel
Copy link
Collaborator

We have some investigation, but not a complete proposal!

@lschurr
Copy link
Contributor

lschurr commented Nov 16, 2022

Great, going to double the price.

@lschurr lschurr changed the title [$500] IOU - Correct digit is not getting deleted in IOU request reported by @daraksha-dk [$1000] IOU - Correct digit is not getting deleted in IOU request reported by @daraksha-dk Nov 16, 2022
@puneetlath
Copy link
Contributor

@tgolen this is one of the oldest issues in the /App repo. To help us clear out the large backlog of bugs, can you:

  • Decide whether any proposals currently meet our guidelines and can be approved as-is
  • For any that can't, please take this issue internal and treat it as one of your highest priorities
  • If you have any questions, don't hesitate to start a discussion in #bug-zero

@tgolen tgolen changed the title [$1000] IOU - Correct digit is not getting deleted in IOU request reported by @daraksha-dk [HOLD react-native 25643] [$1000] IOU - Correct digit is not getting deleted in IOU request reported by @daraksha-dk Nov 18, 2022
@tgolen
Copy link
Contributor

tgolen commented Nov 18, 2022

Considering that this doesn't impact any of our supported platforms, I am going to close this out. There is an upstream fix in react-native that @AndreasBBS opened, and I think it's still good to pursue that, and I think closing this in the meantime is the right move.

@tgolen tgolen closed this as completed Nov 18, 2022
@AndreasBBS
Copy link
Contributor

Considering that this doesn't impact any of our supported platforms, I am going to close this out. There is an upstream fix in react-native that @AndreasBBS opened, and I think it's still good to pursue that, and I think closing this in the meantime is the right move.

I actually moved the upstream fix to react itself once I realized the bug is originating from that package and not react-native. You can follow the issue here: facebook/react#25643

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something is broken. Auto assigns a BugZero manager. Engineering External Added to denote the issue can be worked on by a contributor Help Wanted Apply this label when an issue is open to proposals by contributors Weekly KSv2
Projects
None yet
Development

No branches or pull requests

9 participants