-
Notifications
You must be signed in to change notification settings - Fork 245
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
(fix) Pass patient as a prop where-ever possible in the chart #2015
Conversation
Size Change: -1.77 MB (-10.71%) 👏 Total Size: 14.8 MB
ℹ️ View Unchanged
|
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.
LGTM
@@ -58,7 +57,7 @@ const CapturePhoto: React.FC<CapturePhotoProps> = ({ initialState, onCapturePhot | |||
className={styles.editButton} | |||
kind="ghost" | |||
onClick={showCam} | |||
renderIcon={(props) => <Edit {...props} />} | |||
renderIcon={(props: Partial<Parameters<typeof EditIcon>[0]>) => <EditIcon {...props} />} |
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.
Comparing apples to apples, but maybe this signature could be simplified to:
renderIcon={(props: Partial<Parameters<typeof EditIcon>[0]>) => <EditIcon {...props} />} | |
renderIcon={(props: React.ComponentProps<typeof EditIcon>) => <EditIcon {...props} />} |
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.
Actually, no. That's way more explicit, even if not necessarily shorter. Let me fix that.
In retrospect, I probably should've done the icons in a separate PR, but here we hare. |
Let's see how this pans out in the refapp! |
Requirements
Summary
The bulk of the changes here focus on ensuring that we are using the
fhir.Patient
object from the Patient Chart state wherever possible, meaning that most components do not require a separate call tousePatient()
. I've re-written a few hooks to adjust for this (i.e.,usePatientDeceasedStatus()
is basically just a hook that gets a boolean result based on the patient).There are a few miscellaneous changes:
Screenshots
Related Issue
Other
Unchanged for now are most workspaces and panels.
This removes about 7 requests when loading the chart summary and 1-per-row when loading the result viewer, so the chart should feel snappier.
I'm unsure about some of the action buttons functioning when the banner is rendered in the search results, since this uses dummied FHIR objects. However, I'm of the view that actions in the search results are bad UX and should be removed.