-
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
[Monitoring/Accessibility] Remove title attributes from elements #18025
Comments
Original comment by @cjcenizal: Nice! We should also bear in mind that if an icon is used in the UI to supplement text, e.g. a button with both icon and text inside of it, then we can just use |
Original comment by @tsullivan: action item: get a list of how many title attributes are being used |
Original comment by @tsullivan: For the React components that use a title attribute, we can replace them with the Tooltip component: LINK REDACTED But then what we need to do is make that component accessible |
Original comment by @tsullivan:
Tricky to do with our current tooltip library because of: pivotal-cf/pivotal-ui#426 |
Original comment by @cjcenizal: @tsullivan It's very fortuitous that you mention tooltips, because this is actually a significant blocker for our effort to incorporate EUI into Kibana. I think the solution to this would be to add a new Tooltip component to EUI (and then port it to the Kibana UI framework). Would you be interested in taking this on? The pivotal tooltip source code looks pretty straightforward. I think we could copy it to EUI, reformat it to be consistent with our JS style, and make the accessibility improvements we need. @snide could help from a design side. |
Original comment by @snide: I can certainly help from the design side. If someone wants to give me a working tooltip in our docs I can style 'er up. Only request I'd have looking at that tooltip system is pass along a title AND body prop. We often need a title to give context to the content of the body (especially for long tooltips). Anyways, happy to help. |
Original comment by @cjcenizal: Just got word that @nreese is going to take a swing at this! 🥇 |
Original comment by @tsullivan: Cool! It wouldn't have been an easy commitment for me to make. But I can certainly help on the review and testing side. |
Original comment by @tsullivan: AFAIK we're still waiting on an EUI component for tooltips with accessibility baked in that we can integrate to close this issue. |
Original comment by @nreese: @tsullivan Tooltips are now a part of EUI as of 0.0.26, LINK REDACTED and they are accessible |
Original comment by @tsullivan:
Not sure how I missed this earlier, but this is awesome! |
This should be very easy to implement once more views are Reactified. We should focus the next Reactification efforts on any view that needs a tooltip to make this go quicker |
Depends on #19052 |
@tsullivan dependency was closed, can we close this one? |
@AlonaNadler I'm not sure if title attributes are still used in the Monitoring UI. If so then this should stay open. |
@elastic/kibana-monitoring do you know if this is still relevant? if not, when did it stop being relevant 6.4? |
Checking again with @elastic/stack-monitoring, new components are used in the UI that make this issue n/a, right? |
There looks to be just one usage left that we need to remove |
Original comment by @tsullivan:
We're using title attributes in various places in the Monitoring UI - primarily on icons. For accessibility, we should remove these and use an
aria-label
attribute instead.Perhaps at the same time, we would want to add a tooltip to these areas.
cc @cjcenizal
Depends on #19052
The text was updated successfully, but these errors were encountered: