Add possibility to disable multiple date intervals in `Calendar` and `Datepicker` components #2793

razvancir96 · 2022-02-01T16:22:03Z

Closes Add possibility to disable multiple date intervals in Calendar and Datepicker components

✅ Pull Request Checklist:

Included link to corresponding React Spectrum GitHub Issue.
Added/updated unit tests and storybook for this change (for new code or code which already has tests).
Filled out test instructions.
Updated documentation (if it already exists for this component).
Looked at the Accessibility Practices for this feature - Aria Practices

📝 Test Instructions:

After starting Storybook (yarn start), check out those stories:

After starting the docs (yarn start:docs), check out those pages:

The design should look like the following screenshot:

🧢 Your Project:

This feature is for the CJA app, inside Adobe DX.

razvancir96 · 2022-02-08T22:00:17Z

packages/@react-spectrum/calendar/src/CalendarCell.tsx

@@ -60,6 +61,15 @@ export function CalendarCell({state, currentMonth, ...props}: CalendarCellProps)

  let nativeDate = useMemo(() => date.toDate(state.timeZone), [date, state.timeZone]);
  let formatted = useMemo(() => dateFormatter.format(nativeDate), [dateFormatter, nativeDate]);
+


Not sure if I should move this code somewhere else, it depends on the disabled & selected state, as well as the cell's ref 🤔

razvancir96 · 2022-02-08T22:00:55Z

packages/@react-spectrum/calendar/src/CalendarCell.tsx

+  const previousCell = currentCell?.parentElement?.previousElementSibling?.firstElementChild;
+  const nextCell = currentCell?.parentElement?.nextElementSibling?.firstElementChild;
+
+  const isCellDisabled = (cell: Element): boolean => cell?.classList.value.includes('is-disabled');


Not sure if this is the ideal way to verify if the sibling cell is disables, I am open to suggestions

better to use state if you can
I see there is already a function on the state for checking if the date is disabled
https://github.com/adobe/react-spectrum/pull/2793/files#diff-fd8b93994dbea995798535e4fb0d1323e12e7290667d0329b10ab4209db5199fR199

and there are functions for getting the next day and the previous day I believe

What happens for this if we're on the first cell or the last cell of a grid? just undefined?

You can use state for the current cell, but from what I've seen, each cell has its own state. You can either control the state for the whole calendar or the state for one cell. I don't think you can use the states of two adjacent cells simultaneously (though I might be wrong) 🤔
There are functions for focusing the next day or calculating the next date. However, what I need is a function that returns the state of the next cell (or the effect of the state, like having a certain class), and I didn't find something like that (again, I might be wrong).
And about the last questions: yes, in that case, it returns undefined. Should I add a fallback to false (... || false), so that the API would be more consistent?

I mean, can't you check it like this?

state.isCellDisabled(props.date.subtract({days: 1})) state.isCellDisabled(props.date.add({days: 1}))

code pulled from just below the link I shared, isPreviousVisibleRangeInvalid which I feel like you might just be able to use that function directly?

I'm quite sure we do not want to look in the dom for this state though. My suggestion may not pan out, I'm not super familiar with this code.

Oh, I see now what you meant and you're 100% right. Thank you, I've updated this logic and it's way cleaner now 👌 7d9032c

LFDanLu · 2022-02-16T23:57:50Z

packages/@react-types/datepicker/src/index.d.ts

+  /** Callback that is called for each date of the calendar. If it returns true, then the date is disabled. */
+  isDateDisabled?: (date: DateValue) => boolean,


Open discussion for team, how do we feel about this api versus say something like disabledDates? Was mulling over this and I feel like the current api is more flexible especially when it comes to disabling a range of dates or specifying a special filter (like even days).

@LFDanLu is comparing this to the disabledKeys API
given that a collection will always be available in memory and you can change the disabledkeys based on what is shown, that API is fine for collections

however, for dates, there is always an infinite number of dates and you don’t have the ability to track what month or week or whatever is currently being shown, so you can’t update the disabled dates props
therefore a callback makes the most sense to me

If we go for disabledDates it would not be possible to make all the weekends disabled (for example). So, I think isDateDisabled is more flexible.

packages/@react-stately/calendar/src/useCalendarState.ts

packages/@react-spectrum/datepicker/stories/DateRangePicker.stories.tsx

