-
Notifications
You must be signed in to change notification settings - Fork 95
feat(NcDateTimePicker): add range limit props and inline option #8009
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
base: main
Are you sure you want to change the base?
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #8009 +/- ##
=======================================
Coverage 53.25% 53.25%
=======================================
Files 101 101
Lines 3145 3145
Branches 872 871 -1
=======================================
Hits 1675 1675
Misses 1232 1232
Partials 238 238 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
c91528b to
033d753
Compare
|
Thanks a lot for the detailed review @ShGKme! Should be good now |
58d7357 to
e344c3a
Compare
Signed-off-by: Grigory Vodyanov <scratchx@gmx.com>
e344c3a to
b66e274
Compare
|
So for people following along: I made a small change to make the min/max function exactly like the native time picker. With my previous implementation it would apply minTime to every day, so for example you would set minTime to 08:00 and that means that you would not able to select before 08:00 on any given day, now however I made min/max represent a range, so if you set min to 08:00 on Monday and max 18:00 on Wednesday you will be able to select any time in between. |
Signed-off-by: Grigory V <scratchx@gmx.com>
|
Now I also added support for ranges and |
| minTime: props.min && value.value ? sameDay(props.min, value.value as Date) ? minTime.value : undefined : undefined, | ||
| maxTime: props.max && value.value ? sameDay(props.max, value.value as Date) ? maxTime.value : undefined : 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.
To remove nested ternary operator
| minTime: props.min && value.value ? sameDay(props.min, value.value as Date) ? minTime.value : undefined : undefined, | |
| maxTime: props.max && value.value ? sameDay(props.max, value.value as Date) ? maxTime.value : undefined : undefined, | |
| minTime: props.min && value.value && sameDay(props.min, value.value as Date) ? minTime.value : undefined, | |
| maxTime: props.max && value.value && sameDay(props.max, value.value as Date) ? maxTime.value : 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.
Is it guaranteed that value.value is a Date here?
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.
It isn't always Date but it should be for the types I am considering
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.
If it is not always Date here, we cannot use as Date. What happens when it is not Date?
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 I mean is if we are talking about the only time selector then we get a TimeObj, but those as are inside ifs that won't be reached in case of an only time selector
Signed-off-by: Grigory V <scratchx@gmx.com>
|
Hello all, happy holidays. Just wanted to bump this PR to get the reaming, piece of the migration solved. |
We noticed that some properties that we used of the old datepicker in calendar didn't exist in the new one.
☑️ Resolves
🖼️ Screenshots
Added:

🏁 Checklist
stable8for maintained Vue 2 version or not applicable