-
Notifications
You must be signed in to change notification settings - Fork 242
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
(feat) O3-4083 : Standardize lab result ranges: display absolute min/max values to match validation rules #2161
base: main
Are you sure you want to change the base?
Conversation
…max values to match validation rules
Size Change: -25 B (0%) Total Size: 16 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.
Thanks @donaldkibet LGTM
} | ||
return ''; | ||
|
||
return ` (${displayUnit})`; |
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.
return ` (${displayUnit})`; | |
return units ? ` (${displayUnit})` : ''; |
So we don't return a string with parentheses and spaces when there are no ranges and no units.
const { | ||
datatype: { hl7Abbreviation }, | ||
} = concept ?? {}; |
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.
const { | |
datatype: { hl7Abbreviation }, | |
} = concept ?? {}; | |
const hl7Abbreviation = concept?.datatype?.hl7Abbreviation; |
The current destructuring could throw an error if datatype
is null/undefined, even with the nullish coalescing operator.
const hasLowerLimit = lowAbsolute !== null && lowAbsolute !== undefined; | ||
const hasUpperLimit = hiAbsolute !== null && hiAbsolute !== undefined; |
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.
const hasLowerLimit = lowAbsolute !== null && lowAbsolute !== undefined; | |
const hasUpperLimit = hiAbsolute !== null && hiAbsolute !== undefined; | |
const hasLowerLimit = !!lowAbsolute; | |
const hasUpperLimit = !!hiAbsolute; |
if (hasLowerLimit && hasUpperLimit) { | ||
return ` (${lowAbsolute} - ${hiAbsolute}) ${displayUnit}`; | ||
} | ||
|
||
if (hasUpperLimit) { | ||
return ` (<= ${hiAbsolute}) ${displayUnit}`; | ||
} | ||
|
||
if (hasLowerLimit) { | ||
return ` (>= ${lowAbsolute}) ${displayUnit}`; | ||
} |
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.
While not functionally different, this makes the logic a little clearer:
if (hasLowerLimit && hasUpperLimit) { | |
return ` (${lowAbsolute} - ${hiAbsolute}) ${displayUnit}`; | |
} | |
if (hasUpperLimit) { | |
return ` (<= ${hiAbsolute}) ${displayUnit}`; | |
} | |
if (hasLowerLimit) { | |
return ` (>= ${lowAbsolute}) ${displayUnit}`; | |
} | |
if (hasLowerLimit && hasUpperLimit) { | |
return ` (${lowAbsolute} - ${hiAbsolute}) ${displayUnit}`; | |
} else if (hasUpperLimit) { | |
return ` (<= ${hiAbsolute}) ${displayUnit}`; | |
} else if (hasLowerLimit) { | |
return ` (>= ${lowAbsolute}) ${displayUnit}`; | |
} |
@@ -63,7 +84,7 @@ const ResultFormField: React.FC<ResultFormFieldProps> = ({ concept, control, def | |||
hideSteppers | |||
id={concept.uuid} | |||
key={concept.uuid} | |||
label={concept?.display + printValueRange(concept)} | |||
label={concept?.display + formatLabRange(concept)} |
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.
Not an issue created by this PR, but I don't think this works the way we'd want if concept
is actually null or undefined.
label={concept?.display + formatLabRange(concept)} | |
label={`${concept?.display ? concept.display + ' ' : ''}${formatLabRange(concept)}`} |
@@ -99,7 +99,7 @@ describe('LabResultsForm', () => { | |||
const user = userEvent.setup(); | |||
render(<LabResultsForm {...testProps} />); | |||
|
|||
const input = await screen.findByLabelText(`Test Concept (0 - 100 mg/dL)`); | |||
const input = await screen.findByLabelText(`Test Concept (0 - 100) mg/dL`); |
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.
See I actually think the original text here is better than this version, since the numbers should be grouped with the units.
Requirements
Summary
See https://openmrs.atlassian.net/browse/O3-4083
Screenshots
Kapture.2024-12-17.at.15.43.48.webm
Related Issue
Other