[EuiBasicTable][EuiInMemoryTable] Improve tooltip screen-reader output in action button #9140
Conversation
…n action button when `name` and `description` are the same, the tooltip should be visual-only, to avoid the same text being announced twice
…t in action button
acstll
added
accessibility
skip-changelog
acstll
removed
the
skip-changelog
alexwizp
left a comment
alexwizp
left a comment
There was a problem hiding this comment.
LGTM! Thanks for fixing that!
| toolTipProps={{ | ||
| delay: 'long', | ||
| // Avoid screen-readers announcing the same text twice | ||
| disableScreenReaderOutput: |
There was a problem hiding this comment.
Non blocking suggestion:
Looking at the documentation, I feel like we could add some information about the expected usage to ensure accessibility.
There was a problem hiding this comment.
that's a really good point, I will try and something in this regard to the docs 👍
There was a problem hiding this comment.
There was a problem hiding this comment.
💭 The main issue I have with this is that this is a callout and we're placing too much content in it. Callouts should just be text.
How about we add an "Accessibility" section with admonition and snippet instead?
There was a problem hiding this comment.
Updated in 667eac0
(feedback and suggestions very much appreciated 🙇)
mgadewoll
left a comment
mgadewoll
left a comment
There was a problem hiding this comment.
🟢 Changes LGTM. The tooltips behave as expected.
💭 While there can still be cases of duplicate output (non-string), we at least improve the base string usage with this update 👍
regarding actions names and descriptions
💚 Build SucceededHistory
cc @acstll |
💚 Build Succeeded
History
cc @acstll |
5 participants
Summary
This helps prevent an accessibility issue when screen-readers would announce the same text twice for action buttons in tables, when
action.name(the label of the button) andaction.description(that gets rendered in a tooltip) are exactly the same.🔗 Row actions docs
Relates to elastic/kibana#215902
Why are we making this change?
To help improve accessibility. EUI alone cannot guarantee compliance. Ideally consumers would pass something different in the aforementioned props, e.g.
✅ Good
❌ Bad
This PR makes the tooltip visual-only for the latter.
Impact to users
🟢 Accessibility issues like elastic/kibana#215902 should get fixed by this.
QA
WIP
Remove or strikethrough items that do not apply to your PR.
General checklist
Checked in both light and dark modesChecked in both MacOS and Windows high contrast modes(emulate forced colors if you do not have access to a Windows machine.)Checked in mobileChecked in Chrome, Safari, Edge, and FirefoxChecked for accessibility including keyboard-only and screenreader modesAdded documentationProps have proper autodocs (using@defaultif default values are missing) and playground togglesChecked Code Sandbox works for any docs examplesUpdated visual regression testsIf applicable, added the breaking change issue label (and filled out the breaking change checklist)If applicable, file an issue to update EUI's Figma library with any corresponding UI changes. (This is an internal repo, if you are external to Elastic, ask a maintainer to submit this request)