-
Notifications
You must be signed in to change notification settings - Fork 5.3k
[WIP] Calendar / Datepicker: Accessibility changes #1829
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
base: datepicker
Are you sure you want to change the base?
Conversation
This will improve the experience for screen reader users, since the role of the button will be spoken as "button", rather than "toggle button".
@jongund FYI |
@jongund @scottgonzalez @jzaefferer Any feedback on this PR? |
I will look at it and give you some feedback by Wednesday.
Jon
|
Felix,
I think this is looking pretty good.
I think you should truncate the month and year information from the aria-label in the “role=gridcell”, since you are also including a aria-describedby to the month year information:
CHANGE:
<td role=”gridcell” aria-label=”Sunday, 11/26/17” …
TO:
<td role=”gridcell” aria-label=”Sunday, 11”
If you make this change I would be happy to send it out to some screen reader users for additional testing.
Jon
|
@jongund Hey Jon, thanks for the feedback and sorry for the late response. Not quite sure what you want me to do as we removed the Hope you had a good start of the new year! |
Just discovered the https://github.com/jongund/jquery-ui-datepicker/blob/master/aria-markup-example.html#L67 |
@fnagel I see you mention setting the aria-label to '11/26/17' in an earlier comment. I'm just wondering whether this format is local sensitive? (1/12 might mean the 12th of January to you but to me it means the first of December). PS: Thanks for the feedback on my PR. I'll close it now. |
@NigelCunningham Yes, that date value is locale specific. Anyway, that has been removed in 931cb89 but I'm not quite sure about this, see #1829 (comment) |
@jongund A gentle reminder :-) |
It would be better to use the named date like, "June 6th, 2018" rather than "6/6/2017". It would make it easier to understand for a screen reader user, they do not have to decode the "/" characters. |
@jongund So we want to keep the aria-label attribute? Asking becasue you wanted me to remove this (if I did understand you correctly), see here:
Please see my comments here #1829 (comment) and #1829 (comment) |
@jongund A gentle reminder :-) |
@jongund Another gentle reminder |
Sorry for the delay in getting back to you. This is the URL I used for testing, let me know if it is the right one to use: I have updated my example page based on some testing I did with NVDA, JAWS and VoiceOver. https://github.com/jongund/jquery-ui-datepicker/blob/master/aria-markup-example.md The items that need to change are:
Let me know when you make these changes and I will do some more testing. |
Thanks for the feedback! The URL in general is correct, but will only reflect merged changes to the https://github.com/jquery/jquery-ui/pull/1829/commits Did you take a look at those changes yet? |
Short summary seems to be:
I will implement those as soon I have some feedback to the current changes. |
@fnagel the tests failed here. Can you check what went wrong? |
@dmethvin If I remember correct, the tests have not been adjusted to those WIP a11y changes yet. The general data getter / setter error is an timezone issue Scott wanted to fix but did not before he dropped out. See section "Next Steps / ToDo (Felix)" at this wiki page: https://jqueryui.pbworks.com/w/page/12137778/Datepicker |
This PR implements Jon's feedback as proposed here: https://github.com/jongund/jquery-ui-datepicker
Summery
"aria-popup" seesm not existing. We already add "aria-haspopup" to the input field.
Changed. This might have some side effects but that's not ok for now.
Removed those attributes.
Tabindex was already set for the button element only, not for the cell. Removed the aria attributes from the cell.
Changed!
Selecting date ranges not supported yet.
Changed!
Changed!
Visual styling should not be part of this PR. @scottgonzalez Any idea how to tackle this? There have been multiple complaints about this...