Skip to content
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

a11y(DatePicker): move year and month selects from caption to navbar (fixes tab order and a11y failure) #6025

Conversation

bvandercar-vt
Copy link
Contributor

@bvandercar-vt bvandercar-vt commented Mar 15, 2023

Fixes #6058

Reason:

  • Fix tab order of datepicker navbar
  • Fix A11y failure issue-- grid caption should not contain interactive elements

Checklist

  • Includes tests
  • [N/A] Update documentation

Changes proposed in this pull request:

Move Year and Month selects out of React-Day-Picker caption (renderCaption) to navbar (renderNavbar). Fixes tab order of these elements with the navigation arrows, and fixes accessibility issue-- a grid caption should not contain interactive elements. The caption is within the role="grid" element, and the grid's caption should not contain the selects, because it's supposed to be only that, a caption for the grid.

A11y failures aside, these components generally make more sense as a "navigation" component than a "caption".

Screenshot

Tab Order Before and After:
ScreenRecorderProject1 ScreenRecorderProject2

Proof that is visually and functionally same as before:
ScreenRecorderProject3_2

aXe failure:

image

@adidahiya
Copy link
Contributor

grid caption should not contain interactive elements

@bvandercar-vt do you have documentation you can reference for this claim?

@bvandercar-vt
Copy link
Contributor Author

bvandercar-vt commented Mar 24, 2023

grid caption should not contain interactive elements

@bvandercar-vt do you have documentation you can reference for this claim?

Updated the PR description with the aXe failure and more detailed info.

@bvandercar-vt bvandercar-vt changed the title fix(DatePicker): move year and month selects from caption to navbar a11y(DatePicker): move year and month selects from caption to navbar (fixes tab order and a11y failure) Apr 18, 2023
@bvandercar-vt
Copy link
Contributor Author

@adidahiya I added GIFs to PR description

@bvandercar-vt
Copy link
Contributor Author

@adidahiya refactored this

@adidahiya
Copy link
Contributor

thanks for your effort so far on this PR @bvandercar-vt. I feel like this change has the chance to be breaking in some very rare cases since we try to guarantee no DOM layout breaks in Blueprint components. There are also some major changes to date picker caption / navbar rendering coming in react-day-picker v8 (see #5935). For these reasons, I feel like it's probably best to wait to make this change until we upgrade to react-day-picker v8, where we expect to make some DOM breaking changes anyway. This should be possible in the next few weeks / month.

@adidahiya
Copy link
Contributor

adidahiya commented Oct 25, 2023

@bvandercar-vt we are now using react-day-picker v8 in the "V3" components in @blueprintjs/datetime2. All a11y fixes should be made there.

Most of the code paths now use react-day-picker's caption rendering, so if there are a11y issues there, you should bring it up with that library maintainer.

One code path goes through a custom caption renderer that lives inside Blueprint code, though: non-contiguous date range pickers with multiple months. Those utilize a custom <DatePicker3Caption> component.

I'm going to close this PR for now, and if you want to pick up the a11y work, you may do so in a new PR. Thanks!

@adidahiya adidahiya closed this Oct 25, 2023
@bvandercar-vt bvandercar-vt deleted the bvandercar/datepicker/move-month-year-selects-out-of-caption branch November 7, 2024 19:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

incorrect tab order in DatePicker header, DatePicker also failing aXe a11y checks
2 participants