-
Notifications
You must be signed in to change notification settings - Fork 241
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
(chore) Refactor ward patient card #1273
Conversation
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.
@chibongho generally looks good to me.
I didn't review it with a fine-toothed comb :) but the new WardPatient definition totally makes sense to me, and it looks like most of the other changes were simplifying other props/elements using the new WardPatient type? Great!
* The admission detail. This object is only set if the patient has been | ||
* admitted to the ward | ||
*/ | ||
inpatientAdmission: InpatientAdmission; |
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.
Doesn't this need a question mark? Or are you enforcing that it is explicitly set to null?
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.
Yeah, I'd rather explicitly set these to null. (In fact, let me remove the ?
from the bed
field)
* The admission request. The object is only set if the patient has a | ||
* pending admission / transfer request. | ||
*/ | ||
inpatientRequest: InpatientRequest; |
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.
Same question here.
<div>{getPatientName(patient)} </div> | ||
<div className={styles.headerPatientDetail}>· {patient.gender}</div> | ||
<div className={styles.headerPatientDetail}>· {age(patient.birthDate)}</div> | ||
<div>{patient.person.display} </div> |
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.
Are we swtiching back to display here because it applies the name template formatting and getPatientName does not?
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.
I changed this workspace to not use usePatient()
because we already have the Patient
object and don't need another fetch. usePatient()
actually returns fhir.Patient
, so I need to rework some of this since the fields are a bit different between those 2 types. Patient.display
should respect the name template formatting defined server-side, and getPatientName
is only used for fhir.Patient
.
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.
Hi @chibongho,
Thank you for your work on this. Could you please add some screenshots or screen recordings to help illustrate the changes?
Thanks again!
@usamaidrsk there should be no UI changes. Here're some screenshots to verify that the exisiting UI is still behaving correctly: |
Size Change: -8.12 kB (-0.14%) Total Size: 5.9 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.
Love it
Requirements
Summary
This PR refactors the
WardPatient
type to have values populated from theinpatientAdmission
,inpatientRequest
andadmissionLocation
endpoints wrapped in their own fields. It also conslidates a few prop types separated defined for the WardPatientCard, AdissionRequestCard, and workspace PatientBanner into a common type.Testing done:
Manually verified the follow loads correctly:
Screenshots
Related Issue
Other