FirstStep.tsx: Improve otherAmount input field #1478
Conversation
-Added an upper donation limit of BGN 1.500.000(Stripe allows up to $1M for a single transaction, so we are close enough); -Fixed conflicting labels with input borders; -Fixed Firefox incompatibility, which caused the input field to accept letters and symbols
|
✅ Tests will run for this PR. Once they succeed it can be merged. |
ani-kalpachka
added
the
run tests
github-actions
bot
removed
the
run tests
github-actions
bot
removed
the
run tests
-Fixes e2e tests
Consistency reasons
sashko9807
force-pushed
the
donation-otherAmount-improvement
branch
from
3c33c04
to
6297fe8
Compare
| onKeyDown={(e) => { | ||
| if ( | ||
| formik.errors.otherAmount && | ||
| e.key !== 'Backspace' && | ||
| e.key !== 'Delete' && | ||
| !isInteger(formik.values.otherAmount) | ||
| ) { | ||
| e.preventDefault() | ||
| return | ||
| } | ||
| if (decimalSeparator !== e.key && (e.key === '.' || e.key === ',')) { | ||
| e.preventDefault() |
There was a problem hiding this comment.
it looks the onKeyDown is quite long and deserves to be a separate function, to keep the main code readable
There was a problem hiding this comment.
Considering that the onChange and onPaste behaviors are also changed, should I just extract the whole input field to its own component?
There was a problem hiding this comment.
great suggestion, however I was thinking we should not spend much time on something that the browser should support out-of-the-box. So let's leave the extraction for a later stage unless you want to do it now :)
| onKeyDown={(e) => { | ||
| if ( | ||
| formik.errors.otherAmount && | ||
| e.key !== 'Backspace' && | ||
| e.key !== 'Delete' && | ||
| !isInteger(formik.values.otherAmount) | ||
| ) { | ||
| e.preventDefault() | ||
| return | ||
| } | ||
| if (decimalSeparator !== e.key && (e.key === '.' || e.key === ',')) { | ||
| e.preventDefault() | ||
| return | ||
| } | ||
|
|
||
| if ( | ||
| (e.key.charCodeAt(0) >= 48 && e.key.charCodeAt(0) <= 57) || |
There was a problem hiding this comment.
looks like repeated code from the onKeyDown above, so definitely needs to be a reusable function
github-actions
bot
removed
the
run tests
Breaks the float numbers.
github-actions
bot
removed
the
run tests
| (e.ctrlKey && e.key === 'c') || | ||
| e.key === 'Backspace' || | ||
| e.key === 'Delete' || | ||
| (e.ctrlKey && e.key === 'a') || |
There was a problem hiding this comment.
I've tested after the last changes and noticed that on Mac the Copy/Paste/SelectAll combinations are not working. The ctrlKey on Mac is e.metaKey - could you include in the logic too.
There was a problem hiding this comment.
Done, but I am unable to test it.
github-actions
bot
removed
the
run tests
-Added an upper donation limit of BGN 1.500.000(Stripe allows up to $1M for a single transaction, so we are close enough); -
-Fixed conflicting labels with input borders;
-Fixed Firefox incompatibility, which caused the input field to accept letters and symbols
Closes: Reported in discord
Testing
Steps to test