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

Tooltip stuck when surrounding a disabled button #231

Closed
MayaUribe opened this issue Jul 11, 2019 · 10 comments · Fixed by #712
Closed

Tooltip stuck when surrounding a disabled button #231

MayaUribe opened this issue Jul 11, 2019 · 10 comments · Fixed by #712
Labels
Status: Unconfirmed Bug reports without a repro, not yet confirmed

Comments

@MayaUribe
Copy link

MayaUribe commented Jul 11, 2019

When adding a Tooltip on top of a disabled button, the Tooltip gets stuck, I'm not sure if I'm missing a setting, here is an example of what I'm doing 😄

<Tooltip label="Notifications">
   <button style={buttonStyle} disabled={isDisabled}>
     Click Me
   </button>
</Tooltip>

Screen Recording 2019-07-11 at 03 31 PM

@venikx
Copy link

venikx commented Jul 28, 2019

Can you send me the browser, version + OS?
At least for me I can't reproduce it on the latest Firefox (Nightly), but I can reproduce it for Google Chrome.

@audiolion
Copy link

I think it's related to disabled buttons not firing mouse events. As a workaround you can wrap the button with an other element.

sounds right, however wrapping the button in a div is not fixing the issue for me in Google Chrome

@raunofreiberg
Copy link
Contributor

Any workarounds for this? Wrapping the button does not work for me either. This seems like a pretty common use case for Tooltip (indicating disabled button reasoning)

@bradwestfall bradwestfall added the Status: Unconfirmed Bug reports without a repro, not yet confirmed label Oct 1, 2019
@raunofreiberg
Copy link
Contributor

@chancestrickland Sorry to ping, but lately I noticed that more attention is being put towards the development of Reach UI, hence I'm wondering if this issue is high in the priority list? Thanks a lot for spending time on this, I really enjoy using Reach as a foundation :)

@chaance
Copy link
Member

chaance commented Oct 9, 2019

@raunofreiberg It's in the backlog but we're working through the issues. I'll update as soon as I've merged. If you could help out with a PR or even a CodeSandbox reproduction that would certainly get it moving faster!

@raunofreiberg
Copy link
Contributor

@chancestrickland Thanks for the response! I've set up a Codesandbox with a reproduction of the issue -> https://codesandbox.io/s/reach-ui-tooltip-disabled-button-issue-unwi0

@venikx
Copy link

venikx commented Oct 10, 2019

@raunofreiberg @chancestrickland Confirmed to be broken on Chromium based browsers. On Firefox it doesn't get stuck. Chromium, Google Chrome and Vivaldi are broken.

@chaance
Copy link
Member

chaance commented Oct 10, 2019

So this appears to be a bigger issue with disabled buttons inconsistently firing mouse events across browsers. Related open issues here facebook/react#4251 and here https://bugs.chromium.org/p/chromium/issues/detail?id=120132

I'll need to think through this a bit as to how we deal with this behavior in Reach. Any suggestions from those following are welcome.

@chaance
Copy link
Member

chaance commented Oct 11, 2019

I think we're going to punt on this issue for now. We are discussing some options and may reconsider, but ultimately this should probably be addressed by the browsers. I also feel like different use cases may require different behaviors, so in the mean time:

  1. If you want to hide the tooltip altogether on disabled buttons, you can either use CSS and set pointer-events: none on your disabled buttons, or use onMouseEnter={event => event.preventDefault()} on your button component if it is disabled (this will work as soon as @mjackson publishes the latest changes for the next version bump, referencing tooltip: Pass through events and ref to the tooltip trigger #295)

  2. If you want to show the tooltip regardless of a button's disabled state, you can either

  • wrap the button in a div so the tooltip trigger's events are attached to the div (I'd probably opt for this approach), or
  • use aria-disabled instead of disabled, but you'll need to patch a bunch of other behavior for this to work as expected

@chaance chaance closed this as completed Oct 11, 2019
@raunofreiberg
Copy link
Contributor

@chancestrickland I tried one of your suggestions

wrap the button in a div so the tooltip trigger's events are attached to the div (I'd probably opt for this approach), or

and it doesn't seem to work 🤔

Reproducable case: https://codesandbox.io/s/sad-hertz-howcs

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Unconfirmed Bug reports without a repro, not yet confirmed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants