-
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
Add title and tooltip accessibility information to HTML style guide. #11655
Add title and tooltip accessibility information to HTML style guide. #11655
Conversation
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.
LGTM. Since this is change to the style guide, I'd suggest getting a couple more reviewers, preferably from different functional areas.
* https://www.paciellogroup.com/blog/2012/01/html5-accessibility-chops-title-attribute-use-and-abuse/ | ||
* https://www.deque.com/blog/text-links-practices-screen-readers/ | ||
|
||
### Tooltips |
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 how to apply these guidelines, is there a recommended way to implement tooltips like this?
Placement/showing/hiding should probably be handled by a directive, no?
We already have the tooltip directive from ui-boostrap, and the kbn-tooltip directive that wraps that, but neither of those follow these guidelines.
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 either, to be honest. We'd need to spend some time figuring out a solution to this as part of our accessibility initiative, but it's also not currently on our list of priorities. I think it's OK to have this in our guidelines even if we don't currently follow it or have a way to implement it -- the information is accurate, and worth implementing when possible or updating if we decide it's impossible to achieve.
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 uncomfortable putting rules in the style guide for which we don't yet have a clear path on implementation.
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.
@BigFunger Here's an example of how we'd implement a tooltip with this guidance: https://codepen.io/cjcenizal/pen/aWqRVQ. Does this help?
I think we really need to have this accessibility information somewhere that's accessible (sorry) to everyone, so that we know where we want to go, even if we're not there yet. It's just a way of defining a goal first, and then finding a path there second.
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.
What is the intended path in regards to the other two ways (besides the title
attribute) that are already being used within Kibana that @spalger mentioned?
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.
As far as I can tell, we have two options:
- Update each implementation to adhere to the guidelines.
- Replace each implementation with one which adheres to the guidelines.
We'd need to dedicate time to investigating each option and deciding which path to pursue.
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 have used aria-describedby
in the past with great success.
style_guides/html_style_guide.md
Outdated
|
||
<button aria-describedby="visualizationsTooltip"> | ||
Visualizations | ||
</button> | ||
``` |
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.
Maybe add something like...
If something is meant to be clickable, favor using a button
or a
tag before over the kui-accessible-click
directive or KuiKeyboardAccessible
component.
kui-accessible-click
are incredibly useful as they augment non-clickable elements with onkeypress
and onkeyup
handlers, allowing non-clickable elements to become keyboard accessible. However, button
and anchor
tags provide this functionality natively,
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.
Also
Adding elements to the tab index
To add any element to the tab index, i.e. making it focusable via non-sticky-mode keyboard navigation, it must have an id
attribute as well as a tabindex
attribute. If you don't know which number to use for the tab index, or if you simply want to add it to the general flow of the document, use tabindex="0"
. See also the updated style guide for how to properly construct ids.
@aphelionz Updated. Could you take another look, and share your thoughts on the tooltip conversation? Is this information accurate? Have you implemented tooltips like this before? |
cd2c862
to
d2a25a2
Compare
Per #11601 (comment)