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

[EuiTabs ][A11Y] - Using EuiBetaBadge within EuiTab #7791

Closed
alexwizp opened this issue May 28, 2024 · 3 comments · Fixed by #7805
Closed

[EuiTabs ][A11Y] - Using EuiBetaBadge within EuiTab #7791

alexwizp opened this issue May 28, 2024 · 3 comments · Fixed by #7805

Comments

@alexwizp
Copy link
Contributor

Related to: https://github.com/elastic/observability-dev/issues/3410

We have an issue with using EuiBetaBadge within EuiTab. The AXE plugin triggers a problem: interactive controls must not be nested.

I need your guidance on how to correctly use EuiBetaBadge within EuiTab. I couldn't find a good way to fix this with the existing API. Here are my thoughts on how we can address it:

  1. Extend the EuiBetaBadge props to allow overriding the role="button" attribute.
  2. Extend the EuiTab API to include betaBadgeProps. This would allow us to handle it more natively, similar to how it's done with EuiCards and EuiKeyPadMenuItems.

I vote for option number 2

@cee-chen
Copy link
Contributor

Option 1 should already be fully achievable via ...rest spread, e.g. <EuiBetaBadge role="presentation" /> or even role={undefined}.

Option 2 should not be necessary as the tab itself is not rendering a beta badge directly, it's using an append or prepend API where the consumer is rendering <EuiBetaBadge /> themselves and needs to apply the correct role type. Example:

https://codesandbox.io/p/sandbox/holy-surf-4cj9kk?file=%2Fdemo.js%3A30%2C7

I will note that if these tabs are using the toolTipContent prop on EuiBetaBadge and isn't just rendering text/icons, then something else needs to be done because that tooltip will not be keyboard accessible. We'll need to add a toolTipContent/toolTipProps on the entire EuiTab component/object.

@cee-chen cee-chen added the answered Issues answered by the EUI team - auto-closes open issues in 7 days if no follow-up response label May 29, 2024
alexwizp added a commit to alexwizp/eui that referenced this issue May 31, 2024
@alexwizp
Copy link
Contributor Author

@cee-chen Thank you for your answer. I've checked the EUI code, and I have a quick question about the role=button attribute. Not sure that we need it for that case. The element is not actually a button/link, and I'm considering removing the attribute. If you're okay with this change, I can create a PR to remove it.

@cee-chen
Copy link
Contributor

role="button" and tabIndex={0} is meant to be there for scenarios where tooltipContent exists, so that keyboard users can tab to it to trigger the tooltip. I had previously thought the button role was helpful to indicate tab-ability, is that not correct? If not, I'm fine removing it.

tabIndex={0}
css={cssStyles}
className={classes}
role="button"

@github-actions github-actions bot removed the answered Issues answered by the EUI team - auto-closes open issues in 7 days if no follow-up response label May 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants