platform icon indicating copy to clipboard operation
platform copied to clipboard

DatePicker support for disabling arbitrary dates

Open rolfsmeds opened this issue 2 years ago • 34 comments

Description

A mechanism for dynamically disabling arbitrary dates in the date picker overlay and automatically invalidating entering them manually.

Use cases

As a user I want the date picker to show me which dates are unavailable, and prevent me from inadvertently picking or entering such a date. So that I can avoid entering unavailable dates

Relation to min/max

While this feature partially overlaps the functionality provided by the existing min/max feature, this would give developers more granular control over available dates (allowing them to e.g. disable certain weekdays or individual dates), rather than just enabling a range of dates.

API proposal

An API similar to ItemEnabledProvider in CheckboxGroup and RadioButtonGroup would probably make sense here, i.e. something along the lines of public void setDateEnabledProvider​(SerializablePredicate<LocalDate> dateEnabledProvider) that would be used like

datepicker.setDateEnabledProvider(date->{
  return !date.getDayOfWeek().equals(DayOfWeek.SUNDAY);
});

Acceptance criteria

  • [ ] Disabled dates rendered as such in the picker calendar (same as those outside min/max range)
  • [ ] Manual entry of a disabled date invalidates field
  • [ ] Min/max range has priority, i.e. if min/max range is also provided, anything outside it will be disabled even if the enabledProvider returns true.
  • [ ] Dates disabled this way shall be keyboard-focusable (as opposed to dates outside the min/max range, which are, and shall continue to be, non-focusable)
  • [ ] Java and WC APIs for providing the function

General criteria

  • [ ] APIs reviewed
  • [ ] Performance (should not have noticeable effect on datepicker performance)
  • [ ] UX/DX tests in Alpha
  • [ ] Documentation: (add new subsection under the Validation section in Date Picker docs)
  • [ ] How to test?
  • [ ] Limitations:

rolfsmeds avatar Mar 28 '22 12:03 rolfsmeds

Re: min/max: I suppose we could migrate those to internally use this feature? Or should those be marked as deprecated and instruct users to use this new feature instead?

jouni avatar Mar 28 '22 12:03 jouni

I would suggest to have min/max stay like they are at the moment. Now the web component can check if something is > max without additional server round-trips.. with the proposed java API the web component has to ask the server again.

knoobie avatar Mar 28 '22 12:03 knoobie

Hmm, right. I was only considering the client-side use. But I still feel min/max should be possible to internally use this new feature instead of being a separate implementation.

jouni avatar Mar 28 '22 12:03 jouni

Neither for or against refactoring min/max to be based on this, but I do think it's meaningful to provide the min/max API after this is in, as it's a simpler, dedicated API for what is probably a more common use case than disabling arbitrary dates.

rolfsmeds avatar Mar 28 '22 20:03 rolfsmeds

I worry that an additional min/max API requires developers to think how these APIs interact, and that less API would be easier to maintain when there would not be the need to orchestrate them together. That’s all.

I assume min/max would always override this other way of disabling dates, and I feel that’s how the majority will assume it works. But there might be some who think they can re-enable a particular day outside the min–max range by returning true from setDateEnabledProvider(). A thing to mention in the documentation, I believe.

Min/max is surely more convenient than something like this, I admit that:

datePicker.isDateAvailable( date => Date.UTC(2022, 0) <= date && date <= Date.UTC(2022, 11) );

or

render() {
  return html`<vaadin-date-picker is-date-available="${ date => Date.UTC(2022, 0) <= date && date <= Date.UTC(2022, 11) }">`;
}

compared to

<vaadin-date-picker min="2022-01" max="2022-12">

jouni avatar Mar 29 '22 08:03 jouni

I've just realised that implementing the proposed Java API might be problematic. The server side check needs to run for each date cell and this means there can be multiple requests especially while scrolling 🤔

web-padawan avatar Mar 29 '22 09:03 web-padawan

Argh, well, yes, of course it does, now that I think about it. And thanks to the design of the picker, it's really easy to scroll a lot very quickly.

Of course multiple "pages" of months could be evaluated each time, a few on each side of the month requested, but even that might be tricky considering how quickly one can scroll in the picker.

I suppose one "solution" could be to initially render all dates as disabled or "undecided", delay the availability evaluation until scrolling has stopped, and then update the dates when you have the results. But I worry that the UX would be awful.

