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

[Tabs] Nested tabs cause console error #33606

Closed
2 tasks done
fzaninotto opened this issue Jul 21, 2022 · 4 comments Β· Fixed by #34026
Closed
2 tasks done

[Tabs] Nested tabs cause console error #33606

fzaninotto opened this issue Jul 21, 2022 · 4 comments Β· Fixed by #34026
Labels
component: tabs This is the name of the generic UI component, not the React module! package: material-ui Specific to @mui/material ready to take Help wanted. Guidance available. There is a high chance the change will be accepted

Comments

@fzaninotto
Copy link
Contributor

fzaninotto commented Jul 21, 2022

Duplicates

  • I have searched the existing issues

Latest version

  • I have tested the latest version

Current behavior 😯

Given the page contains a tab with tabs:

β”Œ-------β”“
| FIRST | SECOND 
|       └––––––––┓
| β”Œ---β”“          |
| | A | B        |
| |   └–––┓      |
| | lorem |      |
| └–––––––┛      |
└––––––––––––––––┛

When the user switches to the second tab, MUI logs an error:

MUI: The value provided to the Tabs component is invalid.

Expected behavior πŸ€”

No error πŸ˜„

Steps to reproduce πŸ•Ή

  1. Open https://codesandbox.io/s/frosty-fast-k5whqv
  2. Click on the 'SECOND' tab header
  3. An error occurs in the console

Context πŸ”¦

This bug appeared in react-admin, when a user tried to nest <TranslatableInputs> inside a <TabbedForm> (marmelab/react-admin#7922)

Your environment 🌎

  • Chrome
  • MUI 5.9.1
@fzaninotto fzaninotto added the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Jul 21, 2022
@hbjORbj hbjORbj added component: tabs This is the name of the generic UI component, not the React module! package: material-ui Specific to @mui/material labels Jul 22, 2022
@mnajdova
Copy link
Member

Thanks for the report. Agree, it looks strange. Maybe we can update the condition for the warning to not do it if the whole tabs element is hidden, maybe something like this would be better:

index 4dbc2a5dee..052b96a54b 100644
--- a/packages/mui-material/src/Tabs/Tabs.js
+++ b/packages/mui-material/src/Tabs/Tabs.js
@@ -359,7 +359,9 @@ const Tabs = React.forwardRef(function Tabs(inProps, ref) {
             !warnedOnceTabPresent &&
             tabMeta &&
             tabMeta.width === 0 &&
-            tabMeta.height === 0
+            tabMeta.height === 0 &&
+            // if the tab list element is hidden, don't warn
+            tabsMeta.clientWidth !== 0
           ) {
             tabsMeta = null;
             console.error(

Would be great if we can cover these two scenarios with tests. Would you be interested in creating a PR for this?

@mnajdova mnajdova added ready to take Help wanted. Guidance available. There is a high chance the change will be accepted and removed status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels Aug 18, 2022
@fzaninotto
Copy link
Contributor Author

Maybe we can update the condition for the warning to not do it if the whole tabs element is hidden

As long as it fixes the warning in the case I described, it's fine by me.

Would you be interested in creating a PR for this?

No, sorry - I have my own bugs to fix in react-admin!

@joshlang
Copy link

This one's getting me, too

Here's my scenario:

<Box display={{ xs: "none", lg: "block" }}>
   <Tabs ...
</Box>

Annoying because of the debug error messages dumped into console. But everything still works.

@hbjORbj
Copy link
Member

hbjORbj commented Dec 29, 2022

I added commits to wrap up the PR: #34026

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: tabs This is the name of the generic UI component, not the React module! package: material-ui Specific to @mui/material ready to take Help wanted. Guidance available. There is a high chance the change will be accepted
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants