Replaced some NumberPickers to reduce the number of warnings#3161
Replaced some NumberPickers to reduce the number of warnings#3161offtherailz wants to merge 3 commits intogeosolutions-it:masterfrom
Conversation
| const isValid = v => { | ||
| return TEMP_NUMBER_REGEX.test(`${v}`); | ||
| }; | ||
| const isNumber = v => { |
There was a problem hiding this comment.
why don't we use lodash isNumber?
There was a problem hiding this comment.
Is a little different: isNumber don't exclude -Infinity, +Infinity, NaN. Maybe I can use _.isFinite somehow, but the scope of this function is to check if the number inserted by the user "as string" is a valid number.
| return !isNaN(vv) && isValid(vv); | ||
| }; | ||
|
|
||
| const round = (v, precision = 0) => { |
There was a problem hiding this comment.
Is this different than lodash round?
There was a problem hiding this comment.
Not sure, it returns a string. I get it from the original NumberPicker. It also avoid float .99999 issues
There was a problem hiding this comment.
The purpose is to re-use as much as possible the logic of the original NumberPicker.
See this
| @@ -0,0 +1,105 @@ | |||
| /* | |||
There was a problem hiding this comment.
I don't get any documentation for this component, not even from PropTypes, so it's not very clear how to use it.
There was a problem hiding this comment.
Sure, I should document it. Going to do it
| </Col> | ||
| <Col xs="4"> | ||
| <NumberPicker | ||
| format="- ###.###" |
There was a problem hiding this comment.
So we lost the ability to specify a format... we can live without it, I think
There was a problem hiding this comment.
Yes, for the moment I couldn't implement all the functionalities of the original one.
| <NumberPicker | ||
| format="- ###.###" | ||
| value={classItem.max} | ||
| precision={3} |
There was a problem hiding this comment.
So, we also lost the ability to specify precision, but I can see precision handling in the component. How can I use it?
There was a problem hiding this comment.
I removed this because is misaligned with NumberPicker for min value, so I thought it was a typo.
|
@offtherailz, documentation needed before merging this. |
|
@mbarto I found a second to document a little bit more the component. Please take a look, so we can merge this. |
|
This one (#3207) should solve the same issue, completely, without code rewrite. If you don't see any drawback I think it's preferrable. |
|
Great solution! didn't found this option. Going to close this PR |
Description
Last travis and jenkins builds became unreadable. This is mainly due to ThematicMaps usage of NumberPickers. Tests on this
react-widgetscomponent create a lot of warnings that make the log grow a lot.This replaces NumberPicker for ThematicMaps and for FilterField, where the NumberPickers is used with minimal functionalities, with an home made class that preserves the style of react widgets and implements only minimal functionalities.
The NumberPicker component is also used in styler for shape file and raster styler (that I don't know if it is yet used).
---> Please test thematic maps before merge this pull request. <--
Please check if the PR fulfills these requirements
What kind of change does this PR introduce? (check one with "x", remove the others)
What is the current behavior? (You can also link to an open issue here)
Travis log is very full of warning like this
What is the new behavior?
The log is still big, the warnings above was not removed at all, but a lot of them is not there anymore. This makes the log a little more manageble.
Does this PR introduce a breaking change? (check one with "x", remove the other)