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.

Sign up for GitHub

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

Jump to bottom

[Grid2] Export Grid2Classes interface #44420

Merged
merged 16 commits into from
Nov 26, 2024
Merged

[Grid2] Export Grid2Classes interface #44420

merged 16 commits into from
Nov 26, 2024

Conversation

sai6855
Copy link
Contributor

@sai6855 sai6855 commented Nov 15, 2024 •
edited
Loading

@sai6855 sai6855 added bug 🐛 Something doesn't work component: Grid The React component. package: material-ui Specific to @mui/material labels Nov 15, 2024
@mui-bot
Copy link

mui-bot commented Nov 15, 2024 •
edited
Loading

Netlify deploy preview

https://deploy-preview-44420--material-ui.netlify.app/

Bundle size report

Details of bundle changes (Toolpad)
Details of bundle changes

Generated by 🚫 dangerJS against 31e97e1

@sai6855 sai6855 requested a review from siriwatknp November 15, 2024 12:52
@oliviertassinari oliviertassinari changed the title [material-ui][Grid2] Export Grid2Classes interface [Grid2] Export Grid2Classes interface Nov 15, 2024
@oliviertassinari oliviertassinari removed the package: material-ui Specific to @mui/material label Nov 15, 2024
ZeeshanTamboli
ZeeshanTamboli reviewed Nov 22, 2024
View reviewed changes
Copy link
Member

@ZeeshanTamboli ZeeshanTamboli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we add a test? I think this also needs to be updated: https://github.com/mui/material-ui/blob/master/packages/mui-material/src/styles/overrides.d.ts#L193.

packages/mui-material/src/Grid2/grid2Classes.ts Outdated Show resolved Hide resolved
packages/mui-material/src/Grid2/grid2Classes.ts Outdated Show resolved Hide resolved
ZeeshanTamboli
ZeeshanTamboli reviewed Nov 22, 2024
View reviewed changes
Copy link
Member

@ZeeshanTamboli ZeeshanTamboli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, but I'll let @DiegoAndai give a final review.

ZeeshanTamboli
ZeeshanTamboli approved these changes Nov 26, 2024
View reviewed changes
Copy link
Member

@ZeeshanTamboli ZeeshanTamboli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, I think we can merge this. Looks good.

@sai6855 sai6855 merged commit 18eee29 into mui:master Nov 26, 2024
23 checks passed
@smerling-coremedia
Copy link

thank you for getting this fixed so fast :) It will really help me out in the next release! Thanks!!!

@DiegoAndai
Copy link
Member

Hey, I'm so sorry. I'm just reviewing this now.

I prefer reverting this and split it into separate PRs:

What do you think @sai6855 @ZeeshanTamboli

Again, I'm sorry for the delay and the inconvenience.

@DiegoAndai DiegoAndai mentioned this pull request Nov 26, 2024
Merged
1 task
@ZeeshanTamboli
Copy link
Member

I agree. We should reconsider if all these dynamic classes are necessary. For now, container and root are enough. @sai6855, could you handle splitting it?

@sai6855
Copy link
Contributor Author

sai6855 commented Nov 26, 2024 •
edited
Loading

Okay, I'll revert this PR and split changes into 2 PRs

sai6855 added a commit to sai6855/material-ui that referenced this pull request Nov 26, 2024
@sai6855
Revert "[Grid2] Export Grid2Classes interface (mui#44420)"
799a8b9 
This reverts commit 18eee29.
DiegoAndai pushed a commit that referenced this pull request Nov 26, 2024
@sai6855
[Grid2] Revert Export Grid2Classes interface (#44420) (#44560)
dfe7067
@sai6855 sai6855 mentioned this pull request Nov 27, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ZeeshanTamboli ZeeshanTamboli ZeeshanTamboli approved these changes

@siriwatknp siriwatknp Awaiting requested review from siriwatknp

@DiegoAndai DiegoAndai Awaiting requested review from DiegoAndai

Assignees
No one assigned
Labels
bug 🐛 Something doesn't work component: Grid The React component. typescript
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[material-ui][Grid2] styleOverride does not work for container, only root
6 participants
@sai6855 @mui-bot @smerling-coremedia @DiegoAndai @ZeeshanTamboli @oliviertassinari