-
Notifications
You must be signed in to change notification settings - Fork 17
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
Add ability to add batch of cells #797
Conversation
bceeac4
to
d8fa5bd
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #797 +/- ##
==========================================
- Coverage 67.26% 67.25% -0.01%
==========================================
Files 62 62
Lines 3858 3857 -1
==========================================
- Hits 2595 2594 -1
Misses 1263 1263
|
Test summaryRun details
This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Cloud |
49d0da0
to
1137cd8
Compare
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.
Perfect, everything works and looks nice. 👍
A small difference with "Add an item" is that Cell components are not written as optional in "Add batch of cells", If you want to add it.
1137cd8
to
fc1f713
Compare
@BenjaminCharmes This is a good point, but I just tried adding in (optional) and it makes it a little bit messy, especially on small fields. I think that people will be able to figure it out pretty well, but if we get questions from users about this we can revisit in the future. In general, I think this component probably needs to be redesigned to be more flexible/powerful at some point, and also mobile-friendly. |
c6dd527
to
1319b45
Compare
1319b45
to
e729b6f
Compare
…gets rid of console warnings)
rename samples -> items in Samples.vue (code and gui) add ability to add cell constituents in the batch modal rename BatchCreateSampleModal.vue to BatchCreateItemModal.vue rename various instances of sample->item in the source code for BatchCreateItemModal.vue use placeholder to indicate poisitive electrode, negative electrode, and electrolyte fields when adding a batch of cells fix typo in BatchCreateItemModal
…properly add in the additional electrode/electrolyte components unless the cell already had a component already
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.
Looks great, thanks @jdbocarsly! Will push an additional test to this PR for the Python change, but otherwise this is ready to go.
I'm going to test it on the demo server with some legacy data immediately after merging, and if all is good I'll make a release.
Currently, the "batch add" feature only allows for adding samples. This PR adds the ability to also add cells, with a template option for positive electrode, electroylte, and negative electrode.
taggable
in theItemSelect
components in the batch sample modalCloses #396