-
-
Notifications
You must be signed in to change notification settings - Fork 32.4k
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
[Grid] Fix horizontal scrollbar and nested dimensions #24332
Conversation
8a51140
to
588e4e7
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have polished it, looks good otherwise
588e4e7
to
931c71c
Compare
@greguintow I have reverted the width nested handline. It's not something we can fix in the current configuration. Notice the explosion of the CSS generated: PR One step at a time. We first need to migrate the Grid to emotion, then we should be able to apply the logic back and remove the !important by increasing specificity. This solves the concern of @eps1lon as we do no longer need the !important style. |
It's showing 714 grid classes found but you have to consider this part of the code. return {
...obj,
[`&$container$item$spacing-xs-${spacing}`]: {
flexBasis: fullWidth,
maxWidth: fullWidth,
},
}; This part shows that each breakpoint size will have this, but in that line have 3 .Mui-Grid, because will be .MuiGrid-container.MuiGrid-item.MuiGrid-spacing-xs-y. So the real amount of Mui-Grid would be 238 if we divide it by 3. |
@greguintow True, the only issue is that I already account for this in the search (notice all the whitespace prefixing the query). I would have found 2.5k matches if I didn't (x3) 😁 |
LOOL, okay |
@greguintow But it's OK. We can work on the migration to emotion then applying the same solution. The size of the generated CSS should no longer be a concern. |
@oliviertassinari Alright sounds good! Does the migration to emotion change the component style in some way? Because I'm having to use version 5.0.0-alpha.20, because the Button migration made my app change completely, I think I should create an issue about it. |
You can find 10 duplicate issues about this in the issue tracker. There is a documented workaround only necessary during the phase were we have both jss and the new styled API. |
I'm rebasing on top of #24395 |
065a948
to
bd56d2b
Compare
@greguintow Thanks for sharing the solution and spending time on it :) |
Fixes #7466
This PR is to be a way to implement what #24299 does without creating a new prop.