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

fix(VDatePicker): select same date as start/end in date range picker #19990

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

bulatgab
Copy link

fixes 19989

Description

fixes #19989

Markup:

<template>
  <v-app>
    <v-container>
      <v-date-picker v-model="dates" multiple="range" />
      Selected dates: {{ dates }}
    </v-container>
  </v-app>
</template>

<script setup>
  import { ref } from 'vue'

  const dates = ref([new Date('2024-06-11'), new Date('2024-06-11')])
</script>

@johnleider
Copy link
Member

How do you clear a value then?

@bulatgab
Copy link
Author

Thank you for your question!

In my understanding, if the user clicks on the wrong first date, they can click on any other date to complete the range selection, and then click again to select the right first date. This UX seems intuitive enough.

If we are talking about clearing the input completely - yes, it's not possible. But it's not possible for a single date picker either (:multiple="false"). And it makes sense - after all, we show the form to get the user's input and we are probably not interested in an empty value. (And if the developer is interested in this functionality, they can implement a custom 'clear' icon to clear the state)

@johnleider
Copy link
Member

Thank you for your question!

In my understanding, if the user clicks on the wrong first date, they can click on any other date to complete the range selection, and then click again to select the right first date. This UX seems intuitive enough.

If we are talking about clearing the input completely - yes, it's not possible. But it's not possible for a single date picker either (:multiple="false"). And it makes sense - after all, we show the form to get the user's input and we are probably not interested in an empty value. (And if the developer is interested in this functionality, they can implement a custom 'clear' icon to clear the state)

Fair arguments. @vuetifyjs/core-team thoughts?

@johnleider johnleider requested a review from a team July 16, 2024 19:22
@johnleider
Copy link
Member

and we are probably not interested in an empty value. (And if the developer is interested in this functionality, they can implement a custom 'clear' icon to clear the state)

We still run into the case that people more than likely already have applications that depend on or expect this behavior, so we can't simply remove it, especially in a patch.

@RagingRoosevelt
Copy link

We still run into the case that people more than likely already have applications that depend on or expect this behavior, so we can't simply remove it, especially in a patch.

Counterpoint is that people upgrading from Vuetify2 expect the original

@MajesticPotatoe MajesticPotatoe added T: bug Functionality that does not work as intended/expected C: VDatePicker VDatePicker labels Aug 1, 2024
@bulatgab
Copy link
Author

bulatgab commented Aug 7, 2024

For the context, other libraries also have the proposed behavior in their date-range pickers:
https://primevue.org/datepicker/#range
https://vue3datepicker.com/props/modes/#range
https://quasar.dev/vue-components/date#range-selection
(although in Quasar, the output format is different for the same-date range compared to the different-date range. But that's another story. The main point is that it allows you to select the same date).

@bulatgab
Copy link
Author

bulatgab commented Aug 7, 2024

The range with the same start/end dates is a valid date range and not a bizarre edge case. And so it must be covered by the date selector out of the box.

We can think of the date range as a time period that starts at 12:00 AM, startDate and finishes at 11:59 PM, endDate.
The current behaviour allows you to select a minimal period of 48h only.

But what about a 24h period?

You can imagine a use case for booking something, e.g. a venue for an event. How can you choose to book it for one day only?


@bulatgab
Copy link
Author

bulatgab commented Aug 7, 2024

If clearing the state is critical, it could also be done when the user clicks on the existing range selection (the same approach as in Quasar). Or a dedicated “x” icon can be added for clearing.
I suggest this be done in a separate PR though.

@KaelWD KaelWD force-pushed the master branch 2 times, most recently from e20cfec to 2766105 Compare August 15, 2024 09:17
@KaelWD KaelWD force-pushed the master branch 3 times, most recently from 4c970f9 to 6a3285f Compare September 3, 2024 18:11
@bulatgab
Copy link
Author

bulatgab commented Oct 5, 2024

We still run into the case that people more than likely already have applications that depend on or expect this behavior, so we can't simply remove it, especially in a patch.

I have added an allowOneDayRange prop for backward compatibility 🎉
The new behaviour is enabled only if the prop is set to true.

@bulatgab
Copy link
Author

bulatgab commented Oct 5, 2024

Problem

Unfortunately, there is another problem. modelValue becomes [startDate] after the first click already, and doesn't change after the second click. This is to comply with the existing contract for modelValue, which is a list of dates in the selected range, so for the same-day date range, modelValue should be a one-item array.

But in the parent component, we might want to do something when the user completes the range selection. When we receive [startDate] value, we cannot know if that is the whole range (the same day as start/end) or an incomplete range.

Proposal

Ideally, we could change when we emit update:modelValue.
We could avoid emitting it when only the startDate is selected (after all, the range selection is not yet complete then).
And emit it only when both startDate and endDate get selected.
Would this be acceptable?

Alternative proposal

Alternatively, we could emit an additional event on the second click, to indicate range selection completion.

  • name it update:range?
  • the payload could be:
    • empty (we can read the modelValue instead),
    • the same as for update:modelValue: [startDate, ..., endDate],
    • or [startDate, endDate] - another convenient format for date ranges, although it kind of increases the api surface area.

@johnleider
Copy link
Member

I like the alternate proposal.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C: VDatePicker VDatePicker T: bug Functionality that does not work as intended/expected
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug Report][3.6.8] Date picker with range: Can't select the same date as start/end
4 participants