O3-3210 ward app - configuration system for ward patient cards #1184
Conversation
chibongho
requested review from
mseaton,
brandones,
mogoodrich,
denniskigen,
vasharma05,
ibacher and
usamaidrsk
|
Size Change: -13.9 kB (-0.4%) Total Size: 3.48 MB
ℹ️ View Unchanged
|
| {patients.map((patient, index: number) => { | ||
| const last = index === patients.length - 1; | ||
| return ( | ||
| <div key={patient.uuid + ' ' + index}> |
There was a problem hiding this comment.
Shouldn't patient.uuid be unique?
There was a problem hiding this comment.
However you should append a namespace for the key. We don't want random components around the app all having their key equal the patient UUID.
| <div key={patient.uuid + ' ' + index}> | |
| <div key={`occupied-bed-pt-${patient.uuid}`}> |
| @@ -0,0 +1,24 @@ | |||
| @use '@carbon/styles/scss/spacing'; | |||
| @use '@carbon/styles/scss/type'; | |||
| @import '~@openmrs/esm-styleguide/src/vars'; | |||
There was a problem hiding this comment.
| @import '~@openmrs/esm-styleguide/src/vars'; | |
| @use '@openmrs/esm-styleguide/src/vars'; |
Requires vars.$ui-02, etc., but slightly friendlier to the Sass compiler.
There was a problem hiding this comment.
I didn't know about this! Does it compile faster, or result in better output, or both?
@denniskigen should we add something about this to the docs, if we have docs on SCSS/style?
There was a problem hiding this comment.
Theoretically, it compiles faster and produces more predictable output. However, the strongest reason is that the Sass team has said they're planning on deprecating @import altogether. See here for more. (I say theoretically, because the vars file itself doesn't really add any bloat, but @import parts of Carbon definitely would).
| export const configSchema: ConfigSchema = { | ||
| wardPatientCards: { | ||
| _description: 'Configure the display of ward patient cards', | ||
| bentoElementDefinitions: { |
There was a problem hiding this comment.
What is a bentoElementDefinition? I'm not entirely sure this nomenclature is clear.
There was a problem hiding this comment.
I think something like patientCardElement might be clearer.
There was a problem hiding this comment.
Yeah, it's the individual elements that can show up within a patient card row (like bed number, patient name, patiant age, patient address). I'll change it to PatientCardRowElement.
There was a problem hiding this comment.
I see where you're coming from with PatientCardRowElement @chibongho but unfortunately I think the formal logic of the name does not align with intuition—I think it is very easy to read PatientCardRowElement and think the element is supposed to be a row. I think @mseaton 's suggestion of patientCardElement is clearest.
There was a problem hiding this comment.
So this would be patientCardElementDefinitions
| export interface WardPatientCardDefinition { | ||
| card: { | ||
| id: string; | ||
| header: Array<string>; // an array of (either built-in or custom) bento element ids |
There was a problem hiding this comment.
Convert this into a doc string (/** */)
| header: Array<string>; // an array of (either built-in or custom) bento element ids | ||
| }; | ||
| appliedTo?: Array<{ | ||
| location: string; // locationUuid. If given, only applies to patients at the specified ward locations. (If not provided, applies to all locations) |
There was a problem hiding this comment.
This would also be useful as a docstring.
| {t('yearsOld', '{{age}} yrs', { | ||
| age: patient?.person?.age, | ||
| })} |
There was a problem hiding this comment.
I'd probably suggest using esm-framework's age() function here. That handles years, months, weeks, and days (may need some extension for hours).
There was a problem hiding this comment.
The backend isn't returning the birthdate either. I'll add a TODO.
There was a problem hiding this comment.
There is this PR. May be we push for its review
There was a problem hiding this comment.
Ah... I hadn't looked into the backend, but it looks like the fields returned from patient are hard-coded. Here's a ticket to adjust that: https://openmrs.atlassian.net/browse/BED-10.
|
|
||
| const WardPatientName: WardPatientCardBentoElement = ({ patient }) => { | ||
| // TODO: make server return patient.display and use that instead | ||
| const { givenName, familyName } = patient?.person?.preferredName; |
There was a problem hiding this comment.
Better to use patient?.person?.display (patient?.display has the primary identifier as well).
There was a problem hiding this comment.
Also there is likely a utility method to get a patient name (or should be) for display. There may be active work ongoing around this, but the way a patient name is formatted is configurable on the backend, and should likely be respected.
There was a problem hiding this comment.
There is a displayName function from esm-framework, but it takes in a FHIR patient instead of an OpenMRS Patient. I'll use patient?.person?.display for now.
There was a problem hiding this comment.
Actually... the REST endpoint we are using (/adminLocation) does not return patient.person.display even with v=full. I'll keep this (and the TODO) for now.
There was a problem hiding this comment.
patient?.person?.display, when it exists, is already formatted according to the backend name template. We sort of need a utility function for FHIR, because the display name is (in FHIRPath syntax) Patient.name.where(use='usual').text.first(), but we have a fallback if the text element is not defined which happens in cases where we've constructed a pseudo-FHIR patient out of the results of the REST API.
| <span className={admittedPatientHeaderStyles.bedNumber}>{bed.bedNumber}</span> | ||
| <span className={`${wardPatientCardStyles.wardPatientBedNumber} ${wardPatientCardStyles.empty}`}> | ||
| {bed.bedNumber} | ||
| </span> | ||
| <p className={styles.emptyBed}>Empty Bed</p> |
| import styles from './occupied-bed.scss'; | ||
| import { Tag } from '@carbon/react'; | ||
| import { useTranslation } from 'react-i18next'; | ||
| import WardPatientCard from '../ward-patient-card/ward-pateint-card.component'; |
There was a problem hiding this comment.
ward-pateint-card.component is misspelled
| const last = index === patients.length - 1; | ||
| return ( | ||
| <div key={patient.uuid + ' ' + index}> | ||
| <WardPatientCard patient={patient} bed={bed} status={'admitted'} /> |
There was a problem hiding this comment.
An occupied bed != admitted, it just means the bed is occupied. Admission is a separate, orthongonal concern.
|
|
||
| export interface ConfigObject {} | ||
| export const defaultBentoElementConfig: BentoElementConfig = { |
There was a problem hiding this comment.
I don't understand why this is and why it has addressFields in it.
There was a problem hiding this comment.
It's supposed to be a common config object for various types of bentoElements. (See comment below)
| _description: 'The unique identifier for this card definition. Currently unused, but that might change.', | ||
| }, | ||
| card: { | ||
| header: { |
There was a problem hiding this comment.
Per our discussion, I'm still of the belief that a card should be made up of 1-N sections, which contain 1-N elements. And these sections could have a type associated with them, or otherwise be configured to indicate if they should appear up in particular views (eg. in a minimal view that is used in the Admission Request workspace, or in a maximal view used in the ward cards.
There was a problem hiding this comment.
I think your suggestion of having every Patient Card Rows be modeled as a "bento box", even with rows that don't fit neatly within that paradigm as a bento box with one element (possibly an extension), is worth exploring. I've tried modeling this by having different row types and wasn't that satisfied with how complex it was.
There was a problem hiding this comment.
@mseaton Yeah @chibongho and I had a nice discussion about this yesterday and arrived at this design. The card has a header row, with name, age, etc, and a bunch of define-in-config elements. It also has a footer row that just has an array of define-in-config elements. The rows in the middle are I think a very good use of the extension system. Using the extension system for this has the following advantages:
- Simplifies the card config schema. Having rows as define-in-config elements that each contain their own define-in-config elements would make this far more complex than any existing schema. Even in the registration app, which has define-in-config sections containing define-in-config fields, there are not multiple types of sections with different valid types of configuration, as would be required for this.
- Allows new row types to be defined in other modules. If some implementation comes along with some very specific requirement for a row that should appear in the cards, which is very different than any existing row, they can add that extension to a module that they own and plug it in using configuration.
It allows exactly the same amount of flexibility and configurability.
| config?: BentoElementConfig; | ||
| }; | ||
|
|
||
| export interface PatientAddressBentoElementConfig { |
There was a problem hiding this comment.
This is what I would expect to see, not to have this at the top level
| export type WardPatientCardRow = React.FC<WardPatientCardProps>; | ||
| export type WardPatientCardBentoElement = React.FC<WardPatientCardProps>; | ||
|
|
||
| export type WardPatientStatus = 'admitted' | 'pending'; |
There was a problem hiding this comment.
Patients might be in a bed but not admitted.
| {t('yearsOld', '{{age}} yrs', { | ||
| age: patient?.person?.age, | ||
| })} |
|
|
||
| const WardPatientName: WardPatientCardBentoElement = ({ patient }) => { | ||
| // TODO: make server return patient.display and use that instead | ||
| const { givenName, familyName } = patient?.person?.preferredName; |
There was a problem hiding this comment.
Also there is likely a utility method to get a patient name (or should be) for display. There may be active work ongoing around this, but the way a patient name is formatted is configurable on the backend, and should likely be respected.
packages/esm-ward-app/src/ward-patient-card/ward-pateint-card.component.tsx
Outdated
Show resolved
Hide resolved
|
Updated with PR feedback and changed the configuration to allow for multiple rows. Hopefully that simplifies things. |
There was a problem hiding this comment.
This looks great to me @chibongho , thanks! I'm still a little unsure about the address configuration, but it sounds like it has been given lots of thought and attention, so defer more to the experts on that, and if this is the best practice identified, it works for me.
Requirements
Summary
This PR introduces a configuration system to ward app, as mocked up here.
The configuration models each Ward Patient Card as a list of rows. In the link above, a card has 3 rows: the header, middle section (baby info), and footer. On other pages in the mock up, there can be other types of rows as well. (For example, here some cards can have extra rows.) The patient's card (and sometimes only its header) can also appear in different contexts, either fully (in the workspace) or patially with the header (also in the workspace).
The most common type of row is the "Bento Box" row, which can contain 1 or more elements displaying patient attributes. In the first link, the header, baby info, and footer rows should theoretically all be modeled as a bento box row.
The configuration system defines a card as having the header row, and any other additional row can be slotted in as extension. The header row gets special treatment (instead of it being just another extension) as it can sometimes be rendered (with the other rows) in different contexts.
In this PR, 4 types of bento elements (bed number, patient name, patient age, patient address) are defined and can be configured to appear in any bento boxes. In future PRs, more bento elements will be defined to support other elements defined in the mockups.
Screenshots
Related Issue
Other