-
Notifications
You must be signed in to change notification settings - Fork 2.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(DateRangeInput3) Add keyboard accessibility #7080
Conversation
Generate changelog in
|
feat(DateRangeInput3) Add keyboard accessibilityBuild artifact links for this commit: documentation | landing | table | demoThis is an automated comment from the deploy-preview CircleCI job. |
Fix overlapping datesBuild artifact links for this commit: documentation | landing | table | demoThis is an automated comment from the deploy-preview CircleCI job. |
Fix same date overlapBuild artifact links for this commit: documentation | landing | table | demoThis is an automated comment from the deploy-preview CircleCI job. |
const isForwardArrowKey = arrowKey === "ArrowRight" || arrowKey === "ArrowDown"; | ||
// If the arrow key direction is in the same direction as the boundary, then moving that way will not create an | ||
// overlapping date range | ||
if (isForwardArrowKey === (boundary === Boundary.END)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The equivalence of a boolean to a boolean here reads a bit weird. Can we change this to something like:
const isForwardArrowKey = arrowKey === "ArrowRight" || arrowKey === "ArrowDown";
const isEndBoundary = boundary === Boundary.END;
// If the arrow key direction is in the same direction as the boundary, then moving that way will not create an
// overlapping date range
if (isForwardArrowKey && isEndBoundary) {
return shiftDateByArrowKey(otherBoundary, arrowKey);
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We still need equality here for this to work properly, isForwardArrowKey && isEndBoundary
is not equivalent to isForwardArrowKey === isEndBoundary
. I thought about doing something like:
const arrowKeyDirection = arrowKey === "ArrowRight" || arrowKey === "ArrowDown" ? "forward" : "backward";
const boundaryDirection = boundary === Boundary.END ? "forward" : "backward";
if (arrowKeyDirection === boundaryDirection) {
But this is just a lot of extra stuff for the same thing and minimal readability improvement. I'll pull isEndBoundary
out I like that, but I'm not sure how to make it much more readable beyond that.
Changes proposed in this pull request:
This PR adds improved keyboard accessibility to the
DateRangeInput3
component. In particular we align with other date range picker so that when one of the input fields is focused, the user can then use their arrow keys to navigate the calendar and make selections. This properly respects min and max dates and allow single selection only and properly navigates the calendar view based on user selections.Screenshot
date-range-input.mp4