Material-ui: Tooltip not working with forwardRef

Created on 20 Apr 2020  路  10Comments  路  Source: mui-org/material-ui

  • [x] The issue is present in the latest release.
  • [x] I have searched the issues of this repository and believe that this is not a duplicate.

Current Behavior 馃槸

Tooltip is not being displayed.

Expected Behavior 馃

Tooltip is displayed.

Steps to Reproduce 馃暪

Steps:

  1. Wrap functional component with forwardRef
  2. pass the ref to desired element
  3. instantiate a Tooltip with a given title passing the functional component as the only child

Here's a minimal reproduction

Context 馃敠

Your Environment 馃寧

| Tech | Version |
| ----------- | ------- |
| Material-UI | v4.?.? |
| React | |
| Browser | |
| TypeScript | |
| etc. | |

Tooltip docs

Most helpful comment

Hum, yeah, actually mentioning that it should spread props, could be interesting.

All 10 comments

Please provide a full reproduction test case. This would help a lot 馃懛 .
A live example would be perfect. This codesandbox.io template _may_ be a good starting point. Thank you!

Hi @oliviertassinari. There's a minimal reproduction link below the steps to reproduce. It's a stackblitz project, however it has the latest version of react and material-ui/core installed.

@oliviertassinari Here's a live demo in codesandbox. I understand that you want to be efficient by enforcing consistency among live examples by recommending the usage of codesandbox. However, going as far as closing the issue because the live example was provided in stackblitz instead of codesandbox leaves a bad impression on the community and has a bad impact on material ui as a library. I understand that you are not obliged to act friendly, however I hold material ui in high regards and my intentions with opening an issue like this is to potentially help improve the library. I feel like many developers share this opinion.

馃憢 Thanks for using Material-UI!

We use GitHub issues exclusively as a bug and feature requests tracker, however,
this issue appears to be a support request.

For support, please check out https://material-ui.com/getting-started/support/. Thanks!

If you have a question on StackOverflow, you are welcome to link to it here, it might help others.
If your issue is subsequently confirmed as a bug, and the report follows the issue template, it can be reopened.

@mateja176 Thanks for the reproduction. Tip: you need to spread the props too.

Thanks. Indeed, spreading the rest of the props was the missing piece as documented here. However it would be nice it was also mentioned in the api section for the children prop.

If anybody stumbles upon the same issue, the explanation behind this is that the Tooltip component is using the React.cloneElement api as can be seen on here. Furthermore, additional props are injected into the cloned element, namely:

    {
      aria-describedby: null
      title: "World"
      className: ""
      onTouchStart: function handleTouchStart() {}
      onTouchEnd: function handleTouchEnd() {}
      onMouseOver: function () {}
      onMouseLeave: function () {}
      onFocus: function () {}
      onBlur: function () {}
    }

In this limited case, the example would work if at least onMouseOver and onMouseLeave props were passed down. However the library goes the extra mile to support a wide range of use cases for example touch events - that is why it is recommended to spread the rest of the props.

As is, the prop injection cannot be expressed through strongly typing the children prop (the current type React.ReactElement). However this could be accomplished with the render props pattern, for example:

<Tooptip>
  {(tooltipChilProps) => (
    <CustomComponent {..componentProps} {...tooltipChildProps} />
  )}
</Tooltip>

As to why the render props pattern wasn't chosen is, I assume, because of performance reasons.

@mateja176 You are not the first one to fall into this trap, we have tried in the past to guide people with docs changes, as much as we could. I'm not aware of any new leverage we get at this point. Thanks for the detailed explanation.

Hum, yeah, actually mentioning that it should spread props, could be interesting.

@mateja176 You are not the first one to fall into this trap, we have tried in the past to guide people with docs changes, as much as we could. I'm not aware of any new leverage we get at this point. Thanks for the detailed explanation.

You're very much welcome.

Sorry, closing I don't think that we have the bandwidth to spend more time on the problem.

Was this page helpful?
0 / 5 - 0 ratings