lmr: add mandatory fields and error messages to the form #10271
Conversation
Signed-off-by: Mathieu <[email protected]>
Caracol3
added
area:front
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## dev #10271 +/- ##
=======================================
Coverage 81.48% 81.49%
=======================================
Files 1058 1058
Lines 104270 104319 +49
Branches 722 722
=======================================
+ Hits 84969 85013 +44
- Misses 19260 19265 +5
Partials 41 41
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Several issues:
- message is not positionned correctly, it should be below the input
- message gets on top of other inputs and we can’t see or interact with them.
- towed field is required but doesn’t allow me to write anything
What prototype did you use?
We should be able to see the main information of the page, and see at first glance what fields failed validation
| const isTotalMassEmpty = totalMass === undefined || totalMass === null; | ||
| const isTotalLengthEmpty = totalLength === undefined || totalLength === null; | ||
| const isMaxSpeedEmpty = maxSpeed === undefined || maxSpeed === null; |
There was a problem hiding this comment.
Can’t we use the same function for all number fields?
There was a problem hiding this comment.
Yes, you're right, it would be clearer. I will change this
There was a problem hiding this comment.
We could use isNil() from lodash ?
This is because of overriding rules, we use in stdcm, we should get rid of these in another PR, it breaks all the osrd-ui styles... (the tooltip top position here)
This field shouldn't be required, it will be changed |
|
Indeed, towed rolling stock isn't required. I will remove it. thanks |
| const isTractionEngineEmpty = !filters.text.trim(); | ||
| const isTowedRollingStockEmpty = !towedRsFilters.text.trim(); | ||
| const isTotalMassEmpty = totalMass === undefined || totalMass === null; | ||
| const isTotalLengthEmpty = totalLength === undefined || totalLength === null; |
There was a problem hiding this comment.
Instead of that, I think you could use the same logic than @Wadjetz in his consist PR (in StdcmConfig). We could have one object having boolean properties for each required fields changing when these fields changes.
We should also setShowError(false) as soon as the field changes.
close #9707