jquery-ui icon indicating copy to clipboard operation
jquery-ui copied to clipboard

[WIP] Calendar / Datepicker: Accessibility changes

Open fnagel opened this issue 8 years ago • 18 comments

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 avatar Aug 26 '17 13:08 fnagel

@jongund FYI

fnagel avatar Aug 26 '17 13:08 fnagel

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

fnagel avatar Dec 11 '17 10:12 fnagel

I will look at it and give you some feedback by Wednesday.

Jon

jongund avatar Dec 11 '17 22:12 jongund

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:

TO: If you make this change I would be happy to send it out to some screen reader users for additional testing.

Jon

jongund avatar Dec 13 '17 20:12 jongund

@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 avatar Jan 08 '18 22:01 fnagel

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

fnagel avatar Jan 08 '18 22:01 fnagel

@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 avatar Jan 19 '18 08:01 NigelCunningham

@NigelCunningham Yes, that date value is locale specific. Anyway, that has been removed in 931cb89b3876b44e6479f5f07af948433365c4e3 but I'm not quite sure about this, see https://github.com/jquery/jquery-ui/pull/1829#issuecomment-356117785

fnagel avatar Feb 13 '18 11:02 fnagel

@jongund A gentle reminder :-)

fnagel avatar Feb 13 '18 11:02 fnagel

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 avatar Feb 13 '18 18:02 jongund

@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 https://github.com/jquery/jquery-ui/pull/1829#issuecomment-356117785 and https://github.com/jquery/jquery-ui/pull/1829#issuecomment-356119112

fnagel avatar Feb 13 '18 21:02 fnagel

@jongund A gentle reminder :-)

fnagel avatar Mar 05 '18 08:03 fnagel

@jongund Another gentle reminder

fnagel avatar Mar 19 '18 09:03 fnagel

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.

jongund avatar Mar 19 '18 17:03 jongund

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 avatar Mar 21 '18 09:03 fnagel

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.

fnagel avatar Mar 21 '18 09:03 fnagel

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

dmethvin avatar Aug 01 '19 00:08 dmethvin

@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 avatar Sep 05 '19 07:09 fnagel