Implement WAI ARIA recommended focus handling

[pekala]

Fixes #354

Introduces the concept of an active focusable element, to adjust the keyboard interaction to match WAI-ARIA recommendations.

@wojtekmaj Would love your help in pushing this out. Things I don't handle yet (that I know of):

• Min/max dates (should be easy)
• Disabled dates - that one will be tricky. I guess ideally we would jump to the next non-disabled date, but not sure how we can implement that in a sane way

[wojtekmaj]

That's a breaking change - we're currently requiring React 16.3 and up. Not saying this is a bad thing, just noting this to self.

[pekala]

Ah right. Well, you could make that call I guess, in the similar spirit as not supporting older browsers.

[pekala]

@wojtekmaj so, WDYT? Is this an acceptable path forward?

[wojtekmaj]

Yes, absolutely! We will need to wait for the next major release with this though, unfortunately.

[thepuzzlemaster]

You may have already addressed it, but I could see an argument made for still being able to focus a disabled element. W3 guidance around disabled elements within a listbox, for instance support focusing on disabled elements, as otherwise, a non-sighted user would not be aware of them.

Granted, grids, are not clear on if a cell is disabled, how that would effect keyboard navigation/focus - I could see an argument made here.

[skatt931]

Hello, do you have an approximate time for releasing this feature? Would be glad to have some information about it :)

[wojtekmaj]

This change is breaking compatibility with older versions of React so this will need to be merged after the next major, dropping support for React <16.8 is released.

[clicktravel-danielkentfield]

Is there any info on when this might be released?

This is quite an important feature for us.

[Merri]

This focus handling implementation won't work when rendering multiple months:

We don't want the calendar component to automatically shift activeStartDate within a single calendar instance, but would like to see it jump to the next calendar.

Doing this multiple months feature is pretty easy with current react-calendar:

export function MonthlyCalendar(props: Props) {
  const { formatDate } = useIntl();
  // date-fns
  const months = eachMonthOfInterval({ start: props.minDate, end: props.maxDate });

  return (
    <>
      {months.map((startOfMonth) => (
        <Wrapper key={startOfMonth.toISOString()}>
          <Title>
            <FormattedDate value={startOfMonth} month="long" year="numeric" />
          </Title>
          <Calendar
            {...props}
            activeStartDate={startOfMonth}
            maxDetail="month"
            showNavigation={false}
            showNeighboringMonth={false}
            formatLongDate={(_locale, date) => formatDate(date, dayOptions)}
          />
        </Wrapper>
      ))}
    </>
  );
}

It might be that this kind of rendering has to be supported by react-calendar to make the focus handling work as expected in this use case?

Update Decided to do a full-custom implementation to make sure things work nice from a11y standpoint.

[deanohyeah]

can we bring this back now that 4.0.0 is out with react 16.8+

[wojtekmaj]

We most definitely can!

[wojtekmaj]

Some issues I have noticed:

• Using arrows when focus is on calendar tiles causes the page to scroll
• I can't seem to be able to navigate to the tiles - I feel like after you tabbed through navigation the focus should go to the tile grid?

[pekala]

Unfortunately I won't find time to work on this any time soon :(

[thepuzzlemaster]

I'd love to help get this over the finish line, as it seems pretty close. Thanks for the great start @pekala.
I've pulled down the most recent working branch from your repo, and rebased on top of the latest from main in this repo and pushed it up here.

I fixed the issue with the page scrolling on arrow usage, and made a couple other UX improvements.
I also wanted to check on a couple things.

1. I noticed that there are a bunch of test failures when running yarn test. Have the tests ever passed? I'm not sure if this was a change related to rebasing on top of the updates in main, or if they just never got run, and I need to make some fixes to address the changes. Nevermind. I was able to fix the failing tests.
2. Are @pekala and @wojtekmaj okay with me taking the baton in getting this over the finish line? Seems like everyone would be on board, but just wanted to make sure.
3. Should I just create a new PR, or is there a way to edit this PR to point to my fork now instead, so we can continue the conversation here?

Thanks!

[thepuzzlemaster]

I ended up creating a new PR - figuring they will reference each other, so none of the history in this thread will be lost.