mui / material-ui

Material UI: Comprehensive React component library that implements Google's Material Design. Free forever.
https://mui.com/material-ui/
MIT License
93.92k stars 32.27k forks source link

[Tabs] Conditionally rendering of `Tab` not working properly #34740

Open drac94 opened 2 years ago

drac94 commented 2 years ago

Duplicates

Latest version

Steps to reproduce 🕹

Steps:

  1. Add some tabs
    <Tabs
    value={value}
    onChange={handleChange}
    aria-label="basic tabs example"
    >
    <Tab label="Item One" {...a11yProps(0)} />
    <Tab label="Item Two" {...a11yProps(1)} />
    <Tab label="Item Three" {...a11yProps(2)} />
    </Tabs>
  2. Wrap them in a component that checks if the tab should be displayed, for simplicity I am just retuning the children wrapped in a fragment but in theory it should make some checks to return the children or null
    const Authorizer = ({ children }: { children: React.ReactNode }) => (
    <>{children}</>
    );
<Authorizer>
  <Tab label="Item One" {...a11yProps(0)} />
</Authorizer>

https://codesandbox.io/s/unruffled-thunder-f405xf?file=/demo.tsx

Current behavior 😯

Tabs stop working, they can be clicked but don't navigate to the tab panel

Expected behavior 🤔

Tabs should work normally as there is not being introduced anything additional in the tree

Context 🔦

I am trying to show or hide the tabs depending on the user permissions, for this I am using an Authorizer component that returns the children (wrapped in a fragment) or null depending on if the user has the right permissions but by doing so, the tabs stop working correctly.

Your environment 🌎

npx @mui/envinfo ``` System: OS: macOS 12.6 Binaries: Node: 16.15.0 - ~/.nvm/versions/node/v16.15.0/bin/node Yarn: 1.22.18 - ~/.nvm/versions/node/v16.15.0/bin/yarn npm: 8.5.5 - ~/.nvm/versions/node/v16.15.0/bin/npm Browsers: Chrome: 106.0.5249.103 Edge: Not Found Firefox: Not Found Safari: 16.0 npmPackages: @emotion/react: ^11.9.3 => 11.9.3 @emotion/styled: ^11.9.3 => 11.9.3 @mui/base: 5.0.0-alpha.85 @mui/icons-material: ^5.8.4 => 5.8.4 @mui/lab: ^5.0.0-alpha.91 => 5.0.0-alpha.91 @mui/material: ^5.8.4 => 5.8.4 @mui/private-theming: 5.8.4 @mui/styled-engine: 5.8.0 @mui/system: 5.8.4 @mui/types: 7.1.4 @mui/utils: 5.8.4 @mui/x-data-grid: ^5.12.3 => 5.12.3 @types/react: ^17.0.33 => 17.0.44 react: ^17.0.2 => 17.0.2 react-dom: ^17.0.2 => 17.0.2 typescript: ^4.5.4 => 4.6.3 ```

Using Chrome

Nikhil562 commented 2 years ago

I would love to do that , can you please assign to me

drac94 commented 2 years ago

Not sure if this happens with all the components but as soon as I wrap them with a fragment they stop working correctly, check this example when using the grid

getActions: (params: GridRowParams) => [
  <>
    <GridActionsCellItem
      key={params.id}
      icon={<EditOutlined />}
      onClick={editTeam(params.row.id)}
      label="Edit"
      showInMenu
    />
  </>,
  <>
    <GridActionsCellItem
      key={params.id}
      icon={<DeleteOutlined />}
      onClick={deleteTeam(params.row)}
      label="Delete"
      showInMenu
    />
  </>,
],

The above render the buttons directly on the grid instead of in the menu

image

If I remove the fragments it works correctly

image

AChangXD commented 2 years ago

Also having similar issue. For me, the onChange fires correctly the first time, and it successfully set the state that is assigned to value field of to the new value. However, the Tabs component doesn't re-render and it still thinks it was set to the previous value.

AChangXD commented 2 years ago

How I'm creating/passing the : `setToolbarItems( <> <Tabs value={selectedTab} // If I get rid of this value field, things will work again but it won't display which tab is selected. onChange={(event, newValue) => { setSelectedTab(newValue); }}

</> );`

I'm making the dynamically change its content based on the page it's on. On certain pages, it will display an for navigation within that page.

Code for :

`return ( <> <Box sx={{ minHeight: 64, flexGrow: 1 }}> <AppBar position="static" sx={{ paddingLeft: 0, paddingRight: 0 }}

<Toolbar sx={{ minHeight: 64, margin: 0, justifyContent: 'center', backgroundColor: 'white', }}

{showLeftNav && ( <IconButton edge="start" color="inherit" aria-label="menu" sx={{ paddingLeft: 0, width: 64 }} onClick={() => { setShowLeftNav((previousState) => !previousState); }}

)}

        {toolbarItems}                        <- where the <Tabs> end up

        {showLeftNav && <Box sx={{ width: 64 }}></Box>}
      </Toolbar>
    </AppBar>
  </Box>
</>

);`

mnajdova commented 1 year ago

The reason is because you are not propagating all props to the underlaying Tab component. The Tabs uses the React.cloneElement API for providing props to the children it receives. Here is a working example: https://codesandbox.io/s/quizzical-hawking-q6ws58?file=/demo.tsx

Another useful example in terms of customization would be: https://mui.com/material-ui/react-tabs/#customization I hope this explanation helps.

If someone wants to extend the docs adding more information around this, would be great. cc @samuelsycamore