-
Notifications
You must be signed in to change notification settings - Fork 5k
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
fix: Prevent coercing symbols to zero in the edit spending cap modal #28192
Conversation
CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes. |
3a8039d
to
e0b1cf3
Compare
Builds ready [e0b1cf3]
Page Load Metrics (1931 ± 101 ms)
Bundle size diffs [🚨 Warning! Bundle size has increased!]
|
@@ -124,6 +124,8 @@ export const EditSpendingCapModal = ({ | |||
decimals && | |||
parseInt(decimals, 10) < countDecimalDigits(customSpendingCapInputValue); | |||
|
|||
const showSpecialCharacterError = /[-+e]/u.test(customSpendingCapInputValue); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor, is it worth a constant for the regex such as SPECIAL_CHARACTERS_PATTERN
?
@@ -171,6 +173,15 @@ export const EditSpendingCapModal = ({ | |||
{t('editSpendingCapError', [decimals])} | |||
</Text> | |||
)} | |||
{showSpecialCharacterError && ( | |||
<Text |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor, is it worth a local Error
component to avoid the duplication with the above decimal error?
@pedronfigueiredo spendingCapZero.mov |
Hey @sleepytanya, thanks for flagging this. It is expected! |
Description
Since the
TextField
in the "edit spending cap" modal has a typeTextFieldType.Number
, it already blocks most symbols and letters. However, it does currently support+
,-
ande
characters as they can be used to construe numbers.For example, when a
-
sign is introduced in the input field, the interim value is coerced to''
, as there is no numerical equivalent to the sign by itself. The first part of this fix was to disable the "Save" button on such cases. If the user wants to revoke the spending cap, they can introduce0
, but''
is not a valid response.Furthermore, when a valid number is introduced but that uses scientific notation or
+
/-
signs, the submission is disabled and a validation message is shown to the user: "Enter numbers only".Related issues
Fixes: #28096
Manual testing steps
Screenshots/Recordings
Before
After
Pre-merge author checklist
Pre-merge reviewer checklist