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.

Sign up for GitHub

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

Merged
merged 2 commits into from
May 10, 2017
Merged

Improve Management screen-reader accessibility. #11601

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1c47bf1
Improve Management screen-reader accessibility:
cjcenizal May 1, 2017
f8da8b1
Allow formatted label to be presented in lieu of default alt attribute.
cjcenizal May 9, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion .../kibana/public/management/sections/indices/create_index_pattern/create_index_pattern.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,6 @@
class="kuiLink"
href="https://en.wikipedia.org/wiki/ISO_week_date"
target="_blank"
title="Wikipedia: ISO Week Date"
Copy link
Contributor Author

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.

Copy link
Contributor

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.

Copy link
Contributor

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).

Copy link
Contributor

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?

Copy link
Contributor

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.

Copy link
Contributor Author

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.

translate="KIBANA-WIKI_ISO_WEEK_DATE"
></a>
</p>
Expand Down
6 changes: 4 additions & 2 deletions ...gins/kibana/public/management/sections/indices/edit_index_pattern/edit_index_pattern.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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}}"
Copy link
Contributor Author

Choose a reason for hiding this comment

The 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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Silly question, but how do I get the aria-label to display?

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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>
Expand Down
5 changes: 2 additions & 3 deletions src/core_plugins/kibana/public/management/sections/objects/_objects.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 }}">
Copy link
Contributor Author

Choose a reason for hiding this comment

The 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>
Expand Down
2 changes: 1 addition & 1 deletion src/ui/public/stringify/types/url.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ export function stringifyUrl(Private) {

switch (this.param('type')) {
case 'img':
return '<img src="' + url + '" alt="' + label + '" title="' + label + '">';
return `<img src="${url}" alt="A dynamically-specified image located at ${label}">`;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The original title was just the URL, which doesn't describe the image contents at all. Unfortunately, because this image is dynamically defined and AFAIK we don't support description metadata, the best we can do is to just tell screen readers about where this image came from.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

alt="A dynamically-specified image located at ${label}"

Are you sure label is the field that you want to display here? If I add a field formatter to the field, and specify a label, then all of the alt values are going to be identical. Did you want this to be url?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch! Thanks man.

default:
if (hit && hit.highlight && hit.highlight[field.name]) {
label = getHighlightHtml(label, hit.highlight[field.name]);
Expand Down