-
Notifications
You must be signed in to change notification settings - Fork 0
feat(datepicker): add aria labels and aria level #2
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: master
Are you sure you want to change the base?
feat(datepicker): add aria labels and aria level #2
Conversation
template/datepicker/day.html
Outdated
| <th colspan="{{::5 + showWeeks}}"><button id="{{::uniqueId}}-title" role="heading" aria-live="assertive" aria-atomic="true" type="button" class="btn btn-default btn-sm uib-title" ng-click="toggleMode()" ng-disabled="datepickerMode === maxMode" tabindex="-1"><strong>{{title}}</strong></button></th> | ||
| <th><button type="button" class="btn btn-default btn-sm pull-right uib-right" ng-click="move(1)" tabindex="-1"><i aria-hidden="true" class="glyphicon glyphicon-chevron-right"></i><span class="sr-only">next</span></button></th> | ||
| <th><button type="button" class="btn btn-default btn-sm pull-left uib-left" ng-click="move(-1)" tabindex="-1" aria-label="{{::previousButtonLabel}}"><i aria-hidden="true" class="glyphicon glyphicon-chevron-left"></i><span class="sr-only">{{::previousButtonLabel}}</span></button></th> | ||
| <th colspan="{{::5 + showWeeks}}"><button id="{{::uniqueId}}-title" role="heading" aria-level="5" aria-live="assertive" aria-atomic="true" type="button" class="btn btn-default btn-sm uib-title" ng-click="toggleMode()" ng-disabled="datepickerMode === maxMode" tabindex="-1"><strong>{{title}}</strong></button></th> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure about using an aria-level of 5 here now that we don't even have the chronogolf context... I will investigate removing the heading role or alternative solutions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed aria level and heading, confirmed no major change to css
5332088 to
ec3b34b
Compare
ls-michael-smith
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Only thing I wonder is if we're going to get hit with some 'next year' vs 'next' issue.
@ls-michael-smith I don't want to expose 6 different configs (2 for each of month/year/years views), so I'm thinking of only using Previous/Next (so changing localization in the chronogolf repo). |
GOLF-3068
This PR adds:
No more role=heading and aria level

Aria labels passed as options

No aria label passed
