Calendar: add ARIA labels#1476
Conversation
|
This looks fine to me. Any other aria gurus we should ping about this? Or pretty straight forward. |
|
Hey @vibhava, does this look correct to you? |
|
ARIA looks fine. Can we also fix where we're using clickable spans and not true buttons while touching this? |
|
@cschlechty should we be using buttons for all clickable elements in the calendar? (dates included). If so, that might be outside the scope of this quick fix. I'd rather get this in and split that other work out to another PR. |
| onKeyDown={ (ev: React.KeyboardEvent<HTMLElement>) => | ||
| this._navigateMonthEdge(ev, day.originalDate, weekIndex, dayIndex) } | ||
| aria-selected={ day.isSelected } | ||
| aria-label={ day.originalDate.toLocaleDateString() } |
There was a problem hiding this comment.
This I believe requires IE11, which may not work on IE10. Though we plan to not support IE10 in the near future, just to be safe here couldn't we just ask the user to pass this in? Or at least make sure that day.originalDate.toLocaleDateString exists before calling it?
dzearing
left a comment
There was a problem hiding this comment.
Could you just check for the method existence first? Also will day.originalDate ever be null or not a date?
|
Thank you for making the change. Buttons for the clickable divs like Chris mentioned seems like a great idea. Please feel free to won't fix/check with an expert/address in a separate change, but would it be appropriate to set autoFocus to true on the header (month and year name printed) when the dropdown opens so that tabbing would take me through all the contents of the calendar? I didn't think to hit shift tab because hitting tab twice again takes me out of it and closes the dropdown. I can see the benefit in setting focus to the current date too like you are currently doing and arrowing would take me to whichever month I want eventually. I apologise for the delayed response, I was out of the office. |
…c-react into calendar/add-aria-labels
|
Buttons replacing spans is moved to #1535 so this one looks good to merge. |
Pull request checklist
Description of changes
This change adds ARIA labels to day and month buttons within the Calendar component, using toLocaleString.
Focus areas to test
(optional)