-
Notifications
You must be signed in to change notification settings - Fork 92
feat(ocrvs-10351): AGE field #10719
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(ocrvs-10351): AGE field #10719
Conversation
|
Oops! Looks like you forgot to update the changelog. When updating CHANGELOG.md, please consider the following:
|
| export const getAllUniqueFields = (eventConfig: EventConfig) => { | ||
| return uniqBy(getDeclarationFields(eventConfig), (field) => field.id) | ||
| } | ||
|
|
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.
Was a duplicate function
| inPast: () => defineFormConditional(getDayRange(days, 'before')), | ||
| inFuture: () => defineFormConditional(getDayRange(-days, 'before')) |
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.
inPast should have used -days and vice versa
| {label && ( | ||
| <InputLabel | ||
| id={`${id}_label`} | ||
| htmlFor={id} |
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 should allow searching for input fields via label text in tests
| // This was previously cast on FormSectionComponent level. | ||
| // eslint-disable-next-line @typescript-eslint/no-explicit-any | ||
| errors={formikProps.errors as any} | ||
| errors={formikProps.errors} |
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.
Nice 👍
|
|
||
| export const AgeField = { | ||
| Input: AgeInput, | ||
| Output: ({ value }: { value?: AgeValue }) => value?.age ?? '' |
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.
Should the <Output/> simply display the age, or also the asOfDate?
E.g. 'Age: 20 years (on 14th Oct 2025)'
I guess it could also be configurable how the Output should look like. In any case, it doesn't need to be part of this PR if it's not needed yet.
|
Nice work 💯 Left a couple of thoughts |
cibelius
left a comment
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.
I would also allow prefix, as some languages might have the 'years' before the number
fe4d9eb to
fbf997b
Compare
|
Your environment is deployed to https://ocrvs-10351.e2e-k8s.opencrvs.dev |
Description
AGEwhich stores both the input value and a reference date as{ age: number; asOfDate: string }asAgeandasDobcountryconfig: opencrvs/opencrvs-countryconfig#1089
Checklist