-
Notifications
You must be signed in to change notification settings - Fork 3k
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
[$500] [POLISH][Search v2.1] Amount filter - Native - The digit exceeding the max allowed decimal places is briefly inserted into the input #47875
Comments
Triggered auto assignment to @isabelastisser ( |
Job added to Upwork: https://www.upwork.com/jobs/~017b65a6e5c7b77ba2 |
Triggered auto assignment to Contributor-plus team member for initial proposal review - @brunovjk ( |
Validation Function: Create a function to check if the input exceeds the allowed decimal places. |
📣 @eyobai! 📣
|
ProposalPlease re-state the problem that we are trying to solve in this issue.The digit exceeding the max allowed decimal places is briefly inserted into the input What is the root cause of that problem?This issue occurs because we use onChangeText to validate the amount between lines 37 and line 40. If the amount is not valid, we return and remove the text change in the text input App/src/components/AmountWithoutCurrencyForm.tsx Lines 29 to 43 in 0c84552
What changes do you think we should make in order to solve the problem?We should check that the decimal limit has been reached and no more decimals can be entered before the text changes. // src/components/AmountWithoutCurrencyForm.tsx#L17
+ const decimals = 2;
// src/components/AmountWithoutCurrencyForm.tsx#L37
- if (!validateAmount(withLeadingZero, 2)) {
+ if (!validateAmount(withLeadingZero, decimals)) {
// src/components/AmountWithoutCurrencyForm.tsx#L45
+ const getDecimalLength = (numStr: string) => {
+ const parts = numStr.split('.');
+ return parts[1] ? parts[1].length : 0;
+ };
// src/components/AmountWithoutCurrencyForm.tsx#L46
return (
<TextInput
value={formattedAmount}
onChangeText={setNewAmount}
inputID={inputID}
name={name}
label={label}
defaultValue={defaultValue}
accessibilityLabel={accessibilityLabel}
role={role}
ref={ref}
keyboardType={CONST.KEYBOARD_TYPE.DECIMAL_PAD}
+ maxLength={getDecimalLength(formattedAmount) === decimals ? formattedAmount.length : undefined}
// eslint-disable-next-line react/jsx-props-no-spreading
{...rest}
/>
); POC
Screen.Recording.2024-08-25.at.01.13.48.mov |
Thank you for the proposal @huult, but I tested it and it doesn't seem to resolve the issue. The RCA focuses on |
@brunovjk , Thanks for reviewing my proposal. I double-checked and found that it works when the user types from the keyboard but does not work when pasting a number into the input field or entering multiple decimal points. I am investigating further. |
This most likely is related to a known issue for the |
@luacmartins, @isabelastisser, @brunovjk Uh oh! This issue is overdue by 2 days. Don't forget to update your issues! |
Thank you, @AlfredoAlc, for the reference. It aligns with our issue, but I haven’t found a fix yet, only workarounds. |
@brunovjk , test branch for my proposal POC
Screen.Recording.2024-08-29.at.09.27.32.mp4 |
Thank you @huult :D I just tested. Almost there, but the problem is not just in the decimals: Screen.Recording.2024-08-29.at.10.36.14.movIf the video above is no longer available when you watch it, let me know. |
@brunovjk , I've updated the test branch to fix the issue you tested. Screen.Recording.2024-08-29.at.21.24.51.mp4 |
Thank you for your efforts, @huult. Unfortunately, the solution still isn’t working as expected on my end. As we can't attack the root cause directly, we might need a "workaround" to address this effectively. I’m confident we can find one that won’t introduce regressions. Let’s keep exploring options—I’ll revisit this next week, and we can involve an internal engineer if necessary. Screen.Recording.2024-08-29.at.16.57.22.mov |
📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸 |
@luacmartins, @isabelastisser, @brunovjk Uh oh! This issue is overdue by 2 days. Don't forget to update your issues! |
We decided to give the lib a try. @kirillzyusko this is all yours for implementation! |
Not overdue |
@luacmartins, @kirillzyusko, @isabelastisser, @brunovjk Uh oh! This issue is overdue by 2 days. Don't forget to update your issues! |
@MelvinBot not overdue. I'm working on a fix in #53695 Last remaining thing is to make it able to paste values with commas and automatically transform it to dot format. Currently working on that! |
Not overdue :D |
Update: We are still working on the PR. |
|
The solution for this issue has been 🚀 deployed to production 🚀 in version 9.0.94-25 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue: If no regressions arise, payment will be issued on 2025-02-13. 🎊 For reference, here are some details about the assignees on this issue:
|
@brunovjk @isabelastisser @brunovjk The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed. Please copy/paste the BugZero Checklist from here into a new comment on this GH and complete it. If you have the K2 extension, you can simply click: [this button] |
Do not proceed with payment yet, we are working on this additional PR. |
This is a follow-up issue.
Coming from: #47343 (comment) and #47343 (comment)
Bug in iOS and Android native apps: When entering a number, the digit exceeding the maximum allowed decimal places is briefly inserted into the input field before being deleted. They should not be inserted at all.
Simulator.Screen.Recording.-.iPhone.15.Pro.Max.-.2024-08-15.at.15.52.28.mp4
cc @luacmartins @289Adam289
Upwork Automation - Do Not Edit
Issue Owner
Current Issue Owner: @Issue Owner
Current Issue Owner: @kirillzyuskoThe text was updated successfully, but these errors were encountered: