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

MUI Grid size breakpoints stopped working with SDS v.21.7.0 and above #923

Open
crispipear opened this issue Jan 10, 2025 · 4 comments
Open
Assignees
Labels
Bug Something isn't working

Comments

@crispipear
Copy link
Contributor

crispipear commented Jan 10, 2025

Describe the bug
After we upgraded SDS to any version from v.21.7.0 and above, the size breakpoints in MUI Grid component stopped working. However, in v.21.6.3, Grid still works as expected.

Example:

import Grid from '@mui/material/Grid';

<Grid
      xs={7} // size breakpoint no longer works, no more flex basis applied to the underlying CSS
      item
>
 {...}
</Grid>

Our MUI version:
"@mui/material": "5.15.19",

To Reproduce
Steps to reproduce the behavior:

  1. Use SDS to any versions after v.21.6.3
  2. Try using Grid with size breakpoints applied
  3. Inspect the CSS, flex-basis isn't applied in the Grid component

Expected behavior
MUI Grid still works as expected.

Screenshots
with SDS v.21.7.0
image

with SDS v.21.6.3
image

Desktop (please complete the following information):

  • OS: macOS
  • Browser Firefox
  • Version 129.0.1

Additional context
Comparing the differences between 21.7.0 and 21.6.3: @czi-sds/[email protected]...@czi-sds/[email protected]

  • there doesn't seem to be any dependency packages updates
  • could CSS formatter change impact how MUI components process Props?
@masoudmanson
Copy link
Contributor

Hi @crispipear,

I think I’ve identified the potential cause of this issue. In version 21.7.0, we introduced new SDS breakpoints (commit link). These breakpoints were implemented following MUI’s guidelines, but we made a key change: SDS only supports small, medium, and large breakpoints, and we decided to remove the xs and xl breakpoints.

From what I see in your code, it looks like you’re using the xs breakpoint, which might be causing the issue. Let me test this thoroughly and get back to you with a workaround or a hotfix if needed.

In the meantime, could you please confirm if replacing xs with sm restores the grid functionality?

Apologies for the inconvenience, and thank you for your patience!

@masoudmanson masoudmanson self-assigned this Jan 10, 2025
@masoudmanson masoudmanson added the Bug Something isn't working label Jan 10, 2025
@crispipear
Copy link
Contributor Author

Hi @crispipear,

I think I’ve identified the potential cause of this issue. In version 21.7.0, we introduced new SDS breakpoints (commit link). These breakpoints were implemented following MUI’s guidelines, but we made a key change: SDS only supports small, medium, and large breakpoints, and we decided to remove the xs and xl breakpoints.

From what I see in your code, it looks like you’re using the xs breakpoint, which might be causing the issue. Let me test this thoroughly and get back to you with a workaround or a hotfix if needed.

In the meantime, could you please confirm if replacing xs with sm restores the grid functionality?

Apologies for the inconvenience, and thank you for your patience!

Thank you for the context and looking into this @masoudmanson. Replacing xs with sm does restore the grid functionality!

The SDS breakpoint updates seem reasonable, and I believe adopting their definitions makes sense. I understand the rationale behind removing xs, as sm should effectively address smaller device use cases. Though I’m curious about the reasoning behind consolidating xl and using l as the breakpoint for anything over 1024px. From my perspective, standard laptop widths are typically around 1366px, while standard desktop widths are closer to 1920px, which creates a significant range. A lot of the times, the same UI layout doesn’t work equally well across both. Was this gap a factor considered during the Breakpoints rework?

@clarsen-czi
Copy link

Hi @crispipear,
I think I’ve identified the potential cause of this issue. In version 21.7.0, we introduced new SDS breakpoints (commit link). These breakpoints were implemented following MUI’s guidelines, but we made a key change: SDS only supports small, medium, and large breakpoints, and we decided to remove the xs and xl breakpoints.
From what I see in your code, it looks like you’re using the xs breakpoint, which might be causing the issue. Let me test this thoroughly and get back to you with a workaround or a hotfix if needed.
In the meantime, could you please confirm if replacing xs with sm restores the grid functionality?
Apologies for the inconvenience, and thank you for your patience!

Thank you for the context and looking into this @masoudmanson. Replacing xs with sm does restore the grid functionality!

The SDS breakpoint updates seem reasonable, and I believe adopting their definitions makes sense. I understand the rationale behind removing xs, as sm should effectively address smaller device use cases. Though I’m curious about the reasoning behind consolidating xl and using l as the breakpoint for anything over 1024px. From my perspective, standard laptop widths are typically around 1366px, while standard desktop widths are closer to 1920px, which creates a significant range. A lot of the times, the same UI layout doesn’t work equally well across both. Was this gap a factor considered during the Breakpoints rework?

Good question Su. Based on the research we did, even though there's a lot of potential screen widths above 1024px, we found there's not a lot of difference in layouts at these larger screen widths and adding additional breakpoints would simply add time and complexity for designers to consider without much actual benefit to the designs themselves. The layout changes that we saw at these wide widths were largely just wider versions of the same elements/components laid out the same way within the UI as opposed to wholly new layouts being used (as opposed to more narrow widths where layouts need to be rearranged or fully redesigned to fit all necessary content). When widths get really wide, like above 2800px or so, our recommendation is to include max-width on the content so things don't get too stretched out of proportion.

Hopefully that helps provide some context, but definitely let us know if you have any other questions.

@crispipear
Copy link
Contributor Author

Hi @crispipear,
I think I’ve identified the potential cause of this issue. In version 21.7.0, we introduced new SDS breakpoints (commit link). These breakpoints were implemented following MUI’s guidelines, but we made a key change: SDS only supports small, medium, and large breakpoints, and we decided to remove the xs and xl breakpoints.
From what I see in your code, it looks like you’re using the xs breakpoint, which might be causing the issue. Let me test this thoroughly and get back to you with a workaround or a hotfix if needed.
In the meantime, could you please confirm if replacing xs with sm restores the grid functionality?
Apologies for the inconvenience, and thank you for your patience!

Thank you for the context and looking into this @masoudmanson. Replacing xs with sm does restore the grid functionality!
The SDS breakpoint updates seem reasonable, and I believe adopting their definitions makes sense. I understand the rationale behind removing xs, as sm should effectively address smaller device use cases. Though I’m curious about the reasoning behind consolidating xl and using l as the breakpoint for anything over 1024px. From my perspective, standard laptop widths are typically around 1366px, while standard desktop widths are closer to 1920px, which creates a significant range. A lot of the times, the same UI layout doesn’t work equally well across both. Was this gap a factor considered during the Breakpoints rework?

Good question Su. Based on the research we did, even though there's a lot of potential screen widths above 1024px, we found there's not a lot of difference in layouts at these larger screen widths and adding additional breakpoints would simply add time and complexity for designers to consider without much actual benefit to the designs themselves. The layout changes that we saw at these wide widths were largely just wider versions of the same elements/components laid out the same way within the UI as opposed to wholly new layouts being used (as opposed to more narrow widths where layouts need to be rearranged or fully redesigned to fit all necessary content). When widths get really wide, like above 2800px or so, our recommendation is to include max-width on the content so things don't get too stretched out of proportion.

Hopefully that helps provide some context, but definitely let us know if you have any other questions.

The context definitely helps! Thank you Connor!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants