Skip to content

[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

Draft
wants to merge 7 commits into
base: datepicker
Choose a base branch
from

Conversation

fnagel
Copy link
Member

@fnagel fnagel commented Aug 26, 2017

This PR implements Jon's feedback as proposed here: https://github.com/jongund/jquery-ui-datepicker

Summery

The date textbox must have aria-popup="true" attribute.

"aria-popup" seesm not existing. We already add "aria-haspopup" to the input field.

Keyboard focus should not move into the "grid" until the user presses "up" or "down" arrow keys while in the textbox.

Changed. This might have some side effects but that's not ok for now.

When a table element has role=grid attribute the default roles for tr or td elements are row and griodcell, no need to add these roles in markup to the tr and td elements.

Removed those attributes.

Remove tabindex=-1, aria-label and aria-describedby from gridcells from gridcells with buttons.
Since you are using aria-activedescendant you should set the tabindex of the button elements to tabindex=-1.

Tabindex was already set for the button element only, not for the cell. Removed the aria attributes from the cell.

Change role=alert for month information to role=status.

Changed!

Add an offscreen live region (e.g. role="status") to indicate the selected date range (e.g. March 4th through March 11th selected) when a date range is marked.

Selecting date ranges not supported yet.

Instead of using aria-select on the td elements, use aria-pressed="true" on the selected button elements, this will work better with screen readers.

Changed!

Remove the aria-pressed attribute from dates that are not selected, again this will improve the experience for screen reader users, since the role of the button will be spoken as "button", rather than "toggle button".

Changed!

Keyboard focus styling is also still an issue, the date with keyboard focus should has a very easy to see border

Visual styling should not be part of this PR. @scottgonzalez Any idea how to tackle this? There have been multiple complaints about this...

@fnagel fnagel self-assigned this Aug 26, 2017
@fnagel
Copy link
Member Author

fnagel commented Aug 26, 2017

@jongund FYI

@fnagel
Copy link
Member Author

fnagel commented Dec 11, 2017

@jongund @scottgonzalez @jzaefferer Any feedback on this PR?

@jongund
Copy link

jongund commented Dec 11, 2017 via email

@jongund
Copy link

jongund commented Dec 13, 2017 via email

@fnagel
Copy link
Member Author

fnagel commented Jan 8, 2018

@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 role=gridcell attribute and the aria-label attribute with the "Sunday, 11/26/17" value. Did I misunderstand your former change requests?

Hope you had a good start of the new year!

@fnagel fnagel requested a review from arschmitz January 8, 2018 22:31
@fnagel
Copy link
Member Author

fnagel commented Jan 8, 2018

Just discovered the aria-label is still there in your example markup, but only for a single day cell. Is that on purpose?

https://github.com/jongund/jquery-ui-datepicker/blob/master/aria-markup-example.html#L67

@NigelCunningham
Copy link

@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.

@fnagel
Copy link
Member Author

fnagel commented Feb 13, 2018

@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)

@fnagel
Copy link
Member Author

fnagel commented Feb 13, 2018

@jongund A gentle reminder :-)

@jongund
Copy link

jongund commented Feb 13, 2018

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.

@fnagel
Copy link
Member Author

fnagel commented Feb 13, 2018

@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:

Remove tabindex=-1, aria-label and aria-describedby from gridcells from gridcells with buttons.

Taken from: https://github.com/jongund/jquery-ui-datepicker/commit/da1e5c57a6619d2c66244301835dbd04b0f684e9#diff-04c6e90faac2675aa89e2176d2eec7d8L7

Please see my comments here #1829 (comment) and #1829 (comment)

@fnagel
Copy link
Member Author

fnagel commented Mar 5, 2018

@jongund A gentle reminder :-)

@fnagel
Copy link
Member Author

fnagel commented Mar 19, 2018

@jongund Another gentle reminder

@jongund
Copy link

jongund commented Mar 19, 2018

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:
https://rawgit.com/jquery/jquery-ui/datepicker/demos/datepicker/default.html

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:

  • remove aria-label from the TD elements
  • move aria-described to the BUTTON elements
  • Change labels for previous and next moth buttons to "Previous Month" and "Next Month" using aria-label attribute on the buttons

Let me know when you make these changes and I will do some more testing.

@fnagel
Copy link
Member Author

fnagel commented Mar 21, 2018

Thanks for the feedback!

The URL in general is correct, but will only reflect merged changes to the datepicker branch -- not Pull Requests like this one. Instead take a look here to review my changes:

https://github.com/jquery/jquery-ui/pull/1829/commits
https://github.com/jquery/jquery-ui/pull/1829/files
or download directly: https://github.com/jquery/jquery-ui/archive/master.zip

Did you take a look at those changes yet?

@fnagel
Copy link
Member Author

fnagel commented Mar 21, 2018

I have updated my example page based on some testing I did with NVDA, JAWS and VoiceOver.

Short summary seems to be:

  • Keep aria-describedby and tabindex=-1 for button elements (which reverts some your former recommendation)
  • Add role=status to the month info element
  • Use aria-label for next and prev month elements

I will implement those as soon I have some feedback to the current changes.

@dmethvin
Copy link
Member

dmethvin commented Aug 1, 2019

@fnagel the tests failed here. Can you check what went wrong?

@fnagel
Copy link
Member Author

fnagel commented Sep 5, 2019

@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

@fnagel fnagel changed the title Calendar / Datepicker: Accessibility changes [WIP] Calendar / Datepicker: Accessibility changes Dec 9, 2019
@fnagel fnagel marked this pull request as draft July 23, 2020 20:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

4 participants