feat(calendar): keyboard navigation #4667

mizgaionutalexandru · 2024-08-12T09:53:13Z

Description

Mainly this PR adds the keyboard navigation functionality to the Calendar's grid of days. As per WAI keyboard support for date picker dialogs, the selected day is the only one tabbable from the grid. If there is no day selected the fallback will be the current day and the first day of the displayed month. After the said day is in focus, the following keys and functions apply.

Key	Action
Space/ Enter	Selects the focused date
Up/ Down Arrow	Moves the focus to the same day of the previous/next available week. An "available week" is a week that has at least one day available. If the said date is disabled, the focus is moved to the closest available day with respect to the navigation direction (a minimum focus movement difference).
Left/ Right Arrow	Moves the focus to the previous/next available day. It can go from one week to another if the focus is currently on the first available day of the week

If the user changes the focus out from the grid and back in, his previous location should be the one tabbable.

Also, this PR addresses some code suggestions/improvements such as:

removing the unnecessary "spectrum-Calendar-*" from class names
small performance gains when parsing date table cell properties such as "isToday" or "isSelected"
passing a single method for every date cell for click events (see previous comment)
don't dispatch change event when the user selects the previously selected date
allow clicks only in the date cell radius shown by the styles for consistent UX

While creating tests for this new functionality I also overwritten old tests. Some tests have a TODO comment and will be addressed in the next PR to account for the new value API of the components.

Related issue(s)

Motivation and context

Not having keyboard navigation the Calendar wouldn't be keyboard accessible.

How has this been tested?

There can be many use-cases and hopefully most of them are covered in unit tests. As per the behaviour explained above, you can apply said key functions by using the component's storybook.

Test case
1. Go to the Calendar's storybook
2. Click somewhere on the storybook's iframe (where the component is displayed)
3. Press Tab to navigate through the Previous/Next month buttons as well as the selected/initial keyboard navigable day
4. Use your arrow keys (up/down/left/right) to move through the calendar's days
5. Use Space/Enter to select the current day
6. Play around with the mentioned keys, including Tab
Did it pass in Desktop?
Did it pass in Mobile?
Did it pass in iPad?

Screenshots (if appropriate)

keyboard.navigation.mov

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to change)
Chore (minor updates related to the tooling or maintenance of the repository, does not impact compiled assets)

Checklist

I have signed the Adobe Open Source CLA.
My code follows the code style of this project.
If my change required a change to the documentation, I have updated the documentation in this pull request.
I have read the CONTRIBUTING document.
I have added tests to cover my changes.
All new and existing tests passed.
I have reviewed at the Accessibility Practices for this feature, see: Aria Practices

Best practices

This repository uses conventional commit syntax for each commit message; note that the GitHub UI does not use this by default so be cautious when accepting suggested changes. Avoid the "Update branch" button on the pull request and opt instead for rebasing your branch against main.

github-actions · 2024-08-12T10:30:59Z

Branch preview

Visual regression test results

When a visual regression test fails (or has previously failed while working on this branch), its results can be found in the following URLs:

github-actions · 2024-08-12T10:33:13Z

Lighthouse scores

Category	Latest (report)	Main (report)	Branch (report)
Performance	0.99	0.98	0.98
Accessibility	1	1	1
Best Practices	1	1	1
SEO	1	0.92	0.92
PWA	1	1	1

What is this?

Lighthouse scores comparing the documentation site built from the PR ("Branch") to that of the production documentation site ("Latest") and the build currently on main ("Main"). Higher scores are better, but note that the SEO scores on Netlify URLs are artifically constrained to 0.92.

Transfer Size

Category	Latest	Main	Branch
Total	222.526 kB	211.711 kB 🏆	228.673 kB
Scripts	55.128 kB	49.581 kB 🏆	54.519 kB
Stylesheet	34.488 kB	30.144 kB 🏆	42.225 kB
Document	6.125 kB	5.389 kB	5.217 kB 🏆
Third Party	0.00 B	0.00 B	126.712 kB

Request Count

Category	Latest	Main	Branch
Total	48	48	43 🏆
Scripts	40	40	35 🏆
Stylesheet	5	5	5
Document	1	1	1
Third Party	0	0	2

mizgaionutalexandru · 2024-08-12T11:38:25Z

packages/calendar/test/calendar.test.ts

 import { testForLitDevWarnings } from '../../../test/testing-helpers.js';
-import { Calendar } from '@spectrum-web-components/calendar';
-
+import { spy } from 'sinon';


I found this necessary for checking the component's change event. In SWC it seems like sinon is used a lot, even in those testing-helpers, but here I need just the spy functionality.

Related to: #3856 (comment)

mizgaionutalexandru · 2024-08-12T11:39:59Z

