[HOLD for payment 2022-09-26] [$250] User can't use , (comma) In spanish for decimals during IOU - reported by @thesahindia

[mvtglobally]

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. Go to setting > Workspace > Reimburse expenses page
2. Try using comma in the Rate input

Expected Result:

User should be able to use comma, also the decimal should change to comma after changing the language to spanish.

Actual Result:

User can't use comma

Workaround:

unknown

Platform:

Where is this issue occurring?

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

Version Number: 1.1.77-0
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: Any additional supporting documentation

Expensify/Expensify Issue URL:
Issue reported by: @thesahindia
Slack conversation: https://expensify.slack.com/archives/C01GTK53T8Q/p1655491691902889

Job: https://www.upwork.com/jobs/~0174a215853c604f4d

View all open jobs on GitHub

[melvin-bot]

Triggered auto assignment to @isabelastisser (AutoAssignerTriage), see https://stackoverflow.com/c/expensify/questions/4749 for more details.

[melvin-bot]

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

[melvin-bot]

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

[Beamanator]

@kevinksullivan reassigning you b/c I think @joelbettner unassigned you on accident 😅

[kevinksullivan]

Just adding that we believe this already exists for the send/request money flows, so that code likely offers some guidance.

[melvin-bot]

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

[melvin-bot]

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

[kevinksullivan]

@thesahindia please apply for reporting. Thanks!

https://www.upwork.com/jobs/~0174a215853c604f4d

[eVoloshchak]

Proposal:
I'm not sure if this input is considered a Form input, but FORMS.md states that

User input that may include optional characters (e.g. (, ), - in a phone number) should never be restricted on input, nor be modified or formatted on blur. This type of input jacking is disconcerting and makes things feel broken.
Instead we will format and clean the user input internally before using the value (e.g. making an API request where the user will never see this transformation happen)

And I agree with that since I've also tried pressing , key instead of ., even though I'm not using Spanish (many countries use , instead of . as a separator)

I'm proposing we allow user to use both , and . and then just replace comma with a dot when user presses Next

1. Replace

   App/src/pages/iou/steps/IOUAmountPage.js

   Line 122 in 406040d

   const decimalNumberRegex = new RegExp(/^\d+(,\d+)*(\.\d{0,2})?$/, 'i');

   with

const decimalNumberRegex = new RegExp(/^\d+([,.]\d{0,2})?$/, 'i');

2. Replace

   App/src/pages/iou/steps/IOUAmountPage.js

   Line 239 in 406040d

   onPress={() => this.props.onStepComplete(this.state.amount)}

   with

onPress={() => this.props.onStepComplete(this.state.amount.replace(',', '.'))}

3. Remove all of the calls to this.stripCommaFromAmount

Result:

cinnamon-20220625-1.mp4

UPD
OK, i think i got confused, since the issue description is talking about the Rate input on Reimburce Expenses screen, while screenshot contains IOU Amount Page with an amount input. If the description is correct, we need to do the following:

1. Replace

   App/src/CONST.js

   Line 668 in 406040d

   RATE_VALUE: /^\d{1,8}(\.\d*)?$/,

   with

RATE_VALUE: /^\d{1,8}([,.]\d{0,3})?$/,

2. Edit updateRateValue method in WorkspaceReimburseView so that it retains the separator the user has entered while sending a number with dot to the server

    updateRateValue(value) {
        const numValue = parseFloat(value.replace(',', '.'));

        if (_.isNaN(numValue)) {
            return;
        }

        const rateToDisplay = value.includes(',') ? numValue.toFixed(3).replace('.', ',') : numValue.toFixed(3);

        this.setState({
            rateValue: rateToDisplay,
        });

        Policy.setCustomUnitRate(this.props.policyID, this.state.unitID, {
            customUnitRateID: this.state.rateID,
            name: this.state.rateName,
            rate: numValue.toFixed(3) * 100,
        }, null);
    }

Result:

cinnamon-20220625-2.mp4

On a side note, I personally think that updating input's value so that the number always has 3 digits after the separator looks suboptimal (notice that on the video). We could just delete const rateToDisplay and setState after that and the user's input will not be modified on the UI side. What do you think?

[Santhosh-Sellavel]

@eVoloshchak PR has been reverted.

[eVoloshchak]

@eVoloshchak PR has been reverted.

Started working on the PR.
I'm trying to tackle #10108 and there seems to be no keyboard type on iOS that features comma instead of period with numbers. We are using type decimalPad. Instead, it is locale dependent. If user has their phone's language set to Spanish, decimal keyboard will have a comma instead of period

However, if someone was to have a phone set to English with a New Expensify app set Spanish, that would create a problem. What should I do here?

[Santhosh-Sellavel]

@eVoloshchak Lets get others thoughts on slack can you create a thread there.

[eVoloshchak]

Created a thread here

[Santhosh-Sellavel]

@eVoloshchak How/which Spanish language did you set to get this keyboard with a comma, can you share the recording for the same?

[eVoloshchak]

@eVoloshchak How/which Spanish language did you set to get this keyboard with a comma, can you share the recording for the same?

cinnamon-20220805-10.mp4

[Santhosh-Sellavel]

But I'm not seeing a comma there,

Simulator.Screen.Recording.-.iPhone.13.-.2022-08-08.at.22.31.00.mp4

[Santhosh-Sellavel]

@eVoloshchak Bump

Any update on this issue

[eVoloshchak]

Any update on this issue

It seems like a consensus has been achieved:

Android can show both symbols, so there we can just treat each like the configured locale in the app would (ie: if you have english, it would treat . as decimal, if you have spanish it would treat, as decimal.
For iOS it always shows the phone's locale decimal symbol, so we know it will only have one symbol and whichever symbol that is, is the decimal separator and parse it accordingly.

So we accept both signs on IOS and only the correct one on Android. I'll submit a PR shortly

[eVoloshchak]

PR is up

[melvin-bot]

This issue has not been updated in over 15 days. @sketchydroide, @eVoloshchak, @kevinksullivan, @Santhosh-Sellavel eroding to Monthly issue.

P.S. Is everyone reading this sure this is really a near-term priority? Be brave: if you disagree, go ahead and close it out. If someone disagrees, they'll reopen it, and if they don't: one less thing to do!

[kevinksullivan]

PR in review.

[melvin-bot]

⚠️ Looks like this issue was linked to a Deploy Blocker here

If you are the assigned CME please investigate whether the linked PR caused a regression and leave a comment with the results.

If a regression has occurred and you are the assigned CM follow the instructions here.

If this regression could have been avoided please consider also proposing a recommendation to the PR checklist so that we can avoid it in the future.

[melvin-bot]

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.2.1-0 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• Localize decimal separator for the rate field  #10468

If no regressions arise, payment will be issued on 2022-09-26. 🎊

[Santhosh-Sellavel]

@kevinksullivan bump!

[kevinksullivan]

Paid @Santhosh-Sellavel and @eVoloshchak

[kevinksullivan]

@thesahindia please accept this reposted job to get paid for reporting.

https://www.upwork.com/jobs/~01095c4f1bfc429522

[thesahindia]

@thesahindia please accept this reposted job to get paid for reporting.

Applied to the job. Thanks.

[Santhosh-Sellavel]

@kevinksullivan Can you close this issue and the other issue #10108 as this same problem mentioned here #9497 (comment) and it is fixed now, thanks!