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

Fix aria roles on weekday and month elements #276

Merged
merged 3 commits into from
Mar 8, 2017
Merged

Conversation

oigewan
Copy link

@oigewan oigewan commented Mar 7, 2017

Fixes: #277

Per https://www.w3.org/TR/wai-aria/roles#columnheader, the columnheader role should go on the equivalent of a <th> element and should be owned by something with the row role.

Also, per https://www.w3.org/TR/wai-aria/roles#rowgroup rowgroup elements should be owned by something with a grid role, so I moved that up as well.

However, this does pose a bit of an issue for people using the weekdayElement prop. Since you're not outputting the result of that in a wrapping div, people passing in an element on that prop will need to be sure to output that role themselves.

Another possible change is for the library to output the wrapping div with the classname and role. I think that would be a breaking change (resulting change in HTML would probably throw off some people's layout CSS), so I'm not attempting that here.

cc: @gpbl

@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling 09c6604 on oigewan:aria into c9ab28f on gpbl:master.

@oigewan oigewan changed the title Fix aria roles on weekday elements Fix aria roles on weekday and month elements Mar 7, 2017
@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling 5ca45ce on oigewan:aria into c9ab28f on gpbl:master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling 36fdfb7 on oigewan:aria into 9fb17f8 on gpbl:master.

@gpbl
Copy link
Owner

gpbl commented Mar 8, 2017

Awesome thanks for your contribution!

However, this does pose a bit of an issue for people using the weekdayElement prop

Sure, however this is left to the developer.

@gpbl gpbl merged commit f81357b into gpbl:master Mar 8, 2017
@gpbl
Copy link
Owner

gpbl commented Mar 9, 2017

Published as v5.2.0 🎉

@oigewan oigewan deleted the aria branch June 20, 2017 03:37
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.

3 participants