Allow typing comma separator for projected budget #523
Conversation
Generated by E2E-Test |
melissaSchmidt0302
force-pushed
the
allow-comma-separator
branch
2 times, most recently
from
a080067
to
10bc2cf
Compare
|
e2e-test is missing |
daniel-arnauer
force-pushed
the
allow-comma-separator
branch
from
10bc2cf
to
ba62250
Compare
daniel-arnauer
force-pushed
the
allow-comma-separator
branch
2 times, most recently
from
d7914b2
to
f42a54e
Compare
daniel-arnauer
force-pushed
the
allow-comma-separator
branch
4 times, most recently
from
7f3472d
to
00d5619
Compare
There was a problem hiding this comment.
The current solution formats the budget before adding it to the other projected budgets. If I type in the wrong format and add it to the budgets suddenly the budget is in the right format but also it is another value. This is an unpredictable behavior.
I would have expected that the value stays the same or the program doesn't let me add the budget in that format - giving an input field error message. Do you agree?
|
So i will ad an regex to check the input value. If the value does not match the regex, an input field error is thrown.
Other languages: |
daniel-arnauer
force-pushed
the
allow-comma-separator
branch
2 times, most recently
from
e8b3572
to
bedc752
Compare
frontend/src/helper.js
Outdated
| @@ -94,6 +94,11 @@ export const toAmountString = (amount, currency) => { | |||
| return accounting.formatMoney(amount, getCurrencyFormat(currency)); | |||
| }; | |||
|
|
|||
| export const testMoneyInput = amount => { | |||
There was a problem hiding this comment.
Remove the comment and rename the function so it's clear what the function does
There was a problem hiding this comment.
renamed to validateLanguagePattern(amount)
frontend/src/pages/Common/Budget.js
Outdated
| return ( | ||
| <TableCell align="right" data-test="saved-projected-budget-amount"> | ||
| {isEditing && editIndex === currIndex ? ( | ||
| <TextField | ||
| label={strings.common.projected_budget} | ||
| value={budgetAmountEdit} | ||
| onChange={e => setBudgetAmountEdit(e.target.value)} | ||
| onChange={e => { | ||
| if (/^[0-9,.-]*$/.test(e.target.value)) { |
There was a problem hiding this comment.
explain regex or create own variable to describe what's the condition
There was a problem hiding this comment.
added var numberSignsRegex
| @@ -5,7 +5,8 @@ const fr = { | |||
| decimal: ",", | |||
| thousand: ".", | |||
| precision: 2 | |||
| } | |||
| }, | |||
| numberRegex: /^([0-9]{1,3}.([0-9]{3}.)*[0-9]{3}|[0-9]+)(,[0-9]+)?$/ | |||
There was a problem hiding this comment.
describe regex in a comment
There was a problem hiding this comment.
comments:
// numberRegex describes values with "." as decimal separator (matches e.g. 1000; 1,000; 1000.00; 1,000.00)
or
// numberRegex describes values with "," as decimal separator (matches e.g. 1000; 1.000; 1000,00; 1.000,00)
daniel-arnauer
force-pushed
the
allow-comma-separator
branch
2 times, most recently
from
c8bc437
to
8100ba8
Compare
daniel-arnauer
force-pushed
the
allow-comma-separator
branch
from
8100ba8
to
40f3e33
Compare
The number formating depends on the current language. In english language, the "." is used as comma, in other languages, the "," is used as comma. NPM audit fix also includes.
daniel-arnauer
force-pushed
the
allow-comma-separator
branch
from
284c50f
to
45c835a
Compare
Closes #517
Closes #559
Closes #563