-
-
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
[material-ui][Accordion] Improve slots.transition type #42993
[material-ui][Accordion] Improve slots.transition type #42993
Conversation
Netlify deploy previewBundle size report |
TransitionComponent?: React.JSXElementConstructor< | ||
TransitionProps & { children?: React.ReactElement<any, any> } | ||
TransitionComponent?: React.ElementType< | ||
TransitionProps & { children: React.ReactElement<any, any> } |
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.
We are doing this in at least these components:
We could also improve the type by either:
- Make
children
required inTransitionProps
, but that would require more changes and there are places wherechildren
probably is not required. - Update transition components and make
children
optional. Again, that means more changes.
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 agree with you: when we make the change we should be consistent with it. At this point, because it's not a pressing issue (there's no users requests for it), I'm thinking we should:
- Update the original issue to include all the components in the scope
- Take it out of v6's scope
This way we avoid the unrelated breaking change of a non urgent issue.
What do you think?
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.
Makes sense. I'll update the issue and remove it from the v6 milestone.
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!
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.
Updated: #40650
eebfa05
to
7663a21
Compare
7663a21
to
0f34fc2
Compare
Resolves #40650