Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Sign up
Appearance settings

Add keyboard accessibility#757

Open
vahissan wants to merge 4 commits into
arqex:master from
vahissan:keyboard-accessibility
Open

Add keyboard accessibility #757
vahissan wants to merge 4 commits into
arqex:master from
vahissan:keyboard-accessibility

Conversation

@vahissan

@vahissan vahissan commented Dec 1, 2020

Copy link
Copy Markdown
Contributor

Description

  • Added ability to navigate the calendar using the TAB key
  • Added ability to close the calendar by pressing the ESC key when focused on the calendar or the input field
  • Added ability to open the calendar by pressing the ArrowDown key (useful when it was closed by ESC key)

Motivation and Context

Fixes #415 (at least partially)
I was using the library and would like to have the ability to operate the date picker without a mouse. I'm sure many others would like it too.

Checklist

[ x ] I have not included any built dist files (us maintainers do that prior to a new release)
[ x ] I have added tests covering my changes
[ x ] All new and existing tests pass
[ ] My changes required the documentation to be updated
 [ ] I have updated the documentation accordingly
 [ ] I have updated the TypeScript 1.8 type definitions accordingly
 [ ] I have updated the TypeScript 2.0+ type definitions accordingly

connor-baer and emilia-friedberg reacted with heart emoji

vahissan commented Dec 1, 2020

Copy link
Copy Markdown
Contributor Author

These are the things from the original issue which I didn't add in this PR:

  • aria-label attributes for the td cells: I don't think it's required because the screen readers pickup the text inside the td elements without issue.
  • arrow key navigation within the calendar: I think it would impact the performance. Also, I wanted to add the basic functionality first, and maybe improve it later.

vahissan commented Dec 1, 2020

Copy link
Copy Markdown
Contributor Author

CI build failed due to these reasons:

  1. There's a peer dependency check failing on npm install due to npm v7's breaking changes.
  2. Unit tests failed because they depend on the built dist files.

arqex commented Dec 1, 2020

Copy link
Copy Markdown
Owner

Hey @vahissan !

Thanks for this great PR, I need to find some time to review it :)

If you know what's failing with the v7 of npm, can create a different PR with the fix and we merge it before this one?

vahissan commented Dec 1, 2020 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Thanks @arqex !

Sure, I'll take a look. Adding --legacy-peer-deps to npm install in CI is a workaround if you're interested.

EDIT: created PR #758

vahissan commented Dec 4, 2020 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

CI build failed due to these reasons:

  1. There's a peer dependency check failing on npm install due to npm v7's breaking changes.
  2. Unit tests failed because they depend on the built dist files.

Reason 1 is resolved. But CI still fails due to reason 2. Looks like CI does not create a new build before running unit tests.

Copy link
Copy Markdown
Contributor Author

Hey @arqex !

I know you must be busy. Just wanted to send a reminder in case you forgot.

Copy link
Copy Markdown

Hey @arqex
Can please merge this or add keyboard-accessibility feature, very badly need this feature.
emilia-friedberg and FerdiStro reacted with thumbs up emoji

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Keyboard accessibility

AltStyle によって変換されたページ (->オリジナル) /