-
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
Dynamic medical emergency page #16
Conversation
…Screen after pressing 'Navigate to ConditionsSection'; must have device connected on same network and use ipconfig or ifconfig to put IP address in requests.ts const API_BASE_URL otherwise network error is received
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.
Great work guys! Just a few changes I commented about - let me know if you have any clarifying questions.
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 to keep this file for later use, but this shouldn't be the responsibility of the ConditionsSection
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 overall, make sure to get rid of any commented code. Also, note that the Condiitons Section will be showing data that is fetched from the device's storage, not from the MongoDB database. So for the scope of this pull request, just don't worry about fetching the data, and just make the data be passed in as a prop to the component. In the future, we will have a dedicated file that fetches the correct data for a specific emergency/condition, and passes it into the Conditions Section component
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.
However, the progress you have made on emergencies.ts and the fetching will be useful later, especially for admin mode stuff, so great work and feel free to keep this! Just make sure the ConditionsSection.tsx only focuses on showing data that is passed in.
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 as discussed here have been addressed with the commits below by having ConditionsSection take in an Emergency as input as opposed to an id. Tests have been reconfigured accordingly.
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.
get rid of this empty file
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.
File has been deleted in a commit below.
…. Fixed test cases in dummy HomeScreen.tsx to reflect this. Removed awkward vertical spacings in ConditionsSection styling.
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.
Besides for the commented code, it looks good to me. Great work!
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.
Aside from the feedback Anthony gave, this looks good to me!
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 good to go! Just make sure to remove your changes to App.tsx for now
Tracking Info
Resolves #9
Changes
Changed conditions section from static rendering of documents to dynamic rendering which relies on pulling IDs from the backend database to retrieve the documents and displaying them on your device screen.
Currently, a dummy home screen is used to trigger any changes
Update from Meeting 2-15: Recursive rendering has been implemented meaning that headers, subheaders, subsubheaders, etc. and associated plaintext strings or bulleted string lists (as specified by user headers) are rendered.
Testing and Confirmation of Change
In order to test this code, be sure to follow pre-setup:
.env
file that connects to the database andnpm install
followed bynpm start
have been utilized.env
file set to reference documents from RohanTest database:requests.ts
(search "note to reviewers"). Make sure to useipconfig
for Windows orifconfig
for Mac/Linux.So in this case, I would insert
100.112.104.53
in the designated place after thehttp://
part and before the:3001
partnpm install
andnpx expo start
in the frontend folderSteps for Testing:
Testing Screenshots:
Currently, the feature seems to be working relatively well on Android phone and iPad as shown in the screenshots below.
TODO: Points of Improvement
Dummy home page needs to be replaced by global search page
Fix styling space by adding padding below
There is some extra spacing below the title and above the first header that needs to be resolved
Potential resultant linter or package issues that may arise
Feel free to add commits and push as necessary
DM Anthony, Eshaan, or Rohan with questions