rolfsmeds avatar Mar 29 '22 09:03 rolfsmeds

Thinking out load: instead of creating a "high level" API on the server.. could this be abstracted to something like

datePicker.disallowSaturday():
datePicker.disallowSunday();
// allows to specify a full date with year, month and day 
// or month and day to allow the client&service side to automatically disable all years with this date
datePicker.disallowSpecificDates("2022-12-24", "12-25");

This does not allow for full flexible solutions like the shown lambda above.. but would drastically reduce the amount of client/server communication.

knoobie avatar Mar 29 '22 10:03 knoobie

Yeah, that might be good enough. Though instead of datePicker.disallowSaturday() (and Sunday), why not datePicker.disallowWeekDays(6, 7)?

jouni avatar Mar 29 '22 10:03 jouni

Why you should keep min/max Min/max can affect how the popup behaves so that it stops scrolling when reaching a limit. With an arbitrary "is available" you would need to allow the user to keep scrolling to see if a date 100 years ahead happens to be OK.

The web component API Why not make that as good and flexible as possible, i.e. a lambda callback. There are no performance problems there

The Java API Make it "performant enough" by default. Make it possible to use the web component API when you really need all the flexibility. That allows you to optimize the Java API for the most common cases without excluding any use case.

Before optimizing the API though: Which are the primary 3 use cases this is meant for? Disabling weekends? Disabling days when the veterinary has no available timeslots?

Artur- avatar Mar 29 '22 10:03 Artur-

Before optimizing the API though: Which are the primary 3 use cases this is meant for? Disabling weekends? Disabling days when the veterinary has no available timeslots?

Those two are exactly the ones I've had in mind. Don't have a third one 😁

Obviously the first one, disabling weekends or specific weekdays in general, could be solved with a much simpler API and implementation without performance issues, but the second one, disabling, or even distinguishing arbitrary dates is also a fairly common use case.

(Also I just realized that this is very similar to styling individual dates according to some logic as requested in https://github.com/vaadin/web-components/issues/1925)

I guess we could have a Java API that makes it easy to set a clientside lambda callback... and in cases where that callback does its thing solely based on logic, you could implement that logic with JS, but how do you provide it with data for cases where the evaluation is based on things like the availability of appointment time slots?

rolfsmeds avatar Mar 29 '22 13:03 rolfsmeds

As a user, I would prefer to have the callback lambda in Java (as described above) and have you figure out how to make the communication efficient and which dates to check :) It is a bit similar to caching rows in a grid instead of fetching one cell at a time

Artur- avatar Mar 29 '22 14:03 Artur-

Maybe I'm pointing out the obvious, but disabling weekdays like Saturdays or Sundays is only covering one use case; you might also want to disable dates like the 4th of April 2022 because the system has scheduled maintenance that day.

OlliTietavainenVaadin avatar Apr 04 '22 05:04 OlliTietavainenVaadin

You might also want to disable a range between June 1st to August 1st because that's when your summer holiday is.

OlliTietavainenVaadin avatar Apr 04 '22 05:04 OlliTietavainenVaadin

The use case mentioned above, "Disabling days when the veterinary has no available timeslots", is precisely that: any arbitrary dates.

rolfsmeds avatar Apr 04 '22 07:04 rolfsmeds

This request is exactly what we need in our use of the vaadin date picker, except we have no need for the Java side of things. We use the date picker purely as a web component consumed within a LitElement. Would it be acceptable for us to develop a PR that implements a Javascript-only implementation where the is-date-available function is provided as a Javascript function that returns boolean? This is how our legacy datepicker is implemented and we're hoping to replace it with the Vaadin date picker yet keep this critical piece of functionality for disabling arbitrary dates.

ghost avatar Jan 04 '23 15:01 ghost

@dethell-jh we will want to have that Java API also. Although we could of course implement that ourselves, the clientside implementation needs to be built in a way that makes it possible to implement the Java API in a sufficiently flexible and performant way.

We can try to come up with a technical design for this, after which you can contribute your clientside implementation.

rolfsmeds avatar Jan 04 '23 21:01 rolfsmeds

I think quite common use case for this feature would be to disable selection of non-working days / i.e. allow selecting only dates that are open for business. This naturally can in practice mean any combination of arbitrary dates, as bank holidays differ by country, and some businesses are open regardless of those, etc. Alternative case could be some sort of availability restriction, i.e. disable selection of dates that are "fully booked".