packages/@react-spectrum/datepicker/docs/DateRangePicker.mdx

packages/@react-spectrum/calendar/test/CalendarBase.test.js

packages/@adobe/spectrum-css-temp/components/calendar/skin.css

snowystinger

I noticed something, and I'm not sure the answer, but right now if you use the arrow keys to keyboard navigate the available dates, when you reach a disabled date, it looks like focus gets lost. Either we need to skip disabled dates or we need to keep the focus ring visible even on disabled dates

It used to be that focus just couldn't move past the boundaries of min/max dates. But we do need to be able to traverse these disabled dates.

I'm leaning towards them being navigable with a visible focus ring because:
You could technically provide a disable range of infinite dates, and we wouldn't necessarily know that, so we can't just skip to the next non-disabled date
Up/Down arrow navigating becomes less predictable

LFDanLu · 2022-02-22T23:27:45Z

packages/@react-spectrum/calendar/docs/Calendar.mdx

+The `isDateDisabled` callback prop makes it possible to disable custom dates from the `Calendar`. It is being called for each date and if it returns `true`, the date cell is disabled.
+


Mind updating the wording in the rest of the docs files (Calendar, RangeCalendar, DatePicker) to match DateRangePicker's change?

Suggested change

The `isDateDisabled` callback prop makes it possible to disable custom dates from the `Calendar`. It is being called for each date and if it returns `true`, the date cell is disabled.

The `isDateDisabled` callback prop acts like a filter function. It is called for each visible date and if it returns `true`, the date cell is disabled.

Updated it in all files 👌

packages/@adobe/spectrum-css-temp/components/calendar/skin.css

snowystinger · 2022-02-23T19:58:48Z

Hey we're in the midst of determining how the disabled ranges should work with keyboard navigation. Does your product team have any opinions or insight?

razvancir96 · 2022-02-24T09:00:50Z

I noticed something, and I'm not sure the answer, but right now if you use the arrow keys to keyboard navigate the available dates, when you reach a disabled date, it looks like focus gets lost. Either we need to skip disabled dates or we need to keep the focus ring visible even on disabled dates

It used to be that focus just couldn't move past the boundaries of min/max dates. But we do need to be able to traverse these disabled dates.

I'm leaning towards them being navigable with a visible focus ring because: You could technically provide a disable range of infinite dates, and we wouldn't necessarily know that, so we can't just skip to the next non-disabled date Up/Down arrow navigating becomes less predictable

From what I see, the AirBnb datepicker keeps the focus on disabled dates: http://react-dates.github.io/react-dates/?path=/story/drp-day-props--with-some-blocked-dates

snowystinger · 2022-02-24T19:12:25Z

Haha, yeah, we were looking at AirBnb as well as the Chrome native input.

One issue with navigating them is that they are disabled dates, but they are focusable, so we're following up with Accessibiliy if those "disabled" dates would need to meet contrast because they are focusable and are therefore interactable even if you can't select them. It'd look pretty weird to someone who is visually impaired, it might look like an empty focusring.
One of us will let you know what accessibility comes back with.

razvancir96 · 2022-03-02T10:47:13Z

Haha, yeah, we were looking at AirBnb as well as the Chrome native input.

One issue with navigating them is that they are disabled dates, but they are focusable, so we're following up with Accessibiliy if those "disabled" dates would need to meet contrast because they are focusable and are therefore interactable even if you can't select them. It'd look pretty weird to someone who is visually impaired, it might look like an empty focusring. One of us will let you know what accessibility comes back with.

Do you have any timeline for this? We would like to know if it would impact our next release.

snowystinger · 2022-03-02T21:38:57Z

Hey, so we are going to have the dates be navigable within the disabled ranges (as they currently are in this PR), we still need to determine the styling of it, but the team can always handle that in a followup. If you wouldn't mind putting the focus ring around the date that is currently being navigated, that should tie up the loose ends for this.

packages/@adobe/spectrum-css-temp/components/calendar/skin.css

packages/@react-spectrum/calendar/src/CalendarCell.tsx

Co-authored-by: Daniel Lu <dlu@livefyre.com>

devongovett

Went ahead and made the above changes. I'll open a followup PR to implement the keyboard navigation changes. Thanks for the contribution!

resolved

