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

Breakpoints order on grid direction shouldn't affect the behaviour #29175

Closed
2 tasks done
lunatupini opened this issue Oct 20, 2021 · 5 comments · Fixed by #29300
Closed
2 tasks done

Breakpoints order on grid direction shouldn't affect the behaviour #29175

lunatupini opened this issue Oct 20, 2021 · 5 comments · Fixed by #29300
Labels
bug 🐛 Something doesn't work component: Grid The React component. component: masonry This is the name of the generic UI component, not the React module! component: Stack The React component. ready to take Help wanted. Guidance available. There is a high chance the change will be accepted

Comments

@lunatupini
Copy link

lunatupini commented Oct 20, 2021

Breakpoints order on <Grid> direction props are affecting behaviour. IMHO that's not the intended behaviour.

E.g

  • 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 😯

This is my current component:

<Grid
  direction={{ sm: 'row', xs: 'column' }}
  container
  justifyContent="space-between"
  columnSpacing={{ xs: 0, sm: 4 }}
  sx={{
  margin: {
    xs: theme.spacing(0, 'auto'),
    sm: theme.spacing(0, 0, 0, -4),
  },  
  }}
>

  <Grid item xs={4} sx={{ paddingTop: { xs: 7, sm: 0 } }}>
    foobar
  </Grid>
</Grid>

Although it's defined as row SM is showing as a column. If I change the order it works.
E.g: <Grid direction={{ xs: 'column', sm: 'row' }}>

Expected Behavior 🤔

SM size should be row

Your Environment 🌎

`npx @mui/envinfo`
  System:
    OS: Windows 10 10.0.19043
  Binaries:
    Node: 14.18.1 - C:\Program Files\nodejs\node.EXE
    Yarn: 3.0.2 - C:\Program Files\nodejs\yarn.CMD
    npm: 8.1.0 - C:\Program Files\nodejs\npm.CMD
  Browsers:
    Chrome: 94.0.4606.81
    Edge: Spartan (44.19041.1266.0), Chromium (94.0.992.50)
  npmPackages:
    @emotion/react: 11.4.1 => 11.4.1
    @emotion/styled: 11.3.0 => 11.3.0
    @mui/core:  5.0.0-alpha.51
    @mui/material: 5.0.4 => 5.0.4
    @mui/private-theming:  5.0.1
    @mui/styled-engine:  5.0.1
    @mui/styles: 5.0.1 => 5.0.1
    @mui/system:  5.0.4
    @mui/types:  7.0.0
    @mui/utils:  5.0.1
    @types/react: 17.0.30 => 17.0.30
    react: 17.0.2 => 17.0.2
    react-dom: 17.0.2 => 17.0.2
    typescript: 4.4.3 => 4.4.3
@lunatupini lunatupini added the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Oct 20, 2021
@mnajdova
Copy link
Member

Thanks for the report. Looks like a bug indeed. In the style function for the sx prop we are already handling it, but looks like it is not being handled on the props of the components: Grid, Masonry etc. We should fix it.

The way it works in the styleFunctionSx is:

  1. create an object with empty breakpoints in order (from xs to xl)
  2. merge the other props value/styles
  3. remove the unused breakpoints

This will ensure those are always in order. I am marking it as good to take, feel free to take it if you would like to. cc @hbjORbj may be interested to take a look to.

@mnajdova mnajdova added bug 🐛 Something doesn't work component: Grid The React component. component: masonry This is the name of the generic UI component, not the React module! component: Stack The React component. 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 Oct 20, 2021
@Razaik25
Copy link

If no one else has picked it up, would love to work on this. Please let me know, thanks

@hbjORbj
Copy link
Member

hbjORbj commented Oct 20, 2021

@Razaik25 Sure, you can go ahead and request a review from me. I will be happy to help.

@hbjORbj
Copy link
Member

hbjORbj commented Oct 21, 2021

@Razaik25 Any status?

@Razaik25
Copy link

@Razaik25 Any status?

I have not looked into it yet, will do this weekend. But if you have starting points, that would really help.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 Something doesn't work component: Grid The React component. component: masonry This is the name of the generic UI component, not the React module! component: Stack The React component. 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