Improving prime Datatable #862
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
BenjaminCharmes
force-pushed
the
bc/next-improvement
branch
from
681b8b3
to
5bd7e24
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #862 +/- ##
=======================================
Coverage 68.22% 68.22%
=======================================
Files 62 62
Lines 3909 3909
=======================================
Hits 2667 2667
Misses 1242 1242
|
BenjaminCharmes
commented
Aug 30, 2024
datalab
|
||||||||||||||||||||||||||||
| Project |
datalab
|
| Branch Review |
bc/next-improvement
|
| Run status |
|
| Run duration | 05m 48s |
| Commit |
|
| Committer | Benjamin Charmes |
| View all properties for this run ↗︎ | |
| Test results | |
|---|---|
Failures
|
0
|
Flaky
|
0
|
Pending
|
0
|
Skipped
|
0
|
Passing
|
131
|
| View all changes introduced in this branch ↗︎ | |
BenjaminCharmes
force-pushed
the
bc/next-improvement
branch
2 times, most recently
from
fd81b56
to
9e3c180
Compare
BenjaminCharmes
force-pushed
the
bc/next-improvement
branch
from
87d9f15
to
935d9b8
Compare
…ry and Equipment Fix global-filter-fields for creators and collections Add dynamic prime table for starting materials + some cleaning Add dynamic prime table for equipments New ButtonPrimeTable component Fix deleteCollection Fix button still clickable after deleting a sample Add API tests for collections/<collection_id> POST route
Rename navbar link
BenjaminCharmes
force-pushed
the
bc/next-improvement
branch
from
935d9b8
to
b7d94a3
Compare
Fix editPage.cy e2e test Fix NavbarTest.cy component test Fix NavbarTest.cy component test
BenjaminCharmes
force-pushed
the
bc/next-improvement
branch
from
7464148
to
9d3e814
Compare
ml-evs
reviewed
Sep 18, 2024
There was a problem hiding this comment.
Looking really good @BenjaminCharmes! Great to see how much old code we can get rid of without breaking anything!
Minor initial comments: I think we should retain the old component names just for consistency (of the git history more than anything). Plus, the name equipment should be kept throughout to match the underlying model.
Sadly I'm still having issues with my local set up where the components within the table are not being used -- I'm still using node 18 locally, so perhaps this is just an a "me problem" but I'd like to at least find the reason before we merge this. Perhaps the minimum node version needs to be updated in package.json too.
I'll give this a more thorough review tomorrow and we can discuss on Friday.
jdbocarsly
requested changes
Sep 20, 2024
There was a problem hiding this comment.
A few notes from today's chat:
- The "select all" button is slow with lots of samples/inventory. Can we change the select button to only select the shown samples as a workaround?
- Each individual checkbox has to be clicked precisely to be clicked. Can we change the @ click even to be on the cell, rather than on the checkbox
- Tests for checking whether components are rendered in the table
- Test for adding to inventory when editable_inventory is turned on etc
- Check whether add to collection select on create new item modal still works
- Consider future refactor around how components are specified in the table, i.e., whether they can be mapped from the datalab server schema
Is there a way to make the filter symbol filled in when a filter is on?"Add a collection" from the CollectionSelect is great, but the spinner seems to be broken as it never goes away even once the collection is createdWhen you insert something into a collection, and then search for the same collection again, it gives you the option to create the collection again, and then causes an error.seems to be possible to "double add" something to a collection that was already in that collection, which causes an error when you try to access the page "relationships and collection mismatch"
ml-evs
reviewed
Sep 24, 2024
7 tasks
This was referenced Sep 24, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
PrimeDataTable (Dynamic DataTable) becomes the default version for Samples, Collections, Inventory and Equipment.
Keep the old SampleTable and EquipmentTable components so that the cypress e2e test still works on the /old-sample and /old-equipment routes long enough to write the new tests on the new table.
And lots of cleanup
Closes #877 & #878