-
Notifications
You must be signed in to change notification settings - Fork 232
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
O3-283 Configurable Support for Recording Registration Observations #221
Conversation
Error: Error while trying to collect info after merging reg-obs into main. Error: Command failed: git merge FETCH_HEAD --allow-unrelated-histories at ChildProcess.exithandler (node:child_process:398:12) at ChildProcess.emit (node:events:527:28) at maybeClose (node:internal/child_process:1092:16) at Process.ChildProcess._handle.onexit (node:internal/child_process:302:5) Generated by @jsenv/file-size-impact during Report bundle size#2671913996 on a2f73c1 |
packages/esm-patient-registration-app/src/patient-registration/form-manager.ts
Outdated
Show resolved
Hide resolved
@@ -26,13 +26,13 @@ const PastAppointmentDetails: React.FC<PastAppointmentDetailsProps> = ({ pastApp | |||
{pastAppointments.length >= 1 ? ( | |||
pastAppointments.map((appointment, index) => { | |||
return ( | |||
<> | |||
<React.Fragment key={`past-appointment-${index}`}> |
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.
Fixes warnings about needing unique key
s when creating elements with map
.
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.
Great work @brandones!
Thank you!
@@ -9,14 +9,14 @@ export interface SectionDefinition { | |||
export interface FieldDefinition { | |||
id: string; | |||
type: string; | |||
label: string; | |||
label: string | 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.
We can also use label?: string
instead of using |null
. Both representations denotes that this value can be null.
Just a suggestion.
Thanks!
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.
Optional and "possibly null" are distinct. Optional is more or less the same as | undefined
. In this case, the object will always have the label
property, but it will sometimes be null-valued. See https://stackoverflow.com/a/52893726/1464495
It'd be really helpful if you could share a small video, showing how is this working? |
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.
LGTM
There is one in the PR description. Can you not see it? Or are there other details you're interested in? |
Woohoo!! Way to go Brandon! I can think of at least 3 different orgs who will want to use this right away. Thank you so much. |
Requirements
Summary
This makes it possible to configure the registration app to include obs.
I have also fixed up a few tests which either were being skipped or which, presumably by accident, did not test anything.
Screenshots
Related Issue
https://issues.openmrs.org/browse/O3-283