packages/calendar/test/calendar.test.ts

+    });
+
+    after(() => {
+        Date.now = originalDateNow;


If sinon is used in these tests, should we also use a sandbox to stub the Date.now function?

mizgaionutalexandru · 2024-08-12T14:12:16Z

packages/calendar/stories/calendar.stories.ts

-        if (container) {
-            render(story, container as HTMLElement);
-        }
+        if (container) render(story, container as HTMLElement);


Can this be done in a different manner or should we stick to requestAnimationFrame?
It seems like without this wrapper approach the component renders only the header (month navigation buttons and the current month indicator) and not the grid of days. Does this hide a problem in the component's lifecycle? 🤔

It seems like the issue comes from the interaction of sp-theme and LanguageResolutionController. Explicitly that the controller's hostConnected is called before the sp-theme's constructor, causing the sp-language-context event to be dispatched before the event listener for said event is added in Theme.ts.
This seems to be a workaround but the StoryDecorator locale picker would work only for the stories that use this approach.

Two possible solutions that I've found are:

Set bundled to false in the swcThemeDecoratorWithConfig (here) - I'm not aware of all the implications and where is this variable set other than the default value.

Add one more call of resolveLanguage in the LanguageResolutionController using the hostUpdate (making sure to call this only for the first update). For environments other than the storybook this one extra call should not be a problem because Theme checks if the contextConsumer was already subscribed (here).

Personally I'm leaning more towards the second solution. What do you think about this @Rajdeepc ?

PS: Worth mentioning that both approaches enable the StoryDecorator locale picker for all existing stories. All components that use the LanguageResolutionController will have real-time updates for locale updates from the storybook decorator picker (i.e. sp-number-field). If this behavior is desired I can create a separate PR and land this quicker in SWC.

Do you need more discussion on this @mizgaionutalexandru . I think adding a @state() decorator will reconcile the changes as mentioned by Ruben on the channel

Yes, I would love to clarify this because there are 2 different topics. The state() decorator works for 'normal app environments' let's say, and that's how the storybook decorator is implemented but due to the fact that in storybook the story function gets called before the sp-theme's constructor from the wrapper decorator gets the chance to add the event listener that is needed for the language resolver.
I found that those 2 approaches would fix the issue in the storybook environment.

Rocss · 2024-08-14T12:36:15Z

I found the following edge case while manual testing this feature:

Go to https://imizga-calendar-keyboard--spectrum-web-components.netlify.app/storybook/?path=/story/calendar--min-and-max-dates
Using the keyboard, go ahead and select a different date besides today, for example, tomorrow
Now using the arrows in the header, go to next month
Return
Try to select a day again => No longer working, calendar days are no longer into tab order.

mizgaionutalexandru · 2024-08-16T11:39:34Z

I found the following edge case while manual testing this feature:

Thank you for checking this out! I think the issue is fixed now, could you double check please? @Rocss

Rajdeepc · 2024-08-19T06:55:56Z

I would also like to add the below functionality.

Go to https://imizga-calendar-keyboard--spectrum-web-components.netlify.app/storybook/?path=/story/calendar--min-and-max-dates
Tab to one date. Then using navigation keys try to go to another date for next/previous month. This is not working.
Users should be able to freely move to any date in any month by navigation keys

Example: https://atlassian.design/components/calendar/examples

mizgaionutalexandru · 2024-08-19T07:28:55Z

Tab to one date. Then using navigation keys try to go to another date for next/previous month. This is not working.

Users should be able to freely move to any date in any month by navigation keys

Thank you for your input on this one!
Just to be clear, if you're talking about adding a new keyboard shortcut to be able to navigate to the previous month more easily like Page Up here, I can see that working although the RFC does not include this.

If you're saying that unavailable/disabled days should be navigable, I think the current approach that aligns with React Spectrum is better, this way the constraints the user has makes him pick an available day, as per the RFC. You can check out the React Spectrum approach here.
Also, worth mentioning is that using the DateTimePicker the user will be able to type-in an unavailable day, this PR is only for the Calendar component.

What are your thoughts on this?

mizgaionutalexandru · 2024-08-19T11:17:19Z

packages/calendar/src/Calendar.ts

+        }
+
+        if (this.currentDate.month !== initialMonth)
+            this.setWeeksInCurrentMonth();


This call, as well the ones from handlePreviousMonth and handleNextMonth will be removed from here and added in a special check inside willUpdate, I think this makes more sense and it's less error prone.

Rajdeepc · 2024-08-19T16:11:59Z

Tab to one date. Then using navigation keys try to go to another date for next/previous month. This is not working.

Users should be able to freely move to any date in any month by navigation keys

Thank you for your input on this one! Just to be clear, if you're talking about adding a new keyboard shortcut to be able to navigate to the previous month more easily like Page Up here, I can see that working although the RFC does not include this.

If you're saying that unavailable/disabled days should be navigable, I think the current approach that aligns with React Spectrum is better, this way the constraints the user has makes him pick an available day, as per the RFC. You can check out the React Spectrum approach here. Also, worth mentioning is that using the DateTimePicker the user will be able to type-in an unavailable day, this PR is only for the Calendar component.

What are your thoughts on this?

I was looking for this rovingtabindex functionality on the date items. Please check how we can navigate through months from date

datepicker.mov

* feat(calendar): keyboard navigation * fix: focus on disabled dates * refactor: storybook absolute Dates for VRTs * chore: update golden img hash * fix(calendar): focusable day management when changing months

mizgaionutalexandru force-pushed the imizga/calendar-keyboard branch 2 times, most recently from 25416fd to 774ac04 Compare August 12, 2024 10:36

feat(calendar): keyboard navigation

a05159f

mizgaionutalexandru force-pushed the imizga/calendar-keyboard branch from 08b7a46 to a05159f Compare August 12, 2024 10:42

chore: fix todo message

bba0ffa

mizgaionutalexandru changed the title ~~Imizga/calendar keyboard~~ feat(calendar): keyboard navigation Aug 12, 2024

mizgaionutalexandru commented Aug 12, 2024

View reviewed changes

mizgaionutalexandru marked this pull request as ready for review August 12, 2024 11:41

mizgaionutalexandru requested review from AndreiBaicu26, Rajdeepc, Rocss and loredanaspataru August 12, 2024 11:42

mizgaionutalexandru added 2 commits August 12, 2024 17:08

fix: focus on disabled dates

9cf1fac

refactor: storybook absolute Dates for VRTs

3e12b02

mizgaionutalexandru commented Aug 12, 2024

View reviewed changes

chore: update golden img hash

25e48a4

mizgaionutalexandru force-pushed the imizga/calendar-keyboard branch from ee8f9f1 to 25e48a4 Compare August 14, 2024 06:51

fix(calendar): focusable day management when changing months

8f114cb

mizgaionutalexandru commented Aug 19, 2024

View reviewed changes

mizgaionutalexandru mentioned this pull request Aug 23, 2024

feat: locale picker in the storybook's decorator #4687

Merged

16 tasks

Rajdeepc approved these changes Aug 27, 2024

View reviewed changes

mizgaionutalexandru merged commit c17af0e into feature/date-time-picker Aug 28, 2024

mizgaionutalexandru deleted the imizga/calendar-keyboard branch August 28, 2024 08:12

feat(calendar): keyboard navigation #4667

feat(calendar): keyboard navigation #4667

Uh oh!

Conversation

mizgaionutalexandru commented Aug 12, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Related issue(s)

Motivation and context

How has this been tested?

Screenshots (if appropriate)

Types of changes

Checklist

Best practices

Uh oh!

github-actions bot commented Aug 12, 2024

Branch preview

Uh oh!

github-actions bot commented Aug 12, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Lighthouse scores

Transfer Size

Request Count

Uh oh!

mizgaionutalexandru Aug 12, 2024 • 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!

mizgaionutalexandru Aug 12, 2024

Choose a reason for hiding this comment

Uh oh!

mizgaionutalexandru Aug 12, 2024 • 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!

mizgaionutalexandru Aug 15, 2024 • 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!

mizgaionutalexandru Aug 16, 2024 • 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!

Rajdeepc Aug 19, 2024 • 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!

mizgaionutalexandru Aug 19, 2024 • 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!

Rocss commented Aug 14, 2024

Uh oh!

mizgaionutalexandru commented Aug 16, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Rajdeepc commented Aug 19, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mizgaionutalexandru commented Aug 19, 2024

Uh oh!

mizgaionutalexandru Aug 19, 2024

Choose a reason for hiding this comment

Uh oh!

Rajdeepc commented Aug 19, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

mizgaionutalexandru commented Aug 12, 2024 •

edited

Loading

github-actions bot commented Aug 12, 2024 •

edited

Loading

mizgaionutalexandru Aug 12, 2024 •

edited

Loading

mizgaionutalexandru Aug 12, 2024 •

edited

Loading

mizgaionutalexandru Aug 15, 2024 •

edited

Loading

mizgaionutalexandru Aug 16, 2024 •

edited

Loading

Rajdeepc Aug 19, 2024 •

edited

Loading

mizgaionutalexandru Aug 19, 2024 •

edited

Loading

mizgaionutalexandru commented Aug 16, 2024 •

edited

Loading

Rajdeepc commented Aug 19, 2024 •

edited

Loading

Rajdeepc commented Aug 19, 2024 •

edited

Loading