(feat) O3-4430: Make the indications field configurable in the drug order form #2228
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Size Change: +292 B (0%) Total Size: 16.3 MB ℹ️ View Unchanged
|
vasharma05
changed the title
ibacher
requested changes
Feb 4, 2025
| ) : null} | ||
| <span> | ||
| <span className={styles.label01}>{t('indication', 'Indication').toUpperCase()}</span>{' '} | ||
| {medication?.order?.orderReasonNonCoded ?? t('none', 'None')} |
There was a problem hiding this comment.
Suggested change
| {medication?.order?.orderReasonNonCoded ?? t('none', 'None')} | |
| {medication?.order?.orderReasonNonCoded ?? t('noIndication', 'No indication provided')} |
There was a problem hiding this comment.
In general, be careful when placing text in front of users indicating the absence of data.
| @@ -36,6 +36,11 @@ export const configSchema = { | |||
| 'Number of milliseconds to delay the search operation in the drug search input by after the user starts typing. The useDebounce hook delays the search by 300ms by default', | |||
| _default: 300, | |||
| }, | |||
| isIndicationFieldOptional: { | |||
There was a problem hiding this comment.
Suggested change
| isIndicationFieldOptional: { | |
| requireIndication: { |
There was a problem hiding this comment.
+1 for requireIndication: { _default: true }.
Positive phrasing is easier to grok (implies "we require this by default") as opposed to the double negative isIndicationFieldOptional: { _default: false } (implies "it's optional by default").
The validation logic reads better as well:
// Current validation logic
if (!isIndicationFieldOptional) { /* require indication */ }
// With requireIndication
if (requireIndication) { /* require indication */ }| isIndicationFieldOptional: { | ||
| _type: Type.Boolean, | ||
| _description: 'Whether to make the indication field optional in the drug order form', | ||
| _default: false, |
There was a problem hiding this comment.
Suggested change
| _default: false, | |
| _default: true, |
Comment on lines
49
to
52
| <span> | ||
| <span className={styles.label01}>{t('indication', 'Indication').toUpperCase()}</span>{' '} | ||
| {medication.orderReasonNonCoded ?? t('none', 'None')} | ||
| </span> |
There was a problem hiding this comment.
I actually think the previous implementation here is better. Can we reverse this please?
There was a problem hiding this comment.
Actually the quantity text right after the Indication text has a &mdash, hence when we don't have indication, the row looks like: - QUANTITY 5 tablets, hence I needed to put a fallback value for the missing indication.
There was a problem hiding this comment.
I see. I think I'd prefer just tracking whether or not we need to render the em-dash.
There was a problem hiding this comment.
The rule I'm going for is consistency: we should either render a "placeholder" for every field without data or, if, as we've done, we only selectively render fields when the convey some piece of information, we should do so consistently here.
| @@ -207,8 +207,9 @@ const OrderDetailsTable: React.FC<OrderDetailsProps> = ({ patientUuid, showAddBu | |||
| dosage: | |||
| order.type === 'drugorder' ? ( | |||
| <div className={styles.singleLineText}>{`${t('indication', 'Indication').toUpperCase()} | |||
| ${order.orderReasonNonCoded} ${'-'} ${t('quantity', 'Quantity').toUpperCase()} ${order.quantity} ${order | |||
| ?.quantityUnits?.display} `}</div> | |||
| ${order.orderReasonNonCoded ?? t('none', 'None')} ${'-'} ${t('quantity', 'Quantity').toUpperCase()} ${ | |||
denniskigen
reviewed
Feb 6, 2025
packages/esm-patient-medications-app/src/add-drug-order/drug-order-form.component.tsx
Show resolved
Hide resolved
| @@ -36,6 +36,11 @@ export const configSchema = { | |||
| 'Number of milliseconds to delay the search operation in the drug search input by after the user starts typing. The useDebounce hook delays the search by 300ms by default', | |||
| _default: 300, | |||
| }, | |||
| isIndicationFieldOptional: { | |||
There was a problem hiding this comment.
+1 for requireIndication: { _default: true }.
Positive phrasing is easier to grok (implies "we require this by default") as opposed to the double negative isIndicationFieldOptional: { _default: false } (implies "it's optional by default").
The validation logic reads better as well:
// Current validation logic
if (!isIndicationFieldOptional) { /* require indication */ }
// With requireIndication
if (requireIndication) { /* require indication */ }
Co-authored-by: Ian <52504170+ibacher@users.noreply.github.com>
vasharma05
force-pushed
the
feat/configurable-indication-field
branch
from
7ed27d0 to
0062335
Compare
denniskigen
changed the title
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Requirements
Summary
This PR allows configuring the indication field as optional when saving the drug order form
Screenshots
Related Issue
https://openmrs.atlassian.net/issues/O3-4430
Other