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 Console screen-reader accessibility. #11602

Merged
merged 1 commit into from
May 11, 2017
Merged

Improve Console screen-reader accessibility. #11602

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
15 changes: 12 additions & 3 deletions src/core_plugins/console/public/index.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,20 @@
</a>
</kbn-tooltip>
<span dropdown>
<a class="editor_action" dropdown-toggle href="#">
<i class="fa fa-wrench"></i>
<a
id="consoleRequestOptions"
Copy link
Member

Choose a reason for hiding this comment

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

is the plan to move ids towards camel case?

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 don't know if there's a formal plan for this but we're using camel case pretty much everywhere (e.g. test subject selectors, even CSS class names), so I think it makes sense to be consistent by applying camel case to IDs too.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Want me to add a note to the HTML style guide?

Copy link
Member

Choose a reason for hiding this comment

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

👍 works for me

class="editor_action"
dropdown-toggle href="#"
aria-label="Request options"
>
<span class="kuiIcon fa-wrench"></span>
</a>

<ul class="dropdown-menu" role="menu" aria-labelledby="split-button">
<ul
class="dropdown-menu"
role="menu"
aria-labelledby="consoleRequestOptions"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This attribute lets us to refer to another element on the screen, which will describe the element to screen readers using an aria-label attribute.

>
<li role="menuitem">
<a class="link" id="copy_as_curl">Copy as cURL</a>
</li>
Expand Down
5 changes: 3 additions & 2 deletions src/core_plugins/console/public/src/directives/history.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
ng-mouseenter="history.viewingReq = req"
ng-mouseleave="history.viewingReq = history.selectedReq"
ng-dblclick="history.restore(req)"
title="{{ req.description }}"
aria-label="{{:: 'Request: ' + history.describeReq(req) }}"
Copy link
Member

Choose a reason for hiding this comment

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

should this use aria-labelledby? ...If there is visible text labeling the element, use aria-labelledby instead..

Copy link
Contributor Author

@cjcenizal cjcenizal May 8, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

Hm, I think this is correct as-is. Based on my understanding,aria-labelledby is used to point to another element on the screen which contains text that labels the element, e.g. an input element which is labeled by a label element.

In this case, we technically don't even need aria-label because the element already contains text which is self-descriptive. But I thought that the text might lack a little context, so I created an aria-label so we can prepend the word "Request", just to make it a little clearer. What do you think of this?

Copy link
Member

Choose a reason for hiding this comment

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

i did some digging, my notes below for reference. i'm not going to nit too much, there's room for opinions with the request prefix.

https://github.com/w3c/aria/blob/master/aria/aria.html#L9578:
relevant portions:
The purpose of aria-labelledby is the same as that of aria-label. It provides the user with a recognizable name of the object
and
If the interface is such that it is not possible to have a visible label on the screen, authors should use aria-label and should not use aria-labelledby

class="list-group-item history-req"
>
{{ history.describeReq(req) }}
Expand All @@ -25,7 +25,8 @@
<button
class="kuiButton kuiButton--primary"
ng-disabled="!history.selectedReq"
ng-click="history.restore()">
ng-click="history.restore()"
>
Apply
</button>
</div>