Material-ui: [Tabs] Improve TabPanel demos, avoid validateDOMNesting warning

Created on 13 May 2020  路  12Comments  路  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 馃槸

New experimental Tabs API uses <Typography> as the root element for children, preventing nesting anything besides simple text without validateDOMNesting errors. See here: https://github.com/mui-org/material-ui/blob/8c2abebab524a68e97ffb5cf0c2015a915bd2e12/packages/material-ui-lab/src/TabPanel/TabPanel.js#L36

Expected Behavior 馃

I believe that removing <Typography> completely and rendering children directly would be the best solution with maximum flexibility.

Context 馃敠

Will sent a PR if you agree with the solution.

Your Environment 馃寧

| Tech | Version |
| ----------- | ------- |
| Material-UI | v4.9.13 |
| Material-UI lab | v4.0.0-alpha.53 |
| React | 16.13.1 |

Tabs docs good to take

Most helpful comment

Note: the warning appears only because the demo code has:

function TabPanel(props) {
  const { children, value, index, ...other } = props;

  return (
    <div
      role="tabpanel"
      hidden={value !== index}
      id={`simple-tabpanel-${index}`}
      aria-labelledby={`simple-tab-${index}`}
      {...other}
    >
      {value === index && (
        <Box p={3}>
          <Typography>{children}</Typography>
        </Box>
      )}
    </div>
  );
}

Changing it like this takes care of it:

function TabPanel(props) {
    const {children, value, index, classes, ...other} = props;

    return (
        <div
            role="tabpanel"
            hidden={value !== index}
            id={`simple-tabpanel-${index}`}
            aria-labelledby={`simple-tab-${index}`}
            {...other}
        >
            {value === index && (
                <Container>
                    <Box>
                        {children}
                    </Box>
                </Container>
            )}
        </div>
    );
}

All 12 comments

I believe that removing completely and rendering children directly would be the best solution with maximum flexibility.

Typography is definitely inappropriate for a container-like component.

But I'm leaning towards swapping the div[role="tabpanel"] for a Paper[role="tabpanel"] so that we have some sensible default styles.

But I'm leaning towards swapping the div[role="tabpanel"] for a Paper[role="tabpanel"] so that we have some sensible default styles.

Or not. Depending on the use case there could be a rich UI inside the panel and a Paper is probably not always correct. You could always do <TabPanel><Paper>{children}</Paper></TabPanel> if you want to.

The issue makes me wonder on a related matter, do we want a default padding?

https://github.com/mui-org/material-ui/blob/303e5a0eef0e760bc7a808860305541f9a8f8bee/packages/material-ui-lab/src/TabPanel/TabPanel.js#L11

It looks opinionated, serving a narrow set of use-cases.

It looks opinionated, serving a narrow set of use-cases.

Our styles are opinionated, sure.

Hello
Is this issue now resolved?
Thanks Sam

Note: the warning appears only because the demo code has:

function TabPanel(props) {
  const { children, value, index, ...other } = props;

  return (
    <div
      role="tabpanel"
      hidden={value !== index}
      id={`simple-tabpanel-${index}`}
      aria-labelledby={`simple-tab-${index}`}
      {...other}
    >
      {value === index && (
        <Box p={3}>
          <Typography>{children}</Typography>
        </Box>
      )}
    </div>
  );
}

Changing it like this takes care of it:

function TabPanel(props) {
    const {children, value, index, classes, ...other} = props;

    return (
        <div
            role="tabpanel"
            hidden={value !== index}
            id={`simple-tabpanel-${index}`}
            aria-labelledby={`simple-tab-${index}`}
            {...other}
        >
            {value === index && (
                <Container>
                    <Box>
                        {children}
                    </Box>
                </Container>
            )}
        </div>
    );
}

Yes that fixed it for me thanks a lot. As you said the demo code should probably be updated.

Thank you @VikR0001

Thanks @VikR0001 !

To highlight what's in that comment - removing <Typography>...</Typography> does the trick.

Thanks @VikR0001 removing <Typography>...</Typography> was the solution

Note: the warning appears only because the demo code has:

function TabPanel(props) {
  const { children, value, index, ...other } = props;

  return (
    <div
      role="tabpanel"
      hidden={value !== index}
      id={`simple-tabpanel-${index}`}
      aria-labelledby={`simple-tab-${index}`}
      {...other}
    >
      {value === index && (
        <Box p={3}>
          <Typography>{children}</Typography>
        </Box>
      )}
    </div>
  );
}

Changing it like this takes care of it:

function TabPanel(props) {
    const {children, value, index, classes, ...other} = props;

    return (
        <div
            role="tabpanel"
            hidden={value !== index}
            id={`simple-tabpanel-${index}`}
            aria-labelledby={`simple-tab-${index}`}
            {...other}
        >
            {value === index && (
                <Container>
                    <Box>
                        {children}
                    </Box>
                </Container>
            )}
        </div>
    );
}

I think that we need to change the documentation, this p > div nesting has been a continuous pain: https://github.com/mui-org/material-ui/issues/21015#issuecomment-663208786. I think that we should either drop the Typography from the demo or use the lab tab panel component. I would be leaning for the latter.

Was this page helpful?
0 / 5 - 0 ratings