Feature/patient original file

yalla-coop / curenetics

A platform enabling medical professionals to more quickly find relevant clinical trials for their patients

2 stars 0 forks source link

Feature/patient original file #97

Closed othman-shamla closed 4 years ago

othman-shamla commented 4 years ago

related #60 #80

NOTE

in upload page i left the state of the input to app to pass it to patient-list page
i used modal component from ant-design to view the pdf
i used react-pdf to render the pdf from state

problems

viewing the pdf its not that good in mobile
there is a warning from react-pdf, can't make it disappear

how it works?

go to /upload page
upload pdf that match any fileReference name in dummy data like (001001.pdf , 001131.pdf)
pressing upload button will send you the patient-list page (for now)
press (Click to view original file) button that match the pdf name you uploaded

wright1 commented 4 years ago

@othman-shamla I'm finding it challenging to follow your code. This is not a reflection of your code more a lack of my knowledge. Would it be possible for you to put a few comments in especially around your mapping/filter functions? Thank you.

wright1 commented 4 years ago

@othman-shamla Thanks for the comments they were really helpful. The only question I have is why do we have to change the name of the file before we're able to view it?

othman-shamla commented 4 years ago

this PR just to demonstrate the functionality of viewing the PDF of your uploaded file this restriction will removed in this issue #21 after making sure the API is running

so this restriction is just because in the dummy data we have patients fileReference with 001001, 001131 my assumption for now fileReference is just the name of the PDF

in other words its just for linking the patient dummy data to the PDF that we uploaded to demonstrate the functionality @wright1

othman-shamla commented 4 years ago

yes this the warning i told you about on the stand-up yesterday its working yes, but i cant remove this warning.

wright1 commented 4 years ago

yes, this is a know issue with react/pdf The solution is here: https://github.com/wojtekmaj/react-pdf/issues/280 scroll down to @maciejmatu I found that his solution work but it means taking a few steps. including adding another dependency, creating another file with a small script , also changing the json so that it include a new start script. It was shortly after this I decided to use another module. How important is it that this error is resolved?

othman-shamla commented 4 years ago

this a workaround for not showing the warring, not a solution but i'm happy to apply it

othman-shamla commented 4 years ago

i don't know if its okay to add two modules (craco, lodash), i don't think its worth it i think the solution, just to make sure CI for them pass but for us travis and netlify, there are green and no problem when building the app what do you think @wright1 ?

wright1 commented 4 years ago

I think you may be right @othman-shamla What do you think @thejoefriel ???