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

Allow tooltips to function on disabled elements #11601

Closed
2 tasks done
mririgoyen opened this issue May 27, 2018 · 8 comments
Closed
2 tasks done

Allow tooltips to function on disabled elements #11601

mririgoyen opened this issue May 27, 2018 · 8 comments
Assignees
Labels
component: tooltip This is the name of the generic UI component, not the React module! docs Improvements or additions to the documentation good first issue Great for first contributions. Enable to learn the contribution process.

Comments

@mririgoyen
Copy link

mririgoyen commented May 27, 2018

Currently, when a tooltip is attached to an element which is in a disabled state, warnings are outputted in the console and the tooltip fails to operate. This is extremely annoying.

This bug is in reference to: #8416

  • This is a v1.x issue (v0.x is no longer maintained).
  • I have searched the issues of this repository and believe that this is not a duplicate.

Expected Behavior

When a tooltip is assigned to a disabled item, the tooltip works like normal. No console warnings or errors are produced.

Current Behavior

When a tooltip is assigned to a disabled item, it produces annoying console spam. Additionally, the tooltip does not display.

To work around this currently, every element that may become disabled is being wrapped in an empty <span>. This is less that optimal as it clutters the DOM with elements that are completely unneeded.

Steps to Reproduce (for bugs)

https://codesandbox.io/s/rjjn7lx4rq

Context

Tooltips provide context. When a button is disabled, I need to convey to my user why that button is disabled. I currently cannot use tooltips to do this because of this limitation.

Additionally, I have a case where I have a toolbar of icons (20+) that, when content is loading, these buttons are in a disabled state. Once the content loads, the buttons enabled. In this case, the console is spammed with 20+ warnings, making it extremely hard sift through console messages when developing.

Your Environment

Tech Version
Material-UI v1.1.0
React 16.4.0
browser Chrome
@mririgoyen mririgoyen changed the title Remove or make toggleable the warning when a tooltip is attached to a disabled element. Allow tooltips to function on disabled elements May 27, 2018
@oliviertassinari oliviertassinari added the component: tooltip This is the name of the generic UI component, not the React module! label May 27, 2018
@oliviertassinari
Copy link
Member

oliviertassinari commented May 27, 2018

Currently, when a tooltip is attached to an element which is in a disabled state, warnings are outputted in the console and the tooltip fails to operate.

@goyney The warning is here to highlight the problem so it doesn't stay silent. People might never realize that the tooltip is broken otherwise.

When a tooltip is assigned to a disabled item, the tooltip works like normal. No console warnings or errors are produced.

This expected behavior would require to automatically add an extra DOM node.
#8416 (comment)

To work around this currently, every element that may become disabled is being wrapped in an empty .

It's more than a workaround, it's a tradeoff we have taken that I believe is the best answer to all the constraints. Not implementing your expected behavior was a reasoned call. We have thought about it in the past. Right now, the tooltip is transparent from a DOM point of view. Automatically adding an extra DOM node can potentially break the layout and looks magically from a user point of view. I think that we should be explicit about this behavior, hence the warning.

it clutters the DOM with elements that are completely unneeded

⚠️ The DOM element is needed. You can't listen to the relevant DOM events without.

This is less that optimal

At least we warn about it. I couldn't benchmark an alternative that handle the problem:

Regarding improving the anwser to the problem, I can think of two alternatives:

  1. People can write their own tooltip wrapper on top of Material-UI that wrap all the contents with a <span> element.
  2. We can add a handleDisable property (or a different wording) to the tooltip to opt-in the automatic wrapping behavior, making it explicit (it won't be the default behavior).

@oliviertassinari oliviertassinari self-assigned this Jun 22, 2018
@oliviertassinari oliviertassinari added new feature New feature or request docs Improvements or additions to the documentation and removed new feature New feature or request labels Jul 3, 2018
@oliviertassinari
Copy link
Member

Bootstrap is documenting the workaround: https://getbootstrap.com/docs/4.1/components/popovers/#disabled-elements.

@kamranayub
Copy link
Contributor

kamranayub commented Jul 23, 2018

@oliviertassinari Just want to chime in here and say I was bitten by this. Also, if this is intentional, I was looking for docs on MUI related to disabled controls and couldn't find anything. I would prefer a flag on the tooltip to support disabled elements as suggested above (I would have seen the flag in the API docs) but at the very least the API docs or demo docs for Tooltip should mention this caveat as it was non-intuitive and I found it via this issue.

@oliviertassinari
Copy link
Member

@kamranayub Let's document it 👍

@oliviertassinari oliviertassinari added the good first issue Great for first contributions. Enable to learn the contribution process. label Jul 23, 2018
@kamranayub
Copy link
Contributor

kamranayub commented Jul 23, 2018 via email

@kamranayub
Copy link
Contributor

Dang super smooth docs experience, kudos. 👍

@slikts
Copy link

slikts commented Oct 9, 2019

The current behavior means that tooltips either break button group rendering or don't work with disabled buttons in button groups.

Edit: an additional annoyance is that the warning can't be suppressed.

@adamschoenemann
Copy link

+1 to slikts, this is quite a nuisance

@mui mui locked as resolved and limited conversation to collaborators Mar 4, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
component: tooltip This is the name of the generic UI component, not the React module! docs Improvements or additions to the documentation good first issue Great for first contributions. Enable to learn the contribution process.
Projects
None yet
Development

No branches or pull requests

5 participants