Conversation
Fix for a11y by making the span focusable and adds focus ring for visual indicator
chandlerprall
left a comment
There was a problem hiding this comment.
Changes LGTM; pulled and tested beta badge docs locally
myasonik
left a comment
There was a problem hiding this comment.
I thought on this PR for a long while... Probably longer than I needed to...
Just adding a tabIndex to a non-interactive element doesn't sit right but I couldn't figure out a smarter solution and it largely works, so, ultimately a 👍 from me. It definitely improves on the current state!
Ignoring the semantics of it, the one practical issue I came across is when navigating the page with a screen reader (not jumping around with TAB), the description isn't read out and nothing tells me that this element is something special so I should focus on it.
elizabetdev
left a comment
There was a problem hiding this comment.
Tested locally and LGTM! 🎉
|
@myasonik Yeah it's more of a limitation of our basic tooltip behavior than specific to EuiBetaBadge that we should address at some point. Thanks! |
Summary
Since EuiBetaBadge's don't actually have any clickability to them, they were ignored from the tab order and therefore, the description which only shows on hover could never be read by a screenreader. This PR basically just adds
tab-index=0to the element putting it into the focus order of the DOM. We handle this the same way for EuiIconTip.This also then required moving the DOM order of the beta badge in EuiCard so that it didn't come before the title of the card.
Checklist
[ ] Checked in dark mode[ ] Checked in mobile[ ] Props have proper autodocs[ ] Added documentation examples[ ] Added or updated jest tests[ ] Checked for breaking changes and labeled appropriately