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

Using ListItemSecondaryAction within Tooltip logs warning #28675

Open
2 tasks done
MikeRawding opened this issue Sep 28, 2021 · 2 comments
Open
2 tasks done

Using ListItemSecondaryAction within Tooltip logs warning #28675

MikeRawding opened this issue Sep 28, 2021 · 2 comments
Labels
component: tooltip This is the name of the generic UI component, not the React module!

Comments

@MikeRawding
Copy link

MikeRawding commented Sep 28, 2021

Thank you for building MUI! ❤️

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

Current Behavior 😯

Using a ListItemSecondaryAction within a Tooltip logs an error.

The children component of the Tooltip is not forwarding its props correctly.
Please make sure that props are spread on the same element that the ref is applied to.

Expected Behavior 🤔

Using MUI's own components within a Tooltip should not log an error.

Steps to Reproduce 🕹

https://codesandbox.io/s/delicate-paper-fb3c6?file=/src/App.js

Steps:

  1. Just render a component with hierarchy Tooltip > ListItem > ListItemSecondaryAction

Context 🔦

  1. I discovered this while transitioning an app from v4 to v5.
  2. Using the secondaryAction prop of ListItem rather than a using ListItemSecondaryAction as a child does not show the error (but has other effects, changes the dom structure).
  3. I see the demo in v5 is using the secondaryAction prop, but ListItem API docs suggest using ListItemSecondaryAction as a child of ListItem is still supported. https://mui.com/api/list-item/
  4. This error was added to the Tooltip component here: [Tooltip] Improve docs and warning for custom children #22775

Your Environment 🌎

`npx @mui/envinfo`
  Chrome
  System:
    OS: Linux 5.4 Linux Mint 20.2 (Uma)
  Binaries:
    Node: 14.15.1 - ~/.nvm/versions/node/v14.15.1/bin/node
    Yarn: 1.22.10 - /usr/local/bin/yarn
    npm: 6.14.8 - ~/.nvm/versions/node/v14.15.1/bin/npm
  Browsers:
    Chrome: 94.0.4606.61
    Firefox: 92.0.1
  npmPackages:
    @emotion/react: ^11.4.1 => 11.4.1 
    @emotion/styled: ^11.3.0 => 11.3.0 
    @mui/core:  5.0.0-alpha.48 
    @mui/icons-material: ^5.0.1 => 5.0.1 
    @mui/lab: ^5.0.0-alpha.48 => 5.0.0-alpha.48 
    @mui/material: ^5.0.1 => 5.0.1 
    @mui/private-theming:  5.0.1 
    @mui/styled-engine:  5.0.1 
    @mui/styles: ^5.0.1 => 5.0.1 
    @mui/system:  5.0.1 
    @mui/types:  7.0.0 
    @mui/utils:  5.0.1 
    @types/react: ^16.14.15 => 16.14.15 
    react: ^17.0.1 => 17.0.1 
    react-dom: ^17.0.1 => 17.0.1 
    typescript: ^4.0.5 => 4.0.5
@MikeRawding MikeRawding added the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Sep 28, 2021
@mnajdova
Copy link
Member

Thanks for the report. Looks like the warning would be shown with all components that do not spread the props on the first element, or at least the element that holds the ref (for example have some kind of container). Here is the simplest reproduction: https://codesandbox.io/s/nostalgic-fire-qxmv2?file=/src/App.js

Not sure what would be the best fix here, to be honest. If we just look in the children for the special prop anywhere in the hierarchy, it may give us false-positive if any tooltip is used inside its children.

@mnajdova mnajdova added component: tooltip This is the name of the generic UI component, not the React module! and removed status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels Sep 29, 2021
@MikeRawding
Copy link
Author

Can we either

  1. Check the hierarchy, and stop if we bump into a Tooltip, or
  2. Make the prop we're looking for unique per Tooltip

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: tooltip This is the name of the generic UI component, not the React module!
Projects
None yet
Development

No branches or pull requests

2 participants