-
-
Notifications
You must be signed in to change notification settings - Fork 32.4k
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 doesn't work for <IconButton disabled> #8416
Comments
https://stackoverflow.com/questions/18113937/fire-onmouseover-event-when-element-is-disabled Implementing this suggestion looks like this, and it work. <Tooltip title="Tooltip" placement="bottom">
<div>
<IconButton disabled>
<Done />
</IconButton>
</div>
</Tooltip> |
I was also thinking of using the <Tooltip title="Tooltip" placement="bottom">
<IconButton component="div" disabled>
<Done />
</IconButton>
</Tooltip> |
@oliviertassinari Sorry, didn't know. Thanks. |
@Bravecow I think that we can add a warning if more people raise this issue. |
So is the accepted solution to this going to be putting a div between buttons and tooltips? Having a tooltip is typically the most helpful on disabled buttons, to indicate why the button is disabled. What if we added a prop to the |
How do you turn off the tooltip warning? |
|
@oliviertassinari How do I turn off the tooltip warning without cluttering up the DOM? I have several toolbars of buttons that are disabled when content is loading. Each button has a tooltip on it. Once the document loads, they all enable. Having to wrap every. single. button. in a |
@goyney What about conditionally rendering a Tooltip when needed? As far as I understand it, you don't want to display any tooltip when the button is disabled. |
I always want to display a tooltip. How about a |
@goyney Even when the button is disabled? |
Yes, that is what I said. lol |
Like I mentioned above, tooltips are of extra use to users when a button is disabled @oliviertassinari, in order to indicate to them why a button is disabled. |
This helps in showing the tooltip on the disabled button but the button which is enclosed in the 'div' loses its styling for me. What am I missing here? |
@man-u-13 try to replace div tag with React.Fragment or empty tag |
Thank you @BuangHosen, using |
@BuangHosen but |
This is a really painful thing for all mui components. Adding a tooltip on a disabled element is such an essential UX detail (you should always explain why something is disabled, your user won't know, and you the developer will forget all the reasons), that every time I write this I'm a bit frustrated. // API I want
<IconButton tooltip="Snooze a task" />;
<IconButton tooltip="You can't snooze an important task" disabled />; Adding a wrapper like this is a terrible solution:
const MyCustomIconButton = ({ disabled, tooltip, ...props}) => {
let iconButton = <IconButton {...props} />;
if (tooltip) {
if (disabled) {
iconButton = <div>{iconButton}</div>;
}
return <Tooltip title={tooltip}>{iconButton}</Tooltip>;
}
return iconButton
} |
@mui/core would you consider taking another look at this issue? Maybe we can find a way to improve DX without forcing developers to manually add wrapping elements? I.e. what if <Tooltip resolveDisabled> I'm also wondering why is reported with In addition, I think it would be nice to have the option to disable these warnings/errors globally/per-instance, maybe |
@bytasv Do you have more context on the problem you faced?
This idea was suggested and discussed a bit in #11601 (comment). The arguments there are probably still up-to-date.
I don't understand, the cases consider seem to be broken too. For example, what's the link with
Which use case requires to suppress the warning? |
Here is codesandbox example, within
#11601 (comment) as for options discussed there - seems that we might be on the same page except that the discussion mentioned hasn't been resolved. Since this can't be resolved automatically (because of potentially breaking layout) it would be nice to provide "hands free" way to do the wrapping or any other way that would allow to show tooltip even on the disabled items. Anyways if we choose not to handle disabled cases via prop, then I think we should at least address error/warning part:
|
@bytasv I think that we should label https://codesandbox.io/s/tender-meadow-chtejn?file=/demo.tsx as an invalid case. It doesn't show the tooltip correctly on hover, the console error looks correct to me. People can do, which seems to behave correctly: import * as React from "react";
import Tooltip from "@mui/material/Tooltip";
import DeleteIcon from "@mui/icons-material/Delete";
export default function BasicTooltip() {
return (
<button type="button" disabled style={{ pointerEvents: "all" }}>
<Tooltip title="Delete">
<DeleteIcon />
</Tooltip>
</button>
);
} https://codesandbox.io/s/competent-dew-if6uec?file=/demo.tsx |
IMO we can hint users of what could be potentially breaking but we should still let them choose to ignore if they are fine to take the risks. As a developer I'd at least want to have an option to say "Ok MUI, I've heard you, but I choose to ignore this, please don't bother me about it again". Currently there is no way to do that without rewriting how each tooltip is used (which is not the best DX IMO).
Still the best (for me) would be if I could use some prop like |
Place the button inside the span tag. <Tooltip title="You don't have permission to do this">
<span>
<Button disabled>A Disabled Button</Button>
</span>
</Tooltip> |
@bytasv I think people ignore warnings & errors in the console in the POC mode, I personally more or less do.
We define the warning vs. error policy for the docs callouts https://www.notion.so/mui-org/Callouts-and-their-usage-in-the-docs-7e709a02c72142cd8b1a0829861435c5#5e769cdb5102440ab4bb7bbd16e43ada. Error is for when your setup is wrong, either it will either fail today or tomorrow. For the console message in the code, I had this RFC #21979 that goes in the direction you are suggesting. Why not (A.).
Developers can explicitly hide the tooltip when disabled, which doesn't show the console message. <Tooltip title="Delete" open={false}> https://codesandbox.io/s/spring-dew-0vg8m7?file=/demo.tsx:187-224 It sounds like we could improve the error message to make this more obvious. Per https://www.notion.so/mui-org/Technical-writing-7e55b517ac2e489a9ddb6d0f6dd765de#85219822c3194b6f8a49ee08ea82b90a, we could give this alternative way out in the error message (B.). diff --git a/packages/mui-material/src/Tooltip/Tooltip.js b/packages/mui-material/src/Tooltip/Tooltip.js
index a9976a632f..d53ed8093a 100644
--- a/packages/mui-material/src/Tooltip/Tooltip.js
+++ b/packages/mui-material/src/Tooltip/Tooltip.js
@@ -301,10 +301,10 @@ const Tooltip = React.forwardRef(function Tooltip(inProps, ref) {
console.error(
[
'MUI: You are providing a disabled `button` child to the Tooltip component.',
- 'A disabled element does not fire events.',
- "Tooltip needs to listen to the child element's events to display the title.",
+ 'A disabled button does not fire mouse events.',
+ "But the Tooltip needs to listen to the child element's events to display the title.",
'',
- 'Add a simple wrapper element, such as a `span`.',
+ 'To solve the issue, you can add a wrapper element (e.g. a `span`) around the child or hide the tooltip with `open={false}`.',
].join('\n'),
);
} I'm reopening, I think that we could apply suggestions A. and B. |
The proper solution for this is to land #27719 |
IMO that's a bold assumption - i.e. I do ignore SOME warnings/errors which I consider not important, but I still want to see important info without the noise. Having Tooltip complain about disabled is not important to me and I'd like to be able to not see that without having to change the code or disabled ALL errors entirely
Let's look at these two levels - "something will fail today or tomorrow" 👇 vs "a situation that could cause unexpected behaviour" These two lights are basically Now considering those check engine lights examples I'd say the analogy could be if the car threw you RED check engine light as soon as one of the light bulbs wasn't working. We could also argue that it is important because during the night if you loose the second bulb you might need to drive in total darkness. IMO it's important to differentiate what's consider a "functional failure" which doesn't allow us to carry on the work vs what is probably not good but allows us to carry on
I'm not arguing that there is no way to hide these warnings (one can use one of previously mentioned workarounds), what I as developer expect from a library is to be able to tell it "that I know better what I'm doing, thank you for warning me, I will take it from here" - pretty much in a similar fashion as we instruct OS to "not show me these popup warnings again", well except that this could probably be done using some config flag. |
ah 🤯 why it is such a pain to fix simple thing. Though error is self explanatory, not finding proper solution.
Following is the sample code
@oliviertassinari any help. thanks |
Is there any way to suppress the |
I ran into the same issue... Solution that i found is to add a sx style to the GridActionCellItem component like so:
|
I had the same issue. I tried to wrap it into component, and it works as well My code:
|
how about this?
|
Expected Behavior
Tooltip is visible for hover
Current Behavior
Tooltip is hidden for hover
Steps to Reproduce (for bugs)
Your Environment
The text was updated successfully, but these errors were encountered: