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

Improve Console screen-reader accessibility. #11602

Conversation

cjcenizal
Copy link
Contributor

Partially addresses #11520. Extracts some work from #11548.

  • Add aria-label for Console History entries.
  • Add aria-label and aria-labelledby to Console Request 'wrench' menu.

- Add aria-label for Console History entries.
- Add aria-label and aria-labelledby to Console Request 'wrench' menu.
<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.

@cjcenizal cjcenizal added Team:Platform-Design Team Label for Kibana Design Team. Support the Analyze group of plugins. Feature:Dev Tools v5.5.0 v6.0.0 labels May 4, 2017
Copy link
Member

@jbudz jbudz left a comment

Choose a reason for hiding this comment

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

Two small questions, looking good

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

Copy link
Contributor Author

@cjcenizal cjcenizal May 8, 2017

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

<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

@cjcenizal cjcenizal merged commit 880648c into elastic:master May 11, 2017
@cjcenizal cjcenizal deleted the 11520/improvement/console-screen-reader-accessibility branch May 11, 2017 19:42
cjcenizal added a commit to cjcenizal/kibana that referenced this pull request May 11, 2017
- Add aria-label for Console History entries.
- Add aria-label and aria-labelledby to Console Request 'wrench' menu.
cjcenizal added a commit that referenced this pull request May 11, 2017
- Add aria-label for Console History entries.
- Add aria-label and aria-labelledby to Console Request 'wrench' menu.
snide pushed a commit to snide/kibana that referenced this pull request May 30, 2017
- Add aria-label for Console History entries.
- Add aria-label and aria-labelledby to Console Request 'wrench' menu.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants