-
-
Notifications
You must be signed in to change notification settings - Fork 32.5k
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
[material-ui][PaginationItem] Deprecate components prop #41777
[material-ui][PaginationItem] Deprecate components prop #41777
Conversation
Netlify deploy previewPagination: parsed: +2.74% , gzip: +2.57% Bundle size reportDetails of bundle changes (Toolpad) |
Below tests are not removed , because material-ui/packages/mui-material/src/PaginationItem/PaginationItem.test.js Lines 29 to 31 in 73c88b6
I see 2 inconsistencies in
Due to these inconsistencies following tests are not passed ( there are many more, i didn't listed all) on removing
|
….com/sai6855/material-ui into deprecate-pagination-item-components
No, it didn't helped. even on removing |
I see. Some tests can be fixed by removing So slots are not always rendered, as |
@DiegoAndai shall i go ahead with this approach? |
Yes, let's do it. It makes sense to me to cover this custom case separately. Let's add a comment pointing out that the slots tests are covered outside of |
@DiegoAndai added tests to test conditional slots |
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.
Thanks for updating the tests @sai6855, I have a couple of follow-up comments. Let me know what you think 😊
packages/mui-codemod/src/deprecations/pagination-item-props/test-cases/theme.actual.js
Outdated
Show resolved
Hide resolved
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.
Great job @sai6855!
part of #41279