Skip to content

feat: Allow disabling arbitrary dates in vaadin-date-picker via isDateAvailable property #5252

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.

Sign up for GitHub

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

Closed
wants to merge 5 commits into from
Closed

feat: Allow disabling arbitrary dates in vaadin-date-picker via isDateAvailable property #5252

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
2f732de
Add isDateAvailable property
dethell Jan 5, 2023
c90717f
Further changes to allow disabling arbitrary dates
dethell Jan 6, 2023
119e21c
Starting to add tests for isDateAvailable changes
dethell Jan 9, 2023
b1e9d3b
Add validation and disabled tests
dethell Jan 17, 2023
6a2c028
Merge branch 'vaadin:main' into master
Sep 20, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/date-picker/src/vaadin-date-picker-helper.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ declare function dateEquals(date1: Date | null, date2: Date | null): boolean;
*
* @returns True if the date is in the range
*/
declare function dateAllowed(date: Date, min: Date | null, max: Date | null): boolean;
declare function dateAllowed(date: Date, min: Date | null, max: Date | null, isDateAvailable: Function | null): boolean;

/**
* Get closest date from array of dates.
Expand Down
5 changes: 3 additions & 2 deletions packages/date-picker/src/vaadin-date-picker-helper.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,9 @@ export function dateEquals(date1, date2) {
* @param {Date} max Range end
* @return {boolean} True if the date is in the range
*/
export function dateAllowed(date, min, max) {
return (!min || date >= min) && (!max || date <= max);
export function dateAllowed(date, min, max, isDateAvailable) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's add the new parameter to the JSDoc.

const dateIsAvailable = isDateAvailable && typeof isDateAvailable === 'function' ? isDateAvailable(date) : true;
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
const dateIsAvailable = isDateAvailable && typeof isDateAvailable === 'function' ? isDateAvailable(date) : true;
const dateIsAvailable = typeof isDateAvailable === 'function' ? isDateAvailable(date) : true;

The typeof check should be enough.

return (!min || date >= min) && (!max || date <= max) && dateIsAvailable;
}

/**
Expand Down
5 changes: 5 additions & 0 deletions packages/date-picker/src/vaadin-date-picker-mixin.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -237,6 +237,11 @@ export declare class DatePickerMixinClass {
*/
max: string | undefined;

/**
* Function to override the default function for determining if a date is available.
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's use the same description as in the JSDoc here.

*/
isDateAvailable: (date: Date) => boolean;

/**
* Opens the dropdown.
*/
Expand Down
19 changes: 15 additions & 4 deletions packages/date-picker/src/vaadin-date-picker-mixin.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import {
extractDateParts,
getAdjustedYear,
getClosestDate,
parseDate,
parseDate
} from './vaadin-date-picker-helper.js';

/**
Expand Down Expand Up @@ -302,6 +302,15 @@ export const DatePickerMixin = (subclass) =>
sync: true,
},

/**
* A function that is used to determine if a date should be disabled.
*
* @type {function(Date): boolean | undefined}
*/
isDateAvailable: {
type: Function,
},
Comment on lines +305 to +312
Copy link
Contributor

Choose a reason for hiding this comment

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

  1. Since the main purpose of the new feature is to disable specific dates, I think it would make more sense to name this to isDateDisabled.

  2. While passing a date instance is reasonable, I think for consistency we should align this with other functions that can be configured currently: formatDate and parseDate via the i18n property. Those take and return an object of the form: { year: number, month: number, day: number }.

  3. The param should also be documented in some detail in the JSDoc.

Copy link
Author

Choose a reason for hiding this comment

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

Commenting here even though I'm moving to a new PR. Are you sure we should make the object sent to the isDateDisabled function the DatePickerDate interface? The reason I ask is the main place this gets called is in vaadin-date-picker-helper in the dateAllowed function. That function receives a Javascript Date rather than a vaadin DatePickerDate shape.


