mymth / vanillajs-datepicker

A vanilla JavaScript remake of bootstrap-datepicker for Bulma and other CSS frameworks
MIT License
744 stars 154 forks source link

Add variant behavior for out of range dates #89

Closed henkesn closed 2 years ago

henkesn commented 2 years ago

The current behavior for setting dates which are not within the given minDate maxDate range is to just ignore them. When the datepicker is not used to pick the date but the date is entered by hand there is no feedback about the wrong date. There is no way to display a warning or hint, either. Additionally the state between input element value and the datepicker date(s) diverge and if the selected date is processed through Datepicker#getDate(), the user enters a date which he/she is not aware of. I'm currently thinking of a softer approach which allows to set invalid dates when entering them using the keyboard. In this case, the minDate and maxDate could prevent dates to be picked using the picker. Additionally there could be a small API which allows to check the state (valid/invalid) of the entered dates like Datepicker#isDateValid() or perhaps an event like dateInvalid. The latter could be used e. g. to change the border color or other. What do you think?

mymth commented 2 years ago

To me, your approach seems not really different from not using datepicker's min/maxDate options and setting up a date range check in your form validation instead. What do you think is the difference between those two?

If we change the datepicker with your approach, we need to rely on users to call isDateValid(). It means the datepicker itself actually doesn't assure the validity of input date. It seems to me that getting rid of the minDate/maxDate options instead would be better if we have to do something because removing such incomplete (thus, questionable) features reduces the program size and also gives a clear message to users: "Application-level date validation needs to be implemented by users."

I'm unclear what you meant by "the state between input element value and the datepicker date(s) diverge". If it's about the difference between the selected date set in the datepicker and the one represented by the string value of the input field in the middle of editing, I believe it's common to most of date picker libraries that allow users to edit the text in the input field. And to update the selection at every key press like bootstrap-datepicker—I feel its way to refresh the calendar is annoying—is probably the only way to avoid it.

henkesn commented 2 years ago

@mymth, the main difference is that if you don't use the minDate/maxDate, you loose the restricted range in the picker dropdown. With the state I meant the following: Image an input field with an existing value. You now start typing without poping up the datepicker, overriding the input with an invalid date. You now call getDate() on the datepicker and you'll get the old value. There is no possibility to intervene and tell the user to fix the date. I'm with you regarding the every key press and I don't think it is needed and I don't like it neither. And right, the isDateValid() or events are just comfort features and it could be a good approach to let the user handle date validation. In conclusion, it would be good to be able to config the one check here https://github.com/mymth/vanillajs-datepicker/blob/master/js/Datepicker.js#L52 to be optional (same for DateRangePicker). You still have the possibility to restrict the picking part, but you can handle all the date validation on your own without needing to rewrite the great parsing logic of the vanillajs-datepicker.

mymth commented 2 years ago

OK, it looks like the availability of the restrictions on the calendar UI is the only difference.

I started this project to create a date picker with consistent behavior in both mouse and keyboard operations and input field that always displays internal data properly (except while user is editing). To satisfy the first one, the dates that user cannot select from the calendar should not be able to enter from the input field too. And if you allow users to enter any dates via input field, it should be reflected in the calendar UI, I made this one of this library's design principles because it allows us to have the UI that tells users "What you can see is what you can do." So, I'm sorry but since your suggestion in general here violates the principle, it cannot be acceptable.

The dateInvalid event, on the other hand, could be a possible option. However it doesn't work as expected in multi-date mode because, in that mode, an input including invalid date(s) is treated as an error only when all the dates are invalid—one mixed with valid and invalid dates is not. To make the event be fired even when the number of invalid dates is only one, we need to either change the multi-date mode's behavior or add an extra input check. The former will be a breaking change and the latter seems redundant to me. So I'd like to see what other people think about paying that cost for this feature.

And FYI, you don't need to rewrite the parser—it's available as a static (class) method. https://mymth.github.io/vanillajs-datepicker/#/api?id=datepickerparsedate

henkesn commented 2 years ago

Thank you for the hint with the parseDate. Perhaps it's a bit a philosophical question with the input and validation. While I think an input like a datepicker should be only responsible for (graphical) input and some other code in case of a client side framework or even the server should be responsible for validation/enforcement of correctness, you are following a more holistic approach. Probably, your approach is better for the range features. So perhaps it's better to just keep your style instead of rolling up the whole thing and start to mix both. :+1: