-
Notifications
You must be signed in to change notification settings - Fork 203
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
(refactor) Refactor Mark patient deceased form and Mark patient alive modal #1872
base: main
Are you sure you want to change the base?
Conversation
Size Change: -5.49 kB (-0.05%) Total Size: 11.2 MB ℹ️ View Unchanged
|
37cefb0
to
ef236f4
Compare
@@ -17,7 +17,7 @@ | |||
"test:watch": "cross-env TZ=UTC jest --watch --config jest.config.js --color", | |||
"coverage": "yarn test --coverage", | |||
"typescript": "tsc", | |||
"extract-translations": "i18next 'src/**/*.component.tsx' 'src/**/*.hook.tsx' 'src/index.ts' --config ../../tools/i18next-parser.config.js" | |||
"extract-translations": "i18next 'src/**/*.component.tsx' 'src/**/*.workspace.tsx' 'src/**/*.hook.tsx' 'src/index.ts' --config ../../tools/i18next-parser.config.js" |
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.
This just makes it possible to extract translation keys and strings from *.workspace.tsx
files.
@@ -2,29 +2,28 @@ import React, { useCallback } from 'react'; | |||
import { OverflowMenuItem } from '@carbon/react'; | |||
import { useTranslation } from 'react-i18next'; | |||
import { showModal } from '@openmrs/esm-framework'; | |||
import { usePatientDeceased } from '../deceased/deceased.resource'; | |||
import { usePatientDeceasedStatus } from '../mark-patient-deceased/deceased.resource'; |
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.
Renamed for clarity.
}); | ||
}, [patientUuid]); | ||
|
||
return ( | ||
!isPatientLoading && | ||
isDead && ( | ||
<OverflowMenuItem | ||
itemText={t('markAlive', 'Mark alive')} | ||
itemText={t('markPatientAlive', 'Mark patient alive')} |
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.
The key change here.
|
||
return ( | ||
!isPatientLoading && | ||
!isDead && ( | ||
<OverflowMenuItem | ||
itemText={t('markDeceased', 'Mark deceased')} | ||
itemText={t('markPatientDeceased', 'Mark patient deceased')} |
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.
Ditto.
border: 0.0625rem solid $grey-2; | ||
background-color: $ui-01; | ||
|
||
:global(.cds--tile) { | ||
border: none !important; |
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.
This makes it so that the EmptyState
tile doesn't have a double border.
:global(.cds--inline-loading) { | ||
min-height: 1rem; | ||
} | ||
|
||
:global(.cds--inline-loading__text) { | ||
font-size: unset; | ||
} |
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.
This styling supports the submit button's loading spinner state.
"modals": [ | ||
{ | ||
"name": "mark-patient-alive-modal", | ||
"component": "markPatientAliveModal" | ||
} | ||
], | ||
"pages": [ | ||
{ | ||
"component": "root", | ||
"routeRegex": "^patient\/.+\/chart", | ||
"online": true, | ||
"offline": true | ||
} | ||
], | ||
"workspaces": [ | ||
{ | ||
"name": "mark-patient-deceased-workspace-form", | ||
"component": "markPatientDeceasedForm", | ||
"title": "Mark patient deceased", | ||
"type": "form" | ||
} |
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.
@brandones the hints you added to the console about porting over modals and workspaces to use their respective corresponding keys in the routes.json
file came in clutch! Thanks!
markPatientDeceased(deathDate, patientUuid, causeOfDeath) | ||
.then(() => { | ||
closeWorkspace(); | ||
window.location.reload(); |
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.
@ibacher, @brandones is there a better revalidation mechanism for the patient object? fetchCurrentPatient doesn't use SWR so I assume calling mutate with that key won't work. Triggering a full page reload here ensures that the patient's deceased status gets reflected in the Patient Header.
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.
Additionally, I've elected to not show a success notification here as it wouldn't stay on the screen long enough before the page reloads. Should I show it anyway and maybe set an artificial delay to communicate to the user explicitly that marking the patient as deceased succeeded?
)} | ||
<div className={styles.container}> | ||
<span className={styles.warningContainer}> | ||
<WarningFilled aria-label={t('warning', 'Warning')} className={styles.warningIcon} size={20} /> |
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.
@ibacher should we switch to the new icons in the framework across the board?
{causesOfDeath?.length > 10 ? ( | ||
<ResponsiveWrapper> | ||
<Search | ||
onChange={(event) => setSearchTerm(event.target.value)} | ||
placeholder={t('searchForCauseOfDeath', 'Search for a cause of death')} | ||
labelText="" | ||
/> | ||
</ResponsiveWrapper> | ||
) : 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.
This change is very arbitrary. On dev3, we currently have 6 cause-of-death concept answers configured. In practice, would we expect implementations to configure a lot more through this form? Or would most implementers elect to use a custom form for Marking patients as deceased? At AMPATH, we used a custom clinical form for this purpose, so I imagine most will be doing something similar. Should I default to showing the Search input anyway, regardless of the number of options in the list?
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.
This component rendered a confirmation modal after clicking the Save and close
button in the Mark patient deceased form:
I deleted it because I felt that the intent of submitting the form meant that the user must have been sure they wanted to mark a patient deceased. Do you agree? Should we explicitly require the user to confirm that they want to mark a patient deceased after submitting the form?
Requirements
Summary
This PR makes the following refactors to the
Mark patient deceased form
andMark patient alive
modal:Mark patient deceased form
Mark patient alive modal
Screenshots
demo.mp4
Related Issue
https://openmrs.atlassian.net/browse/O3-3452
Other