Update number validation to be more strict #1994
Conversation
| @@ -152,6 +115,10 @@ protected void eval() { | |||
| } | |||
| } | |||
|
|
|||
| private String cleanUpFormatting(String inputText) { | |||
| return inputText.replace(",", ""); | |||
There was a problem hiding this comment.
Unfortunately diff. locales use diff. separator here. I am not a fan of those local number formats and we could just use one default number format independent of users locale.
There was a problem hiding this comment.
I think this would be the safest as well. What do you think about using Locale.US as default?
I think ideally we only have the number formatted when the input is in read mode, and then remove the formats when writing, but that adds a lot of unnecessary complexity imo.
| private String cleanUpFormatting(String inputText) { | ||
| return inputText.replace(",", ""); | ||
| } | ||
|
|
||
| private boolean isNumber(String inputText) { | ||
| return inputText.matches("^-?\\d+(\\.\\d+)?$"); |
There was a problem hiding this comment.
Does that respect any decimal and thousands separator format diff. locals?
There was a problem hiding this comment.
It does respect any decimals, but not the thousands separator (that's why the clean-up first). But we could have it implicitly if we define the locale.
I still see that inputs like this are accepted (although later we ignore the decimals):
Therefore I propose just using NumberFormat to validate for now and if we decide on a locale, then we can make it more specific and target those corner cases better. (In another PR)
| model.getIgnoreMinRequiredReputationScoreFromSecManager().set(isSelected); | ||
| minRequiredReputationScore.validate(); | ||
| }); | ||
|
|
There was a problem hiding this comment.
Is the update from the model to the switch handled? It was bi-directional binding before not its like a normal binding of switch to model.
I guess we only need it at active to apply the model date to the switch as the model data gets not changed by any other source.
There was a problem hiding this comment.
Good catch. No need to remove it then, let's just use the listener to trigger validation.
axpoems
force-pushed
the
imp-number-validation
branch
from
c3d0589 to
f727511
Compare
axpoems
force-pushed
the
imp-number-validation
branch
from
f727511 to
8492f31
Compare
Based on #1993
Use a more strict number validation to prevent cases like "123abc" to be flagged as valid.
Before:
After:
Force a validation after updating the switch for both min. required rep. and diff. factor. This avoids getting into an inconsistent state when switching back to default with a previous errorLabel.
Before:
After:
Misc. refactors and improvements.