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

Inconsistent behavior of tooltip on disabled buttons #471

Closed
btzr-io opened this issue Oct 30, 2019 · 8 comments · Fixed by #552
Closed

Inconsistent behavior of tooltip on disabled buttons #471

btzr-io opened this issue Oct 30, 2019 · 8 comments · Fixed by #552
Labels

Comments

@btzr-io
Copy link

btzr-io commented Oct 30, 2019

🐛 Bug report

Current behavior

Tooltip on disabled buttons fails to hide properly on chromium based browsers ( chrome, brave...) Tested only on linux.

On Firefox works as expected 👍

Steps to reproduce the bug

Provide a repo or sandbox with the bug and describe the steps to reproduce it.

  1. Open sandbox: https://codesandbox.io/embed/reakit-menu-with-tooltip-1fvz3
  2. Hover disabled button.

Expected behavior

I'm not sure if this is the intended behavior or if it should just work as expected ( show on hover, hide on click outside or mouse leave etc...)

Possible solution

Should I just hide the tooltip on a disabled component ? what is the popper way to handle this ?

Environment

Chrome 78.0.3904.70
Firefox 71.0b4
Brave "Version 0.70.121 Chromium: 78.0.3904.70"
$ npx envinfo --system --binaries --npmPackages

Environment:
  OS:  Linux 4.15
  Node:  10.15.3
  Yarn:  1.19.1
  npm:  6.12.0
  Watchman:  Not Found
  Xcode:  N/A
  Android Studio:  Not Found
@diegohaz
Copy link
Member

diegohaz commented Oct 30, 2019

Looks like onMouseEnter is fired on React disabled buttons, but not onMouseLeave:

<button
  disabled
  onMouseEnter={() => console.log("enter")}
  onMouseLeave={() => console.log("leave")}
>
  Button
</button>

TooltipReference uses onMouseEnter and onMouseLeave to show/hide the tooltip:
https://github.com/reakit/reakit/blob/65650b3730a4ebcb6c47280277393cc793d5b44a/packages/reakit/src/Tooltip/TooltipReference.ts#L44-L45

Not sure if it's a bug on React or something. The right behavior is to not fire any of these events. That's the way the DOM works when you use addEventListener("mouseenter").

One solution we could do on our side would be using onMouseOver and onMouseOut instead. They won't fire on <Button disabled>. If we want a disabled button to show the tooltip, we can simply use <Button disabled focusable>.

@diegohaz
Copy link
Member

I've opened an issue on React: facebook/react#17229

By the way, temporarily, you can fix it by grabbing the onMouseEnter and onMouseLeave props and re-assigning them to onMouseOver and onMouseOut:

<TooltipReference {...tooltip}>
  {({ onMouseEnter, onMouseLeave, ...props }) => (
    <button onMouseOver={onMouseEnter} onMouseOut={onMouseLeave} {...props}>
      Button
    </button>
  )}
</TooltipReference>

Things to note:

  • Put {...props} last so, if things change later, your code will not break.
  • This will not show the tooltip on disabled buttons since they're not focusable and non-pointer users wouldn't be able to activate it. To make that work, as I mentioned before, just pass a focusable prop to Button (the Reakit one).
  • onMouseOver is different from onMouseEnter. The former bubbles up, so, if you have child elements within the button, the event will be triggered more times. This shouldn't be a problem for this particular case, but that's something to keep in mind.

@abrahamwilliam
Copy link

@edwincoronado @diegohaz @btzr-io

I have identified the fix to this issue/Bug, just testing it.I am proceeding to it, please let me know if any issues.

@aguscha333
Copy link

Hey @diegohaz this seems to have regressed in the current reakit version.
I noticed that this line that took into consideration if the item was truly disabled is no longer there https://github.com/ariakit/ariakit/pull/552/files#diff-122eed30b245d14614743f01ffc98eeda7690c09389f5bf2efd1c1a6abdd33c0R98

Same line of code in the v1 branch: https://github.com/ariakit/ariakit/blob/v1/packages/reakit/src/Tabbable/Tabbable.ts#L128

I saw that in the v2 you can solve this by using accessibleWhenDisabled but I am not sure if I should be using v2 yet on production.
What is your recommendation? is this sth that you think should be/will be fixed for v1? if not, should I use sth like patch-package to patch the v1 lib for the specific project I am working on or is v2 stable enough and recommended over v1?

@diegohaz
Copy link
Member

@aguscha333 Try this:

<Button disabled focusable style={{ pointerEvents: "auto" }}>

@aguscha333
Copy link

thanks for the reply, that works but I have to update a lot of snapshot tests that I was trying to avoid. I'll see if I go with that route or with the patch package one

@mathias-bradi
Copy link

any update ?

@emilygracekzo
Copy link

emilygracekzo commented Sep 21, 2022

Hi! I'm also experiencing this bug. The workaround style={{ pointerEvents: "none" }} keep us unblocked, but is there any plans to fix this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants