Skip to content

Conversation

@GVodyanov
Copy link
Contributor

We noticed that some properties that we used of the old datepicker in calendar didn't exist in the new one.

☑️ Resolves

🖼️ Screenshots

Added:
image

image image image

🏁 Checklist

  • ⛑️ Tests are included or are not applicable
  • 📘 Component documentation has been extended, updated or is not applicable
  • 2️⃣ Backport to stable8 for maintained Vue 2 version or not applicable

@GVodyanov GVodyanov requested a review from ShGKme December 15, 2025 14:07
@GVodyanov GVodyanov self-assigned this Dec 15, 2025
@GVodyanov GVodyanov added the 3. to review Waiting for reviews label Dec 15, 2025
@GVodyanov GVodyanov requested a review from GretaD December 15, 2025 14:08
@codecov
Copy link

codecov bot commented Dec 15, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 53.25%. Comparing base (7532c0d) to head (cfba6f9).
⚠️ Report is 8 commits behind head on main.

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.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@ShGKme ShGKme changed the title feat(NcDateTimePicker): add additional props needed for calendar feat(NcDateTimePicker): add range limit props and inline option Dec 15, 2025
@ShGKme ShGKme added this to the 9.4.0 milestone Dec 15, 2025
@ShGKme ShGKme added the enhancement New feature or request label Dec 15, 2025
@GVodyanov GVodyanov force-pushed the feat/date-time-picker/add-more-props branch 2 times, most recently from c91528b to 033d753 Compare December 16, 2025 19:53
@GVodyanov GVodyanov requested a review from ShGKme December 16, 2025 19:54
@GVodyanov
Copy link
Contributor Author

Thanks a lot for the detailed review @ShGKme! Should be good now

@GVodyanov GVodyanov force-pushed the feat/date-time-picker/add-more-props branch from 58d7357 to e344c3a Compare December 19, 2025 18:34
Signed-off-by: Grigory Vodyanov <scratchx@gmx.com>
@GVodyanov GVodyanov force-pushed the feat/date-time-picker/add-more-props branch from e344c3a to b66e274 Compare December 19, 2025 18:41
@GVodyanov GVodyanov requested a review from susnux December 19, 2025 19:05
@GVodyanov
Copy link
Contributor Author

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.

@GVodyanov
Copy link
Contributor Author

Now I also added support for ranges and type="time"

Comment on lines 725 to 726
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,
Copy link
Contributor

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

Suggested change
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,

Copy link
Contributor

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?

Copy link
Contributor Author

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

Copy link
Contributor

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?

Copy link
Contributor Author

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

@GVodyanov GVodyanov requested a review from ShGKme December 22, 2025 09:43
@SebastianKrupinski
Copy link

Hello all, happy holidays.

Just wanted to bump this PR to get the reaming, piece of the migration solved.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

3. to review Waiting for reviews enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants