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

[EuiButtonGroup] add tooltip support #6313

Closed
drewdaemon opened this issue Oct 17, 2022 · 9 comments · Fixed by #7461
Closed

[EuiButtonGroup] add tooltip support #6313

drewdaemon opened this issue Oct 17, 2022 · 9 comments · Fixed by #7461
Labels
feature request help wanted The EUI team is looking for community members to pick up and implement this issue

Comments

@drewdaemon
Copy link

Could we allow tooltips to be attached to buttons in a button group?

@drewdaemon drewdaemon changed the title [EuiButtonGroup] [EuiButtonGroup] add tooltip support Oct 17, 2022
@elizabetdev
Copy link
Contributor

Hi @andrewctate,

Each button in a EuiButtonGroup supports an HTML title attibute as we can see on the following example. https://codesandbox.io/s/happy-wilson-uik4bz?file=/demo.js.

Screenshot 2022-10-17 at 18 09 43

I wonder if the title attribute is enough or if there is any particular reason you still need a tooltip.

@MichaelMarcialis
Copy link
Contributor

I wonder if the title attribute is enough or if there is any particular reason you still need a tooltip.

Great question, @miukimiu. From my perspective, I believe using an EuiToolTip would be preferable from a consistency and discoverability standpoint. In Lens, most disabled buttons (excluding those in EuiButtonGroup) and inputs utilize EuiToolTip to explain why that element is disabled and how it can be re-enabled. EuiToolTip also allows us to choose between long and short hover delays. Using native browser tooltips via the title attribute on some elements in Lens would introduce a level of inconsistency into the experience. It also doesn't allow us any control over the delay of the tooltip appearing (in which case, they always show after a fairly lengthy delay).

Another great comment came in a previous Slack conversation between @andrewctate and @1Copenut. In it, @1Copenut mentioned the following:

I'd prefer not to add tooltips to disabled buttons. We use the HTML disabled attribute, so the button won't take keyboard focus. This prevents users who navigate by keyboard and use screen readers from accessing the tooltip information.

This is an excellent point, and one that actually extends beyond this request for tooltips on EuiButtonGroup buttons (as this applies to any disabled EuiButtons using a tooltip). When a button element uses the disabled attribute, the tooltips can't be triggered via keyboard focus, as focus is prevented in a disabled state (though hover is still supported, hence why hovering with the mouse still triggers the tooltip). In reading up on the issue, I found a CSS Tricks article that attempts to remedy this issue by switching out the disabled attribute for aria-disabled.

Any thoughts on such an approach (in addition to the core question of whether we are able and willing to add tooltip support for EuiButtonGroup buttons)?

@elizabetdev
Copy link
Contributor

Thanks, @MichaelMarcialis for providing more context.

From my perspective, I believe using an EuiToolTip would be preferable from a consistency and discoverability standpoint.

It makes sense. We'll see how we can prioritize this.

We also have plans to introduce aria-disabled instead of a disabled attribute: #5666.

@github-actions
Copy link

👋 Hey there. This issue hasn't had any activity for 180 days. We'll automatically close it if that trend continues for another week. If you feel this issue is still valid and needs attention please let us know with a comment.

@MichaelMarcialis
Copy link
Contributor

Still valid.

@github-actions
Copy link

👋 Hi there - this issue hasn't had any activity in 6 months. If the EUI team has not explicitly expressed that this is something on our roadmap, it's unlikely that we'll pick this issue up. We would sincerely appreciate a PR/community contribution if this is something that matters to you! If not, and there is no further activity on this issue for another 6 months (i.e. it's stale for over a year), the issue will be auto-closed.

@MichaelMarcialis
Copy link
Contributor

I believe there is still a valid use case for supporting tooltips on individual buttons within a button group.

@davismcphee
Copy link
Contributor

Hey team, just adding my +1 here on liking to see this prioritized if possible. We recently did a big design refresh in Discover that uses EuiButtonGroup for a number of new controls, and the inconsistency between the tooltips on these buttons and all other buttons in our app takes away from the experience a bit. I wouldn't say it's necessarily a "high" priority on our end, but it would be great to see this in the near future if possible to help add some polish to our design refresh:
image

@JasonStoltz JasonStoltz added the help wanted The EUI team is looking for community members to pick up and implement this issue label Jan 16, 2024
Copy link

👋 Thank you for your suggestion or request! While the EUI team agrees that it's valid, it's unlikely that we will prioritize this issue on our roadmap. We'll leave the issue open if you or anyone else in the community wants to implement it by contributing to EUI. If not, this issue will auto close in one year.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request help wanted The EUI team is looking for community members to pick up and implement this issue
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants