-
Notifications
You must be signed in to change notification settings - Fork 282
feat(ui5-calendar): add disabled dates functionality #12691
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
base: main
Are you sure you want to change the base?
Conversation
|
🚀 Deployed on https://pr-12691--ui5-webcomponents-preview.netlify.app |
Co-authored-by: Dimitar Stoev <[email protected]>
Co-authored-by: Dimitar Stoev <[email protected]>
packages/main/src/DayPicker.ts
Outdated
| * A date is considered disabled if: | ||
| * - It falls outside the min/max date range defined by the component | ||
| * - It matches a single disabled date | ||
| * - It falls within a disabled date range (inclusive of start and end dates) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please double-check the validity of this Copilot suggestion:
Bulleted list states ranges are "inclusive of start and end dates", while the implementation uses exclusive comparisons (">" and "<") when both start and end are present. If you intend inclusivity, update the code to use >= / <= to match the original wording.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in 555c611
Co-authored-by: Dimitar Stoev <[email protected]>
| .realClick(); | ||
|
|
||
| // Verify the date was not selected | ||
| cy.get<Calendar>("#calendar1") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why don't we just check if the day that we are clicking on does not have specific class, but we need to invoke a property and check there?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in 5decd79
| .realClick(); | ||
|
|
||
| // The disabled date should not be selectable | ||
| cy.get<Calendar>("#calendar1") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same as prev comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in 5decd79
packages/main/src/Calendar.ts
Outdated
| return this.getSlottedNodes<SpecialCalendarDate>("specialDates"); | ||
| } | ||
|
|
||
| get _disabledDateRanges() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can't we get this with just this.disabledDates?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in 9d30dc5
packages/main/src/DayPicker.ts
Outdated
| const startTimestamp = this._getTimestampFromDateValue(range.startValue); | ||
| const endTimestamp = this._getTimestampFromDateValue(range.endValue); | ||
|
|
||
| if (endTimestamp) { | ||
| return dateTimestamp > startTimestamp && dateTimestamp < endTimestamp; | ||
| } | ||
|
|
||
| if (startTimestamp && !endTimestamp) { | ||
| return dateTimestamp === startTimestamp; | ||
| } | ||
|
|
||
| return false; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This can be optimised to:
if (endTimestamp) {
return dateTimestamp > startTimestamp && dateTimestamp < endTimestamp;
}
return startTimestamp && dateTimestamp === startTimestamp;
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in e601fb8
| return 0; | ||
| } | ||
|
|
||
| try { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are we sure that either of the methods throw an error to be caught?
If not we can just go for:
const jsDate = this.getValueFormat().parse(dateValue);
const calendarDate = CalendarDate.fromLocalJSDate(
jsDate,
this._primaryCalendarType,
);
return calendarDate.valueOf() / 1000;
And if needed to check if there's calendarDate before returning it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fromLocalJSDate method throws exception.
This PR introduces support for disabling specific date ranges in the ui5-calendar component via the new disabledDates slot. Applications can now pass one or more ui5-date-range elements to the calendar, specifying startValue and/or endValue to mark dates as non-selectable.
Key Changes
Added disabledDates slot to ui5-calendar for passing disabled date ranges.
Implemented logic in DayPicker to prevent selection of dates within disabled ranges.
Updated internal date parsing to respect the calendar’s formatPattern.
Added comprehensive JSDoc documentation for new properties and methods.
Created Cypress tests to verify disabled date functionality, including edge cases and format pattern support.
Updated sample and test pages to demonstrate usage.
Motivation
This feature allows applications to restrict user selection to valid dates only, improving UX for scenarios like booking, scheduling, or compliance.