/**
* The earliest date that can be selected. All earlier dates will be disabled.
* @type {Date | undefined}
Expand Down Expand Up @@ -365,7 +374,7 @@ export const DatePickerMixin = (subclass) =>
return [
'_selectedDateChanged(_selectedDate, i18n)',
'_focusedDateChanged(_focusedDate, i18n)',
'__updateOverlayContent(_overlayContent, i18n, label, _minDate, _maxDate, _focusedDate, _selectedDate, showWeekNumbers)',
'__updateOverlayContent(_overlayContent, i18n, label, _minDate, _maxDate, _focusedDate, _selectedDate, showWeekNumbers, isDateAvailable)',
'__updateOverlayContentTheme(_overlayContent, _theme)',
'__updateOverlayContentFullScreen(_overlayContent, _fullscreen)',
];
Expand Down Expand Up @@ -591,6 +600,7 @@ export const DatePickerMixin = (subclass) =>
const inputValue = this._inputElementValue;
const inputValid = !inputValue || (!!this._selectedDate && inputValue === this.__formatDate(this._selectedDate));
const minMaxValid = !this._selectedDate || dateAllowed(this._selectedDate, this._minDate, this._maxDate);
const isDateAvailableValid = this.isDateAvailable ? this.isDateAvailable(this._selectedDate) : true;
Comment on lines 602 to +603
Copy link
Contributor

Choose a reason for hiding this comment

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

Since dateAllowed is capable of checking for whether the date should be disabled, it seems unnecessary to call isDateAvailable separately. Instead, I would suggest to pass the function into dateAllowed, and generalize the constant name from minMaxValid to something more general like isDateValid.

Copy link
Author

Choose a reason for hiding this comment

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

Much cleaner.


let inputValidity = true;
if (this.inputElement) {
Expand All @@ -602,7 +612,7 @@ export const DatePickerMixin = (subclass) =>
}
}

return inputValid && minMaxValid && inputValidity;
return inputValid && minMaxValid && inputValidity && isDateAvailableValid;
}

/**
Expand Down Expand Up @@ -812,7 +822,7 @@ export const DatePickerMixin = (subclass) =>

/** @private */
// eslint-disable-next-line max-params
__updateOverlayContent(overlayContent, i18n, label, minDate, maxDate, focusedDate, selectedDate, showWeekNumbers) {
__updateOverlayContent(overlayContent, i18n, label, minDate, maxDate, focusedDate, selectedDate, showWeekNumbers, isDateAvailable) {
if (overlayContent) {
overlayContent.i18n = i18n;
overlayContent.label = label;
Expand All @@ -821,6 +831,7 @@ export const DatePickerMixin = (subclass) =>
overlayContent.focusedDate = focusedDate;
overlayContent.selectedDate = selectedDate;
overlayContent.showWeekNumbers = showWeekNumbers;
overlayContent.isDateAvailable = isDateAvailable;
}
}

Expand Down
31 changes: 21 additions & 10 deletions packages/date-picker/src/vaadin-month-calendar.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,12 +49,12 @@ class MonthCalendar extends MonthCalendarMixin(ThemableMixin(PolymerElement)) {
<template is="dom-repeat" items="[[week]]">
<td
role="gridcell"
part$="[[__getDatePart(item, focusedDate, selectedDate, minDate, maxDate)]]"
part$="[[__getDatePart(item, focusedDate, selectedDate, minDate, maxDate, isDateAvailable)]]"
date="[[item]]"
tabindex$="[[__getDayTabindex(item, focusedDate)]]"
disabled$="[[__isDayDisabled(item, minDate, maxDate)]]"
disabled$="[[__isDayDisabled(item, minDate, maxDate, isDateAvailable)]]"
aria-selected$="[[__getDayAriaSelected(item, selectedDate)]]"
aria-disabled$="[[__getDayAriaDisabled(item, minDate, maxDate)]]"
aria-disabled$="[[__getDayAriaDisabled(item, minDate, maxDate, isDateAvailable)]]"
aria-label$="[[__getDayAriaLabel(item)]]"
>[[_getDate(item)]]</td
>
Expand All @@ -75,7 +75,7 @@ class MonthCalendar extends MonthCalendarMixin(ThemableMixin(PolymerElement)) {
/** @protected */
_days: {
type: Array,
computed: '_getDays(month, i18n, minDate, maxDate)',
computed: '_getDays(month, i18n, minDate, maxDate, isDateAvailable)',
},

/** @protected */
Expand All @@ -87,7 +87,18 @@ class MonthCalendar extends MonthCalendarMixin(ThemableMixin(PolymerElement)) {
disabled: {
type: Boolean,
reflectToAttribute: true,
computed: '_isDisabled(month, minDate, maxDate)',
computed: '_isDisabled(month, minDate, maxDate, isDateAvailable)',
},

/**
* A function to be used to determine whether the user can select a given date.
* Receives the `Date` object of the date to be selected and should return a
* boolean.
* @type {Function | undefined}
*/
isDateAvailable: {
type: Function,
value: () => true,
},
};
}
Expand All @@ -106,10 +117,10 @@ class MonthCalendar extends MonthCalendarMixin(ThemableMixin(PolymerElement)) {
}

/** @private */
__getDatePart(date, focusedDate, selectedDate, minDate, maxDate) {
__getDatePart(date, focusedDate, selectedDate, minDate, maxDate, isDateAvailable) {
const result = ['date'];

if (this.__isDayDisabled(date, minDate, maxDate)) {
if (this.__isDayDisabled(date, minDate, maxDate, isDateAvailable)) {
result.push('disabled');
}

Expand Down Expand Up @@ -147,16 +158,16 @@ class MonthCalendar extends MonthCalendarMixin(ThemableMixin(PolymerElement)) {

/** @private */
__isDayDisabled(date, minDate, maxDate) {
return !dateAllowed(date, minDate, maxDate);
return !dateAllowed(date, minDate, maxDate, isDateAvailable);
}

/** @private */
__getDayAriaDisabled(date, min, max) {
__getDayAriaDisabled(date, min, max, isDateAvailable) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Doesn't look like the new function is properly passed to this method.

Also indicates a missing test, it looks like month calendar attributes like part and aria-disabled are currently covered by snapshot tests, so a new snapshot test should be added there.

if (date === undefined || min === undefined || max === undefined) {
return;
}

if (this.__isDayDisabled(date, min, max)) {
if (this.__isDayDisabled(date, min, max, isDateAvailable)) {
return 'true';
}
}
Expand Down
14 changes: 14 additions & 0 deletions packages/date-picker/test/month-calendar.common.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,20 @@ describe('vaadin-month-calendar', () => {
const date10 = getDateCells(monthCalendar).find((dateElement) => dateElement.date.getDate() === 10);
tap(date10);
expect(monthCalendar.selectedDate).to.be.undefined;

monthCalendar.isDateAvailable = (date) => {
if (!date) {
return false;
}
return date.toISOString().split('T')[0] !== '2016-02-09';
};
for (let i = 0; i < dateElements.length; i++) {
if (dateElements[i].date.getDate() === 9) {
// Ninth of February.
tap(dateElements[i]);
}
}
expect(monthCalendar.selectedDate).to.be.undefined;
});

describe('i18n', () => {
Expand Down
1 change: 1 addition & 0 deletions packages/date-picker/test/typings/date-picker.types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ assertType<() => void>(datePicker.close);
assertType<() => void>(datePicker.open);
assertType<string | undefined>(datePicker.max);
assertType<string | undefined>(datePicker.min);
assertType<(date: Date) => boolean | undefined>(datePicker.isDateAvailable);
assertType<boolean | null | undefined>(datePicker.showWeekNumbers);
assertType<boolean | null | undefined>(datePicker.autoOpenDisabled);
assertType<boolean | null | undefined>(datePicker.opened);
Expand Down
18 changes: 18 additions & 0 deletions packages/date-picker/test/validation.common.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,24 @@ describe('validation', () => {
expect(datePicker.invalid).to.be.true;
});

it('should reflect correct invalid value on value-changed eventListener when using isDateAvailable', (done) => {
datePicker.isDateAvailable = (date) => {
if (!date) {
return true;
}
return date.toISOString().split('T')[0] !== '2017-01-02';
};
datePicker.value = '2016-01-01'; // Valid

datePicker.addEventListener('value-changed', () => {
expect(datePicker.invalid).to.be.equal(true);
done();
});

datePicker.open();
datePicker._overlayContent._selectDate(new Date('2017-01-02')); // Invalid
});

it('should fire a validated event on validation success', () => {
const validatedSpy = sinon.spy();
datePicker.addEventListener('validated', validatedSpy);
Expand Down