-
Notifications
You must be signed in to change notification settings - Fork 1
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
Allow upload model #103
Allow upload model #103
Conversation
…NOTES into feature/upload-in-search
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.
Comments with a couple of commits (as proposals).
app/assets/javascripts/3dbio_viewer/src/domain/usecases/UploadAtomicStructureUseCase.ts
Outdated
Show resolved
Hide resolved
app/assets/javascripts/3dbio_viewer/src/webapp/components/dropzone/Dropzone.tsx
Outdated
Show resolved
Hide resolved
app/assets/javascripts/3dbio_viewer/src/webapp/components/model-upload/ModelUpload.tsx
Outdated
Show resolved
Hide resolved
app/assets/javascripts/3dbio_viewer/src/webapp/components/model-upload/ModelUpload.tsx
Outdated
Show resolved
Hide resolved
app/assets/javascripts/3dbio_viewer/src/data/repositories/EbiDbModelRepository.ts
Outdated
Show resolved
Hide resolved
app/assets/javascripts/3dbio_viewer/src/webapp/components/model-upload/ModelUpload.tsx
Outdated
Show resolved
Hide resolved
app/assets/javascripts/3dbio_viewer/src/webapp/components/model-upload/ModelUpload.tsx
Outdated
Show resolved
Hide resolved
app/assets/javascripts/3dbio_viewer/src/webapp/pages/app/App.css
Outdated
Show resolved
Hide resolved
app/assets/javascripts/3dbio_viewer/src/webapp/components/model-upload/ModelUpload.tsx
Outdated
Show resolved
Hide resolved
app/assets/javascripts/3dbio_viewer/src/webapp/components/model-upload/ModelUpload.tsx
Show resolved
Hide resolved
app/assets/javascripts/3dbio_viewer/src/webapp/components/dropzone/Dropzone.tsx
Outdated
Show resolved
Hide resolved
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.
changes look good!
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.
A couple of minor comments, already commited, all good!
<label className="fileFormat">{i18n.t("Upload your annotations")}</label> | ||
<Dropzone ref={annotationFileRef} accept={"application/json"}></Dropzone> | ||
|
||
<button className="uploadSubmit" onClick={useCallbackEffect(onSubmitHandler)}> |
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.
Hook calls should be outside jsx blocks, so const submit = useCallbackEffect(onSubmitHandler)
. It's not only cleaner but also necessary, hooks must be executed always in the same order, and that JSX tag could be inside a conditional.
const structureFileRef = useRef<DropzoneRef>(null); | ||
const annotationFileRef = useRef<DropzoneRef>(null); | ||
|
||
const onSubmitHandler = useCallback(() => { |
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.
[good practices comment] The prop is called onSubmitHandler
, that's ok, it's a generic function, but here we know what it does, so let's use a more declarative name on what it does. For example: const search = ...
.
@@ -47,6 +54,10 @@ export class EbiDbModelRepository implements DbModelRepository { | |||
.value() | |||
); | |||
} | |||
upload(options: UploadOptions): FutureData<unknown> { | |||
const uploadResults = request(config.upload.searchUrl, options); |
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.
We'll need a multipart/form-data POST to actually post these files, but let's leave this for the next PR, as here we concern ourselves mainly with the UI.
📌 References
📝 Implementation
🎨 Screenshots
🔥 How to test it? (If there is any special consideration or the reviewer does not know how to test it)
📑 Others