-
Notifications
You must be signed in to change notification settings - Fork 7
Improve UX of batch selection in boxes#new #1867
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
|
The component isn't generic but tied to the boxes#create action, and goes much further than selecting a list of batches: for each batch we select. Extracts BoxBatchForm to handle the individual form for each form, with one to many concentrations. The React component now merely generates a plain form, never really removing the actual inputs from the DOM, so they're always sent with the form (no need for duplicated hidden inputs). Last but not least, allows to reopen a batch form.
21c0017 to
11f7b6b
Compare
|
Just brillant!! I've pulled and tested and it's working neat. The only important thing to add IMO, and I think this wasn't before the refactor Julien so it's not your fault, is the same validations for the samples creation than for the batches creation:
I'm going through the code in detail and it looks great, let me take another look before approving ─the PR is quite long and some stuff is new to me, I'm learning while reading some cool stuff :)─ |
leandroradusky
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I went over the code and functionality and it looks good to me :)
Overall refactor of the React component and submitted params. BoxBatchesForm is roughly equivalent to BatchesSelector, but with an extracted BoxBatchForm sub-component and a somewhat simplified params, the component generates inputs to the form that is eventually sent by the browser (the forms are always rendered, but hidden with
display:none).No need to click the "ADD BATCH" or "ADD SAMPLE" buttons after choosing to create new samples or selecting. You can immediately go to select your first batch or sample.
No need to manually validate each concentration anymore: just enter valid values, otherwise the OK button is disabled. I removed the CANCEL button as the behavior was confusing to implement. it was currently remove batch, when it feels closer to undo (which didn't felt trivial to implement).
The backend correctly verifies & reports errors for each purpose (verified with integration tests). On error, the form is re-populated. This works for both batches & samples. It should no longer be possible to create a box without samples, hence avoiding #1858. This is validated by integration tests.
Last but not least, I added a "down chevron" to allow reopening a batch.
closes #1845
closes #1846
closes #1859
Potential improvements: