feat: keyboard navigation in calendar #858

jacobdevera · 2020-01-27T08:16:06Z

Description

Adds the following characteristics to Calendar:

Focus is set on the current date by default
Press the LEFT and RIGHT arrow keys to navigate the row by week day.
Press the UP and DOWN arrow keys to navigate between weeks on the same week day.
Will go to next/previous month if:
- Passing first/last day of the month with left/right arrow keys OR
- Passing first/last week of the month with up/down arrow keys
Press the HOME and END keys to jump to the beginning or end of the current row
Press the PAGEDOWN and PAGEUP keys to navigate backwards or forwards by month
Press SHIFT+PAGEDOWN and SHIFT+PAGEUP to navigate backwards or forwards by year
Press the ENTER key to activate the selected date
Mouse users can click the desired date buttons as usual
Tab cycles between date grid and month/year navigation keys
Should be able to activate month/year header to enter month/year selection mode with similar keyboard interactions to selecting dates

Known issues

When a cell is both focused and selected, the white text against light grey background is a contrast accessibility issue. It looks awkward for range selection as well with the border-radius difference. Will require a styles update to address.

Desired

Use Popover for DatePicker to allow for Escape to close the popover, and trap focus within the Calendar: some issues with date validation currently that is making this difficult
Announce instructions for arrow key navigation: accessibility-wise, it's best for this to be onscreen and recognizable by both sighted and blind users, though if we want to hide this, would probably require a styles update.

References

https://www.w3.org/TR/wai-aria-practices/examples/dialog-modal/datepicker-dialog.html

Fixes

fixes #829 & #827 & #787

netlify · 2020-01-27T08:18:51Z

Deploy preview for fundamental-react ready!

Built with commit 6a466a4

https://deploy-preview-858--fundamental-react.netlify.com

bcullman · 2020-01-27T19:21:33Z

src/Calendar/Calendar.js

            .locale(this.props.locale)
-            .month(month)
-            .date(1);
+            .month(month);


careful: This was probably put in place so that you don't accidentally jump from 1/31/2020 to 2/31/2020

With moment, it will now set it to the latest day if the new month doesn't have enough days. Can go either way on this though.

meganaconley · 2020-01-27T22:08:14Z

When I use PAGEUP/PAGEDOWN it selects the day it lands on in the next/previous month, even if I had already selected a day.

Another note is the odd interaction at first. I did a shift-tab back into the calendar and it put my focus on Feb 1 at first, but hitting left arrow got me on Jan 30 instead of Jan 31. I haven't been able to repro though.

jbadan · 2020-01-31T19:08:22Z

Should the Calendar popover close once I move focus to a new DatePicker input?

greg-a-smith · 2020-01-31T22:19:21Z

Not specifically related to keyboard navigation, but when going from April 2020 to May 2020, the size of the calendar grows because of how the days line up. There are actually 6 weeks with days in May. Should we add something such that 6 weeks always show in the Calendar (and then DatePicker) so there is no "jumping"?

greg-a-smith · 2020-01-31T22:46:45Z

Overall, it seems pretty good. Some random thoughts/observations:

The currently selected/focused element (in the day, month or year view) is gray (sometimes with white text). It's difficult to tell that it's where your cursor will start from -- it looks disabled.
When the year view is invoked, the PAGEUP and PAGEDOWN change months. I understand this is the behavior in the day or month view (which makes sense), but it seems odd when you're looking at years.
When the focused day/month/year is on the currently selected value, it loses the blue background and it's just gray.
There's no way to dismiss the month or year view other than selecting a month or year. It seems like ESC should switch back to the day view. Should month and year views be popovers/overlays instead? 🤔

jacobdevera · 2020-02-11T16:29:11Z

Should the Calendar popover close once I move focus to a new DatePicker input?

For sure. May probably take another stab at changing Calendar's popover to a Popover.

Should we add something such that 6 weeks always show in the Calendar (and then DatePicker) so there is no "jumping"?

I don't see a specification in Fiori for always showing 6 weeks, and I would think it would be odd to include an extra week entirely from the next month if the month only has 5 weeks. What some other date pickers do is leave space for 6 weeks, but not show the last week; but again, there's no specification for what that would look like.

The currently selected/focused element (in the day, month or year view) is gray (sometimes with white text). It's difficult to tell that it's where your cursor will start from -- it looks disabled.

When the focused day/month/year is on the currently selected value, it loses the blue background and it's just gray.

These are styles issues that were not initially made in the first pass through changing calendar styles, because there wasn't a spec for focus indication in Fiori. I just opted to use the hover style, which isn't ideal in hindsight. This PR (SAP/fundamental-styles#506) seems to have a good solution for focus styles. We should address that upgrade in a separate change.

When the year view is invoked, the PAGEUP and PAGEDOWN change months. I understand this is the behavior in the day or month view (which makes sense), but it seems odd when you're looking at years.

If PGUP/PGDN changed the year in the year view, then we probably have to change what SHIFT+PGUP/PGDN does too. I'm not sure making those keys do the same thing, do nothing, or making it change the month is a good idea.

There's no way to dismiss the month or year view other than selecting a month or year. It seems like ESC should switch back to the day view. Should month and year views be popovers/overlays instead?

I think changing the month and year views to popover would be quite a big refactor (needing to remove the box shadow, trying to make it work within the Calendar popover, etc.). Ultimately other date pickers I have seen just close the whole widget altogether if there is a separate month/year picker, which doesn't seem to be a bad idea.

bcullman · 2020-02-11T17:30:11Z

@jacobdevera - its important to have a static height for the calendar, otherwise, while flipping through the months in a near page edge situations, popper will flip the calendar up when it encounters 6 weeks, and flip back when it goes back to 5 weeks.

a static height assures that popper wont reposition the calendar while navigating through months.

jacobdevera · 2020-02-11T23:30:20Z

src/DatePicker/DatePicker.test.js

        mount(compactRangeDatepicker);
    });

-    test('open/close by calendar icon button', () => {


These essentially test opening/closing the calendar which should now be covered by using the Popover component itself.

jbadan

All my comment have been addressed 🚢

jbadan · 2020-02-17T21:21:59Z

Once you merge latest master into here, can you also remove these lines: https://github.com/SAP/fundamental-react/blob/master/src/DatePicker/DatePicker.js#L25

Because you replaced the hardcoded html for popover in here with the actual Popover component, it doesn't need this style block again

jacobdevera added 14 commits January 21, 2020 14:55

feat: gridmanager, intial arrow key movement

5245fa7

feat: row and col wrapping

9f4eedd

fix: enter and space handling

f67eac3

fix: button labels, navigation fixes

cb4f411

fix: first focusable element

4d97ec7

feat: skip disabled and block dates

d3b5341

fix: grid boundary logic

e783f40

fix: date labels

53e19d4

fix: focus on init logic

e0b8886

feat: page up, page down, home, end handling

50803e1

fix: header option, tests

de77452

chore: remove edit mode references

59e78b3

fix: datepicker state

0870296

chore: cleanup

0cc38fa

jacobdevera self-assigned this Jan 27, 2020

jacobdevera requested review from a team and meganaconley January 27, 2020 08:16

jacobdevera added 2 commits January 27, 2020 00:17

fix: disabled cells

ba119fe

fix: test

daacb26

jacobdevera added 3 commits January 27, 2020 00:20

fix: prevent default

42d1b1d

fix: default for other keys

7bd09ea

Merge branch 'master' into feat/kb-calendar

74631bb

bcullman reviewed Jan 27, 2020

View reviewed changes

fix: page keys shouldn't select date

44889b7

meganaconley requested a review from a team January 31, 2020 18:12

jacobdevera added 3 commits February 11, 2020 10:19

fix: six weeks

3dd52d7

Merge branch 'master' into feat/kb-calendar

c7fbf0a

fix: datepicker popover

2a2de97

jacobdevera commented Feb 11, 2020

View reviewed changes

jacobdevera added 2 commits February 11, 2020 15:40

fix: visual regression

0c3620e

fix: button label, live area

0791a01

jbadan approved these changes Feb 17, 2020

View reviewed changes

jbadan mentioned this pull request Feb 17, 2020

feat: replace withStyles wrapper with useEffect and remove customStyles property #873

Merged

Merge branch 'master' into feat/kb-calendar

ec9ce22

fix: merge

3c8d202

jbadan mentioned this pull request Feb 18, 2020

Calendar popup should close when date is selected #876

Closed

jbadan linked an issue Feb 18, 2020 that may be closed by this pull request

Calendar popup should close when date is selected #876

Closed

jacobdevera added 3 commits February 18, 2020 12:06

fix: element.matches check

807d405

fix: merge, null checks

c4f3840

fix: close after selecting

6a466a4

jacobdevera mentioned this pull request Feb 18, 2020

Update Calendar markup for a11y #787

Closed

jacobdevera linked an issue Feb 18, 2020 that may be closed by this pull request

Update Calendar markup for a11y #787

Closed

jacobdevera merged commit 650b7b9 into master Feb 20, 2020

jacobdevera deleted the feat/kb-calendar branch February 20, 2020 05:28

jbadan mentioned this pull request Mar 4, 2020

DatePicker Popover should appear above the Input when there is not enough space #828

Closed

jacobdevera mentioned this pull request Mar 4, 2020

DatePicker uses componentWillMount which has been renamed, and not recommended for use. #827

Closed

coding-my-passion mentioned this pull request Mar 26, 2020

Datepicker - Trigger the onBlur event when the Datepicker closes #930

Closed

feat: keyboard navigation in calendar #858

feat: keyboard navigation in calendar #858

Uh oh!

Conversation

jacobdevera commented Jan 27, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Known issues

Desired

References

Fixes

Uh oh!

netlify bot commented Jan 27, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bcullman Jan 27, 2020

Choose a reason for hiding this comment

Uh oh!

jacobdevera Jan 27, 2020

Choose a reason for hiding this comment

Uh oh!

meganaconley commented Jan 27, 2020

Uh oh!

jbadan commented Jan 31, 2020

Uh oh!

greg-a-smith commented Jan 31, 2020

Uh oh!

greg-a-smith commented Jan 31, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jacobdevera commented Feb 11, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bcullman commented Feb 11, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jacobdevera Feb 11, 2020

Choose a reason for hiding this comment

Uh oh!

jbadan Feb 12, 2020

Choose a reason for hiding this comment

Uh oh!

jbadan left a comment

Choose a reason for hiding this comment

Uh oh!

jbadan commented Feb 17, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

jacobdevera commented Jan 27, 2020 •

edited

Loading

netlify bot commented Jan 27, 2020 •

edited

Loading

greg-a-smith commented Jan 31, 2020 •

edited

Loading

jacobdevera commented Feb 11, 2020 •

edited

Loading

bcullman commented Feb 11, 2020 •

edited

Loading