Warn user when a wrong time fromat is used #457
Conversation
| const { dateStart } = this.props; | ||
| const inputValue = returnValidTime(e.target.value, ':'); | ||
| if (isValidTime(inputValue)) { | ||
| showError('Time format is wrong!'); |
There was a problem hiding this comment.
We are not going to use showError to display errors any more.
There was a problem hiding this comment.
@borisyankov what are we going to use instead
?
| onTimeChange = e => { | ||
| const { dateStart } = this.props; | ||
| const inputValue = returnValidTime(e.target.value, ':'); | ||
| this.setState({ isInValidTime: isValidTime(inputValue) }); |
There was a problem hiding this comment.
We don't need this to be state, in can be calculated on render.
|
|
||
| if (durationAllowed) { | ||
| if (isValidTime(time)) { | ||
| yield put(updateTradeError(index, 'durationError', 'Time format is wrong!')); |
There was a problem hiding this comment.
Note that yield is not return, if you want to exit early, explicit return is needed
@nuruddeensalihu
There was a problem hiding this comment.
I think either of the two serve our purpose here, There isn't a code block after the if statement so the block exit there. Plus 'return' is part of what the yield keyword does it self if i understand correctly. I think either of the two should be fine(i.e return or not ). https://developer.mozilla.org/en/docs/Web/JavaScript/Reference/Operators/yield
There was a problem hiding this comment.
It's better to have
if (xxx) {
...
return
}
if (yyyy) {
...
return
}
than to have
if (xxx) {
...
} else if (yyyy) {
...
}
...
return
The first one is generally better, it tells reader that code exit with that condition, and that's all it does, 2nd chunk however requires reader to go until the end of code to know what's going on
I might have make the mistake at the first place, would you help me to fix it altogether?
Thanks
There was a problem hiding this comment.
@qingweibinary would have a look.
4 participants
The attached PR show simply warn user on the timeformat should the format be wrong . E.g
33:333:23. Hrs shouldn't be greater than 24 presumably Hence the warning. This however is not a wrong time as 33 hrs can be translated to 2 days 9 hrs and the two days be added to the date field should the user choose to proceed.Ticket : https://trello.com/c/kv3hj0KG/37-github-nuru-safari-firefox-no-validations-for-date-time-fields