LPS-97292 Add numeric tests #1063
Conversation
Last reference was removed in a40d23f.
Main mechanism is updating to liferay-npm-scripts v4.6.0, which includes a "liferay/metal" preset for dealing with portions of the codebase that use metal-jsx rather than React. This allows us to remove suppressions for 34 problems. Other suppressions were already addressed in prior commits (eg. 84ea82e), but we forgot to remove the suppression comments.
|
Just started reviewing :) |
Sent from 
| const numberMaskOptions = this.getMaskConfig(dataType); | ||
|
|
||
| if (value && dataType == 'integer') { | ||
| inputElement.value = Math.round(value.replace(',', '.')); |
There was a problem hiding this comment.
@alinedoleron is there a reason for using hardcoded comma instead of the symbol variable?
| this.emit('fieldEdited', { | ||
| fieldInstance: this, | ||
| originalEvent: event, | ||
| if (value.substr(-1) != this.symbols.decimalSymbol) { |
There was a problem hiding this comment.
@alinedoleron We can now do early return instead of wrapping the entire method inside an if statement
| @@ -122,7 +131,6 @@ class Numeric extends Component { | |||
|
|
|||
| _internalValueFn() { | |||
| const {value} = this; | |||
|
|
|||
There was a problem hiding this comment.
@alinedoleron Not sure we have a rule for that, but I prefer a line between variable declarations and the return statement.
| expect.objectContaining({ | ||
| fieldInstance: component, | ||
| originalEvent: expect.any(Object), | ||
| value: expect.any(String) |
There was a problem hiding this comment.
@alinedoleron I think it would be better to explicitly say the value we expect here.
alinedoleron
force-pushed
the
LPS-97313_v2
branch
from
95fb83e
to
b653326
Compare
|
ci:test:sf |
liferay-continuous-integration
added
ci:test:sf - pending
and removed
ci:test:sf - success
labels
|
Thanks, @brunobasto. I've made the changes. |
liferay-continuous-integration
added
ci:test:sf - success
and removed
ci:test:sf - pending
labels
✔️ ci:test:sf - 1 out of 1 jobs passed in 3 minutes 33 seconds 4 msClick here for more details.Base Branch:Branch Name: master Sender Branch:Branch Name: LPS-97313_v2 1 Successful Jobs:For more details click here. |
|
Just started reviewing :) |
|
Pull request submitted to https://github.com/jeyvison/liferay-portal/pull/518. See changes here. |
|
Pull request submitted to https://github.com/jeyvison/liferay-portal/pull/519. See changes here. |
No description provided.