Currently doing this is quite complex in even simple scenarios. I have an example of just disabling weekends here using combination of CSS and custom validator

https://github.com/TatuLund/hilla-demo/blob/master/frontend/themes/hillacrmtutorial/components/vaadin-month-calendar.css

And

https://github.com/TatuLund/hilla-demo/blob/master/frontend/util/validators.ts

The example is Hilla application, but CSS applies both Hilla/Flow and similar custom validator can be applied Java.

TatuLund avatar Jan 05 '23 06:01 TatuLund

Just pushed up a draft PR with a working, client-side-only implementation. Figured it was worth seeing an implementation to assist in the discussion. Per the comment there in the PR we as a team are questioning whether any implementer will really want to have server-side, Java-based logic to control the isDateAvailable logic. Seems like a ton of API traffic to paint the calendars.

ghost avatar Jan 06 '23 19:01 ghost

Now you know exactly the hard part about this API, because we Java devs wanna have fun with that feature as well 🤓

knoobie avatar Jan 06 '23 19:01 knoobie

Per the comment there in the PR we as a team are questioning whether any implementer will really want to have server-side, Java-based logic to control the isDateAvailable logic. Seems like a ton of API traffic to paint the calendars.

This is why this feature was postponed. We might need to figure out a better approach that would not have this problem. IMO the set of dates that need to be disallowed for selection could be set separately e.g. via setDisabledDates().

web-padawan avatar Jan 06 '23 21:01 web-padawan

This is why this feature was postponed. We might need to figure out a better approach that would not have this problem. IMO the set of dates that need to be disallowed for selection could be set separately e.g. via setDisabledDates().

Does the team here think it is acceptable to have some Javascript-only validation for things like validation as a first step and have a feature request to explore the Java-side later? Seems a shame to hold up features in the web components just because there is no corollary in the hilla/Java side of things. I came from the Java framework world (way back in Tapestry and prior) so I definitely get the attraction of having this available to Java developers. Just think there could be some middle ground as we progress the web components forward.

For now we intend to go ahead and get working tests in the PR and publish the PR as a working example of how it can be done and let y'all decide what is best. We'll work from a fork if we need to, though not ideal.

ghost avatar Jan 09 '23 15:01 ghost

I think it should be fine to have some features only available in the web component. Just like we have some features that are only available in the Java/Flow component. Not ideal, but can be justified in some cases.

jouni avatar Jan 10 '23 14:01 jouni

I'd say it's ok to have a WC-only feature, but it should be built in a way that can accommodate Java-support later.

rolfsmeds avatar Jan 10 '23 15:01 rolfsmeds

What do you mean exactly? Do you mean that it should be possible to use the WC-only feature together with whatever Java/Flow feature is added later? Or that the WC-only feature should be able to handle the Java/Flow feature in the future as well? The latter implies that it's not really only for the web component.

I was in my mind thinking that there could be both APIs, this client-only isDateAvailable callback function, which is called on every single date element, and in the future a more async-friendly setAvailableDates/setDisabledDates method, which Flow can use.

jouni avatar Jan 10 '23 18:01 jouni

We could indeed have isDateAvailable in the web component and then build Flow component API on top of that. In case of setDisabledDates it would essentially mean configuring a custom isDateAvailable in the connector.

Some examples of naming from existing design systems:

web-padawan avatar Jan 10 '23 18:01 web-padawan

This is all good news. I'll get the tests finished off for this PR and change it to Ready to Review.

ghost avatar Jan 10 '23 21:01 ghost

@jouni these are exactly the kinds of design decisions that should be made before we bring this to the WC

rolfsmeds avatar Jan 11 '23 07:01 rolfsmeds

I have added unit tests and changed the PR status to Ready to Review. I couldn't figure out how to run a coverage report so I'm assuming I've added enough coverage at this point with these tests.

https://github.com/vaadin/web-components/pull/5252

ghost avatar Jan 17 '23 22:01 ghost

I'd like to add the requirement/wish, that it should also be possible to define tooltips for certain disabled dates, so that the user gets the info why the day is disabled, e. g. "Christmas", "New Year", "Day Of The Tentacle", etc.

stefanuebe avatar Sep 11 '23 16:09 stefanuebe