Skip to content

feat(VDatePicker): disable months and years if not allowed #21466

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

Merged
merged 10 commits into from
May 28, 2025

Conversation

ikushum
Copy link
Member

@ikushum ikushum commented May 22, 2025

fixes #20465

Description

Disable month and year buttons based on allowed-dates prop when:

  • allowed-dates is array
  • allowed-dates is function (open for suggestion/help)

Markup:

<template>
  <v-app>
    <v-container>
        <h2>Date picker</h2>
        <v-date-picker
          mode="month"
          hide-header
          :allowed-dates="['2025-05-01', '2025-03-02', '2024-02-01']"
        ></v-date-picker>

        <h2>Date picker years</h2>
        <VDatePickerYears
          :allowed-years="[2025, 2026]"
          class="mb-10"
        />

        <h2>Date picker months</h2>
        <VDatePickerMonths
          :allowed-months="[2, 3]"
        />
    </v-container>
  </v-app>
</template>

Screenshots

image
image

@ikushum ikushum requested review from johnleider and J-Sek May 22, 2025 09:20
@ikushum ikushum self-assigned this May 22, 2025
@ikushum ikushum changed the title feat(VDatePicker): disable months and years not allowed feat(VDatePicker): disable months and years if not allowed May 22, 2025
@ikushum
Copy link
Member Author

ikushum commented May 22, 2025

As mentioned in the PR description, this works only when allowed-dates is an array.

I'm open for suggestion on how we can implement this (without heavy computation) for cases when allowed-dates is a function.

@J-Sek
Copy link
Contributor

J-Sek commented May 22, 2025

By "heavy computation" you probably meant those years.

// check if month is accepted by allowedDates
const firstDay = adapter.parseISO(`${year.value}-${month}-01`)
const monthSize = adapter.diff(
  adapter.endOfDay(adapter.endOfMonth(firstDay),
  adapter.startOfDay(firstDay)
)
return createRange(monthSize)
  .find((i) => props.allowedDates(adapter.addDays(firstDay, i)))
// up to 31 executions per month in the grid -- OK'ish
// check if year is accepted by allowedDates
const firstDay = adapter.parseISO(`${year.value}-01-01`)
const yearSize = adapter.diff(
  adapter.endOfDay(adapter.endOfYear(firstDay),
  adapter.startOfDay(firstDay)
)
return createRange(yearSize)
  .find((i) => props.allowedDates(adapter.addDays(firstDay, i)))
// up to 366 executions per year in the grid --- hm...

Let's consider real life scenarios:

  • a birth date picker - will have all years but filtering specific dates makes no sense
  • meeting date picker - could be forced to use function to cut off the weekends. It makes no sense without min and max, but we would rely on devs to remember. However executions per year will be in range 1-3, so the overhead is minimal

I would vote for including it and wait for feedback. We might first and foremost learn about scenarios that suffer from performance hit - learning about those is a benefit for the community and might exceed the risk of upsetting devs. Once we get some signals, we'd have couple of options to choose from:

  • detect 10+ years in the grid and batch + throttle execution (e.g. 1000 per second)
  • cache array of enabled years (expecting allowedDays function to be pure and rather static)
  • change allowedDays function signature to take year, month, day instead of date and expect and object back (e.g. { year: true, month: false } that would let us drop from the loop earlier)
    • could be shipped in v4.x as breaking change

@ikushum
Copy link
Member Author

ikushum commented May 22, 2025

@J-Sek, For the computation heavy changes, I created a new PR that targets this branch

Copy link
Member

@johnleider johnleider left a comment

Choose a reason for hiding this comment

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

Looks good

@ikushum ikushum requested a review from johnleider May 22, 2025 19:57
@ikushum ikushum requested a review from johnleider May 23, 2025 03:38
@KaelWD
Copy link
Member

KaelWD commented May 23, 2025

up to 31 executions per month in the grid

For some reason this is calling allowedDates 2061 times instead of just 365

up to 366 executions per year in the grid

×152 years is over 55000 calls, should we only do this if min and max are explicitly set?

KaelWD added 3 commits May 23, 2025 16:05
both branches of allowedMonths already check the year
@ikushum
Copy link
Member Author

ikushum commented May 23, 2025

up to 31 executions per month in the grid

For some reason this is calling allowedDates 2061 times instead of just 365

up to 366 executions per year in the grid

×152 years is over 55000 calls, should we only do this if min and max are explicitly set?

@KaelWD

I just tested for worst case senario and allowedMonths makes 353 calls as expected.

image.

Maybe you mixed it up with calls from allowedYears too?

@KaelWD
Copy link
Member

KaelWD commented May 23, 2025

My last commit fixed it.

Maybe you mixed it up with calls from allowedYears too?

Well yeah it doesn't matter how many times a specific function calls it directly, only how many times it is invoked overall.

@ikushum
Copy link
Member Author

ikushum commented May 23, 2025

×152 years is over 55000 calls, should we only do this if min and max are explicitly set?

Sounds fair, should we mention this somewhere in the documentation?

Copy link
Member

@johnleider johnleider left a comment

Choose a reason for hiding this comment

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

Approved pending added unit test.

@ikushum ikushum merged commit 3b1c450 into master May 28, 2025
18 checks passed
@ikushum ikushum deleted the feat/disable-year-and-month branch May 28, 2025 14:06
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.

[Feature Request] Date picker shows years which are not valid
4 participants