Skip to content
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

refactor: Added support for string-based property values for date based components #1470

Merged
merged 9 commits into from
Nov 19, 2024

Conversation

rkaraivanov
Copy link
Member

This commit adds support for passing in string based date values to date-bound properties of calendar, date-time input and date picker components. The component will now try to parse the passed in value without directly throwing an exception.
Unified the type declarations between date-bound component.

Closes #1467

…ed components

This commit adds support for passing in string based date values
to date-bound properties of calendar, date-time input and date picker components.
The component will now try to parse the passed in value without directly
throwing an exception.
Unified the type declarations between date-bound component.
src/components/calendar/calendar.interaction.spec.ts Outdated Show resolved Hide resolved
src/components/calendar/types.ts Outdated Show resolved Hide resolved
src/components/calendar/helpers.ts Outdated Show resolved Hide resolved
src/components/calendar/helpers.ts Outdated Show resolved Hide resolved
src/components/date-picker/date-picker.ts Outdated Show resolved Hide resolved
Added tests for date-time input. The date utilities
class uses the new parse ISO implementation which
returns `null` instead of `Invalid date` for certain
time portions. Adjusted the tests accordingly.
This means that the date-time input additional checks around
the mask value logic could be simplified but that is for
another PR.
Copy link
Member

@damyanpetev damyanpetev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should probably plan to revise the date utils down the line, now that some of the parsing is shared with the calendar. Other than that, LGTM :)

@damyanpetev
Copy link
Member

@Marina-L-Stoyanova Just to check - have you tested this change in some more complete scenario than the repo sample for the issue? As in some of the AB form scenarios - asking specifically because we discussed the code produced for nullable variables or request-bound would produce optional values, see #1470 (comment) and with the latest version those would result in type check errors:
image
Those are visible with a VS Code Lit plugin or lit eslint plugin and strict checks and this PR should clear those up, but I wanted to verify that.

@Marina-L-Stoyanova
Copy link

@Marina-L-Stoyanova Just to check - have you tested this change in some more complete scenario than the repo sample for the issue? As in some of the AB form scenarios - asking specifically because we discussed the code produced for nullable variables or request-bound would produce optional values, see #1470 (comment) and with the latest version those would result in type check errors: image Those are visible with a VS Code Lit plugin or lit eslint plugin and strict checks and this PR should clear those up, but I wanted to verify that.

Yes, everything looks ok. I have tested it with form application that has date picker inputs. I installed the suggested plugin and it is working as expected.

@damyanpetev damyanpetev merged commit 1fa3976 into master Nov 19, 2024
5 checks passed
@damyanpetev damyanpetev deleted the rkaraivanov/fix-1467 branch November 19, 2024 11:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: DatePicker throws error when the data is string
3 participants