-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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
[$250] [Android] mWeb/Chrome - BNP - Inserting decimal point, inconsistent behavior in mweb&Android #47074
Comments
Triggered auto assignment to @mallenexpensify ( |
We think that this bug might be related to #vip-vsb |
@mallenexpensify FYI I haven't added the External label as I wasn't 100% sure about this issue. Please take a look and add the label if you agree it's a bug and can be handled by external contributors |
This has been labelled "Needs Reproduction". Follow the steps here: https://stackoverflowteams.com/c/expensify/questions/16989 |
Gonna try to pawn this off on Sheena when she gets assigned another iOS bug :) |
Oh snap.... we got an Android phone on the way to the bank! I'll be able to test soon. |
@mallenexpensify this issue was created 2 weeks ago. Are we close to a solution? Let's make sure we're treating this as a top priority. Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks! |
Need to get Android phone setup, will do next week, bumped to Daily |
Unsure when in the office this week but planning to get phone setup |
@mallenexpensify Uh oh! This issue is overdue by 2 days. Don't forget to update your issues! |
Still need to get phone setup 😿 |
Waiting on retest |
Issue is reproducible during KI retests. 1728711736643.Screenrecorder-2024-10-12-11-11-24-818_compress_1.mp4 |
Job added to Upwork: https://www.upwork.com/jobs/~021846593217215177350 |
Triggered auto assignment to Contributor-plus team member for initial proposal review - @thesahindia ( |
Wow.. I really kicked the can down the road for a while here :/ |
Hi, I am Michael (Mykhailo) from Callstack, an expert agency and I can work on this issue. |
The bug is replicable on all mobile web. I am investigating. Details
Bug.mp4 |
No overdue. I am working on the issue. |
I understand why the bug exists. The app does not recognize a cursor change for this edge case, and the callback of |
@mallenexpensify, @rezkiy37, @thesahindia Whoops! This issue is 2 days overdue. Let's get this updated quick! |
Since the bug is web-only we can utilize |
ProposalPlease re-state the problem that we are trying to solve in this issue.When the user clicks on the "<" (remove) button in mWeb, any further input is inserted in a previous selection (cursor) position. For instance:
Video
Bug1.mp4What is the root cause of that problem?The app does not recognize a cursor change for this edge case, and the callback of
Since the callback has not been called, the state is outdated. App/src/components/MoneyRequestAmountInput.tsx Lines 143 to 146 in 4f0ca94
Therefore any further input is inserted in order with the outdated state. What changes do you think we should make in order to solve the problem?The app uses BaseTextInputWithCurrencySymbol to display the input. There are platform separation for the component index.android.tsx and index.tsx. It utilizes I propose to create a web-specific file onSelectionChange={(event: NativeSyntheticEvent<TextInputSelectionChangeEventData>) => {
onSelectionChange(event.nativeEvent.selection.start, event.nativeEvent.selection.end);
}}
onPress={() => {
const selectionStart = (textInputRef.current?.selectionStart as number) ?? 0;
const selectionEnd = (textInputRef.current?.selectionEnd as number) ?? 0;
onSelectionChange(selectionStart, selectionEnd);
}} It utilizes one more handler — Video
No.bug.mp4What alternative solutions did you explore? (Optional)N/A |
@mallenexpensify, @rezkiy37, @thesahindia Eep! 4 days overdue now. Issues have feelings too... |
@thesahindia can you review the above proposal plz? thx |
@mallenexpensify, @rezkiy37, @thesahindia 6 days overdue. This is scarier than being forced to listen to Vogon poetry! |
If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!
Version Number: 9.0.18
Reproducible in staging?: Y
Reproducible in production?: Y
If this was caught during regression testing, add the test name, ID and link from TestRail: https://expensify.testrail.io/index.php?/tests/view/4836262
Issue reported by: Applause Internal Team
Action Performed:
Expected Result:
Inserting decimal point, behavior must be consistent in mweb& Android.
Actual Result:
In Android, decimal point is inserted correctly in the same place but in mweb, decimal point is inserted after a number. Inserting decimal point, inconsistent behavior in mweb& Android.
Workaround:
Can the user still use Expensify without this being fixed? Have you informed them of the workaround?
Platforms:
Screenshots/Videos
Bug6565332_1723101158572.Screenrecorder-2024-08-08-12-34-45-180_compress_1.mp4
View all open jobs on GitHub
Upwork Automation - Do Not Edit
Issue Owner
Current Issue Owner: @thesahindiaThe text was updated successfully, but these errors were encountered: