-
Notifications
You must be signed in to change notification settings - Fork 4
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: support min & max date, DD-MM-YYYY date format #36
Conversation
Co-authored-by: @kabaros
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.
looks good to me .. pre-approving with some comments - the most important are to add tests for nepali
and ethiopic
since these follow a different path.
Since this is a PR to alpha, feel free to merge them (to test with the UI component) and handle them separately in a followup PR.
src/hooks/useDatePicker.spec.tsx
Outdated
// minDate: '2015-06-28', | ||
// calendar: 'iso8601' as SupportedCalendar, |
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.
remove commented out code please
src/hooks/useDatePicker.spec.tsx
Outdated
const result = renderedHook.result?.current as UseDatePickerReturn | ||
|
||
expect(result.isValid).toEqual(false) | ||
}) |
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.
could you also add similar tests for ethiopic and nepali calendars please
src/types.ts
Outdated
@@ -16,3 +16,8 @@ export type ResolvedLocaleOptions = { | |||
numberingSystem: string | |||
weekDayFormat: WeekDayFormat | |||
} | |||
|
|||
export type ValidationOptions = { |
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 type doesn't seem to be used anywhere, so can be removed
@@ -1 +1,2 @@ | |||
export const dateStringRegExp = /^(\d{4})([-./])(\d{2})(\2)(\d{2})$/ | |||
export const dateStringRegExp = |
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.
can you add a comment clarifying the regex (that it matches dd-...
and yy-...
)
const [, yearStr, , monthStr, , dayStr] = parts | ||
let yearStr, monthStr, dayStr, format | ||
|
||
if (parts[1]) { |
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.
what does this check exactly? what is parts[1]
in each case
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.
@kabaros I initially wrote it like this, but it caused issues with Prettier, it adds ;
at the beginning of these lines.
[, yearStr, , monthStr, , dayStr] = parts;
[, , , , , , dayStr, , monthStr, , yearStr] = parts;
Parts[1] equal to this part (\d{4}), which means the format provided starts with 4 numbers.
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.
ok .. let's have it like this for now. We can see if it can be refactored later.
monthStr = parts[3] | ||
dayStr = parts[5] | ||
format = 'YYYY-MM-DD' | ||
} else { |
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.
can we also have a test (from the hook) to validate the effect of passing a completely different format (something like 2222-2222-2222
for example.
I see you have logic to fall back to today's date .. so you can have something like this to ensure the test is not dependent on the actual date
}) | ||
}) | ||
|
||
// @todo: add tests for warning scenarios |
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.
add these tests please
# [1.0.0-alpha.26](v1.0.0-alpha.25...v1.0.0-alpha.26) (2024-06-09) ### Features * support min & max date, DD-MM-YYYY date format ([#36](#36)) ([3b2e57e](3b2e57e))
🎉 This PR is included in version 1.0.0-alpha.26 🎉 The release is available on: Your semantic-release bot 📦🚀 |
# [1.3.0-alpha.1](v1.2.4...v1.3.0-alpha.1) (2024-08-28) ### Bug Fixes * format undefined when thers is a mismatch ([79b54fd](79b54fd)) * remove unnecessary console warn ([b2d795d](b2d795d)) ### Features * expose function to convert dates ([622f0eb](622f0eb)) * expose function to convert dates ([9d42640](9d42640)) * expose methods for converting between iso8601 and calendars ([#37](#37)) ([33fa25d](33fa25d)) * reararnge memorized variables so that we only recompute them when necessary ([b754dcc](b754dcc)) * support min & max date, DD-MM-YYYY date format ([#36](#36)) ([3b2e57e](3b2e57e)) * **validation:** add and expose validateDateString utility ([cb4c919](cb4c919))
🎉 This PR is included in version 1.3.0-alpha.1 🎉 The release is available on: Your semantic-release bot 📦🚀 |
# [1.3.0](v1.2.4...v1.3.0) (2024-09-24) ### Bug Fixes * format undefined when thers is a mismatch ([79b54fd](79b54fd)) * localise validation message in multi calendar ([#61](#61)) ([ead860b](ead860b)) * remove unnecessary console warn ([b2d795d](b2d795d)) ### Features * expose function to convert dates ([622f0eb](622f0eb)) * expose function to convert dates ([9d42640](9d42640)) * expose methods for converting between iso8601 and calendars ([#37](#37)) ([33fa25d](33fa25d)) * reararnge memorized variables so that we only recompute them when necessary ([b754dcc](b754dcc)) * support min & max date, DD-MM-YYYY date format ([#36](#36)) ([3b2e57e](3b2e57e)) * **validation:** add and expose validateDateString utility ([cb4c919](cb4c919))
🎉 This PR is included in version 1.3.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Support the implementation of LIBS-418 & DHIS2-17520.
Includes:
yyyy-mm-dd
&dd-mm-yyyy