Fix #4446: ✨ Added support for up and down arrow key movement in the year picker #4698
Conversation
… of the date picker - Prevent the default scroll movement when the arrow key is pressed Closes Hacker0x01#4446
balajis-qb
force-pushed
the
issue-4446/fix/support-up-down-key-navigation-for-year-picker
branch
from
8a1ae17
to
01dbc14
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #4698 +/- ##
==========================================
+ Coverage 97.00% 97.12% +0.12%
==========================================
Files 28 28
Lines 2641 2682 +41
Branches 1119 1149 +30
==========================================
+ Hits 2562 2605 +43
+ Misses 79 77 -2 ☔ View full report in Codecov by Sentry. |
![pullrequest[bot]](https://avatars.githubusercontent.com/in/3434?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ This pull request was sent to the PullRequest network for review. Expert reviewers are now being matched to your request based on the code's requirements. Stay tuned!
What to expect from this code review:
- Comments posted to any areas of potential concern or improvement.
- Detailed feedback or actions needed to resolve issues that are found.
- Turnaround times vary, but we aim to be swift.
@balajis-qb you can click here to see the review status or cancel the code review job - or - cancel by adding [!pr] to the title of the pull request.
There was a problem hiding this comment.
This improvement looks good to me. It mirrors the approach taken in day.jsx and month.jsx for handling "ArrowUp" and "ArrowDown". The tests are well thought-out and cover a variety of cases. I don't see any issues here.
Thanks for taking care of this!
Reviewed with ❤️ by PullRequest
There was a problem hiding this comment.
The change looks good - good job adding tests for the new logic.
Reviewed with ❤️ by PullRequest
| if (key !== "Tab") { | ||
| // preventDefault on tab event blocks focus change | ||
| e.preventDefault(); | ||
| } | ||
|
|
||
| if (!this.props.disabledKeyboardNavigation) { | ||
| switch (key) { |
There was a problem hiding this comment.
I don't think we need a default case in this code block. If the pressed key is Tab, then we're not blocking the default operation like month picker, else we're blocking the default behaviour just to prevent the outside container to auto-scroll on arrow key press. Let me know if you need any more changes.
There was a problem hiding this comment.
PullRequest reviewed the updates made to #4698 since our last review was posted. This includes comments that have been posted by non-PullRequest reviewers. No further issues were found.
Reviewed by:
Description
Linked issue: #4446
Problem
The Year Picker doesn't support the up and down key navigation like the month picker and the date picker.
Changes
This PR adds support for updating the selected year in the year picker using the up and down arrow keys. It also blocks the default page movement on the press of arrow keys.
Contribution checklist