razvancir96 requested review from dannify, devongovett and snowystinger as code owners February 1, 2022 16:22

razvancir96 closed this Feb 1, 2022

razvancir96 reopened this Feb 1, 2022

razvancir96 commented Feb 8, 2022

View reviewed changes

LFDanLu reviewed Feb 17, 2022

View reviewed changes

snowystinger reviewed Feb 22, 2022

View reviewed changes

packages/@adobe/spectrum-css-temp/components/calendar/skin.css Outdated Show resolved Hide resolved

snowystinger reviewed Feb 22, 2022

View reviewed changes

LFDanLu previously requested changes Feb 22, 2022

View reviewed changes

devongovett reviewed Mar 3, 2022

View reviewed changes

packages/@adobe/spectrum-css-temp/components/calendar/skin.css Outdated Show resolved Hide resolved

packages/@adobe/spectrum-css-temp/components/calendar/skin.css Outdated Show resolved Hide resolved

packages/@react-spectrum/calendar/src/CalendarCell.tsx Outdated Show resolved Hide resolved

rcirlugea and others added 13 commits March 2, 2022 17:12

Add isDateDisabled prop to calendars & datepickers

7509cc1

Add stories for isDateDisabled prop

ff6dc11

Add tests for isDateDisabled prop

9c66de7

Add docs for isDateDisabled prop

97ff5d2

Improve stories and docs

444c056

Style disabled cells withing range according to design specs

7038526

Round corners before/after disabled ranges according to design specs

157c9ed

Refactor last/first selected before/after disabled logic

e4766d7

Improve coding style

b349d29

Co-authored-by: Daniel Lu <dlu@livefyre.com>

Improve docs

abca8d0

Co-authored-by: Daniel Lu <dlu@livefyre.com>

Improve tests

48547d2

Update docs

b98405f

Fix css bug

f738b62

Move logic to JS

74d79cb

devongovett force-pushed the disable-multiple-dates-calendar branch from 4f49ab3 to 74d79cb Compare March 3, 2022 01:13

devongovett approved these changes Mar 3, 2022

View reviewed changes

devongovett merged commit 51ff50c into adobe:main Mar 3, 2022

devongovett mentioned this pull request Mar 3, 2022

Allow disabled dates in Calendar to be focused #2909

Merged

3 tasks

LFDanLu added the needs testing label Mar 4, 2022

dannify removed the needs testing label Mar 8, 2022

		@@ -60,6 +61,15 @@ export function CalendarCell({state, currentMonth, ...props}: CalendarCellProps)

		let nativeDate = useMemo(() => date.toDate(state.timeZone), [date, state.timeZone]);
		let formatted = useMemo(() => dateFormatter.format(nativeDate), [dateFormatter, nativeDate]);

		/** Callback that is called for each date of the calendar. If it returns true, then the date is disabled. */
		isDateDisabled?: (date: DateValue) => boolean,

		The `isDateDisabled` callback prop makes it possible to disable custom dates from the `Calendar`. It is being called for each date and if it returns `true`, the date cell is disabled.

	The `isDateDisabled` callback prop makes it possible to disable custom dates from the `Calendar`. It is being called for each date and if it returns `true`, the date cell is disabled.
	The `isDateDisabled` callback prop acts like a filter function. It is called for each visible date and if it returns `true`, the date cell is disabled.

Add possibility to disable multiple date intervals in Calendar and Datepicker components #2793

Add possibility to disable multiple date intervals in Calendar and Datepicker components #2793

Uh oh!

Conversation

razvancir96 commented Feb 1, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Pull Request Checklist:

📝 Test Instructions:

🧢 Your Project:

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

snowystinger left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

snowystinger commented Feb 23, 2022

Uh oh!

razvancir96 commented Feb 24, 2022

Uh oh!

snowystinger commented Feb 24, 2022

Uh oh!

razvancir96 commented Mar 2, 2022

Uh oh!

snowystinger commented Mar 2, 2022

Uh oh!

Uh oh!

Uh oh!

Uh oh!

devongovett left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Add possibility to disable multiple date intervals in `Calendar` and `Datepicker` components #2793

Add possibility to disable multiple date intervals in `Calendar` and `Datepicker` components #2793

razvancir96 commented Feb 1, 2022 •

edited

Loading

snowystinger left a comment •

edited

Loading