-
Notifications
You must be signed in to change notification settings - Fork 0
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
[ENH] Automated upload functionality #107
Conversation
- Moved the required logic from `App` to `Upload` - Updated layout
- Added state variables to retrieve and store user input from input elements - Updated axios request to align with the new api endpoint
✅ Deploy Preview for upload-ui ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #107 +/- ##
===========================================
+ Coverage 45.22% 56.19% +10.96%
===========================================
Files 14 18 +4
Lines 157 210 +53
Branches 46 50 +4
===========================================
+ Hits 71 118 +47
- Misses 86 92 +6
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @rmanaem, this looks very nice! 🎉 Also a big fan of the virtualization, looking forward to trying this on a react annotator soon for our massive controlled terms lists.
I approved this already because it looks nice and works well, but left a couple of comments for you to go through. Most minor, but take a look and apply what is feasible or shunt to issues what isn't.
🧑🍳
Ah, just noticed one thing: you don't have any test on the new request schema that the upload API expects. I'm assuming you're testing against the prod API during development - but let's also add a basic test that asserts over the schema / and example instance of the new request. |
@surchs I added a test, please take a look and let me know what you think. |
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.
nice, looks good @rmanaem!
🧑🍳
Checklist
This section is for the PR reviewer
[ENH]
,[FIX]
,[REF]
,[TST]
,[CI]
,[MNT]
,[INF]
,[MODEL]
,[DOC]
) (see our Contributing Guidelines for more info)skip-release
(to be applied by maintainers only)Closes #XXXX
For new features:
For bug fixes: