-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
Improve Management screen-reader accessibility. #11601
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -73,12 +73,14 @@ | |
class="kuiTab" | ||
ng-repeat="editSection in editSections" | ||
ng-class="{ 'kuiTab-isSelected': state.tab === editSection.index }" | ||
title="{{ editSection.title }}" | ||
ng-click="changeTab(editSection)" | ||
data-test-subj="tab-{{ editSection.index }}" | ||
> | ||
{{ editSection.title }} | ||
<span data-test-subj="tab-count-{{ editSection.index }}"> | ||
<span | ||
data-test-subj="tab-count-{{ editSection.index }}" | ||
aria-label="{{:: editSection.count + ' ' + editSection.title}}" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I tried to make this label a little more informative by including both the count and the title. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Silly question, but how do I get the aria-label to display? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's only accessible to screen-readers, so you'll have to use a screen reader to read it aloud to you. I'm on OS X so I'm just using the Voice Over app which ships with it. I think for Windows, NVDA is really popular (and it's free). Here's a guide on how to use it. |
||
> | ||
({{ editSection.count }}) | ||
</span> | ||
</button> | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -48,12 +48,11 @@ <h1 class="kuiTitle"> | |
class="kuiTab kbn-management-tab" | ||
ng-class="{ 'kuiTab-isSelected': state.tab === service.title }" | ||
ng-repeat="service in services" | ||
title="{{ service.title }}" | ||
ng-click="changeTab(service)" | ||
> | ||
{{ service.title }} | ||
<small> | ||
({{service.data.length}}<span ng-show="service.total > service.data.length"> of {{service.total}}</span>) | ||
<small aria-label="{{:: service.data.length + ' of ' + service.total + ' ' + service.title }}"> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. To make this fully make sense, I combined the visible number of services, the total, and the kind of service. |
||
({{service.data.length}}<span ng-show="service.total > service.data.length"> of {{service.total}}</span>) | ||
</small> | ||
</button> | ||
</div> | ||
|
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.
We should never use
title
, and this element contains text so it's already accessible.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.
Out of curiosity: why is using
title
bad? Not as part of this PR but it might be good to include this guideline (with rationale) in our HTML style guide.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.
Never mind, I found your link in the issue for this PR: https://www.paciellogroup.com/blog/2012/01/html5-accessibility-chops-title-attribute-use-and-abuse/. I'm do think we should add this guideline to our HTML style guide (as part of a separate PR, if you prefer).
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 noticed that the article is from January 2012, over 5 years ago. The main reason it suggests not using
title
is that not all browsers support it. I checked https://caniuse.com/#search=title and that seems to no longer be the case. Thoughts?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.
Never mind (again!) 😛. I misunderstood the article (and the W3C recommendation it links to). It's not so much that browsers don't support
title
, it's that its not very friendly on devices that don't have a keyboard or mouse (screenreaders, etc.). +1 for not using it and documenting it in our HTML style guide.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.
Haha ok I'll open a PR. Thanks man.