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

Collapse component doesn't wrap Grid items correctly #21971

Closed
1 of 2 tasks
zachsa opened this issue Jul 28, 2020 · 6 comments
Closed
1 of 2 tasks

Collapse component doesn't wrap Grid items correctly #21971

zachsa opened this issue Jul 28, 2020 · 6 comments
Labels
component: Collapse The React component

Comments

@zachsa
Copy link

zachsa commented Jul 28, 2020

Using the <Collapse /> component in @material-ui/core@5.0.0-alpha.3, I think there is a bug with the recently added orientation="horizontal" improvements.

Comparing the DOM via developer tool inspection of Collapse code vs Slide code, I see that the Slide component gets classNames from children (I think), whereas the Collapse does not.

  • The issue is present in the latest release. (No - it's in the v5 alpha)
  • I have searched the issues of this repository and believe that this is not a duplicate.

Current Behavior 😯

The <Collapse /> component takes up the whole horizontal space with this structure:

<Grid container>
  <Collapse orientation="horizontal" in={showSidebar}>
    <Grid item xs={4}>
      <Sidebar />
    </Grid>
  </Collapse>

  <Grid item xs>
    <ResultList />
  </Grid>
</Grid>

In this case the Collapse component, when in = true, renders as 100% the width of the <Grid container>. I'm not actually sure that I'm using Grid correctly... But when I use any other Transition this is NOT the case. For example, replacing Collapse with

<Slide direction="right" unmountOnExit in={showSidebar}>
  ...
</Slide>

Then the layout behaves correctly (in that the slide component renders 'transparently', with the child Grid component sizing working as expected).

Looking at the HTML.

The Slide component gets classNames associated with the Grid (which I assume is the children?):

<div class="MuiGrid-root MuiGrid-item MuiGrid-grid-xs-4" style="transform: none; transition: transform 225ms cubic-bezier(0, 0, 0.2, 1) 0ms;" />

Whereas with the Collapse component, unlike with the Slide, no inline styles are giving, and the classes don't include any classes from the children:

<div class="MuiCollapse-container MuiCollapse-horizontal MuiCollapse-entered" style="min-width: 0px;">
  <div class="MuiCollapse-wrapper MuiCollapse-horizontal">
    <div class="MuiCollapse-wrapperInner MuiCollapse-horizontal">
...

Is this intended?

Expected Behavior 🤔

Rewriting my JSX:

<Grid container>
  <Collapse
    timeout={3000}
    className={clsx({
      'MuiGrid-root': true,
      'MuiGrid-item': true,
      'MuiGrid-grid-xs-4': showSidebar ? true : false,
    })}
    orientation="horizontal"
    in={showSidebar}
  >
    <Grid item xs={showSidebar ? 12 : 4}>
      <Sidebar />
    </Grid>
  </Collapse>

  <Grid item xs>
    <ResultList />
  </Grid>
</Grid>

This somewhat achieves what I'm looking for but in a very jumpy, unsatisfactory way (the timout={3000} allows me to see where the problems are).

I apologize if I'm misusing the Transition component in this case! If so, I would be super grateful for pointers in the correct direction.

Context 🔦

I would like to be able to collapse a side panel in and out (on this page - https://atlas.saeon.ac.za/catalogue?terms=%2520SARVA. The filter icon in the right of the results header toggles the sidebar, but without an animation at the moment).

The structure of the page is a grid container with 2 grid items. On small screens the items are xs=12, and on larger screens the items are xs=4, and xs=8. I would like to wrap the xs=4 grid item in a collapse transition, but that is not working at the moment.

@zachsa zachsa added the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Jul 28, 2020
@zachsa
Copy link
Author

zachsa commented Jul 28, 2020

I opened this issue as a response to a pull request that added the Collapse functionality, but that code is where I think there is a bug (#20619)

@darkowic
Copy link
Contributor

Can you provide a simple reproduction example in codesandbox?

@zachsa
Copy link
Author

zachsa commented Jul 28, 2020

@zachsa zachsa changed the title Collapse component can't wrap Grid items correctly Collapse component doesn't wrap Grid items correctly Jul 28, 2020
@oliviertassinari
Copy link
Member

I think that we can close, the Grid item should be a direct descendant of the Grid container.

@zachsa
Copy link
Author

zachsa commented Jul 28, 2020

@oliviertassinari, I see that. why does this work with the Slide component?

@darkowic
Copy link
Contributor

darkowic commented Jul 28, 2020 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: Collapse The React component
Projects
None yet
Development

No branches or pull requests

3 participants