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

[material-ui][Accordion] Improve slots.transition type #42993

Closed

Conversation

aarongarciah
Copy link
Member

@aarongarciah aarongarciah commented Jul 18, 2024

Resolves #40650

@aarongarciah aarongarciah added typescript component: accordion This is the name of the generic UI component, not the React module! package: material-ui Specific to @mui/material labels Jul 18, 2024
@mui-bot
Copy link

mui-bot commented Jul 18, 2024

Netlify deploy preview

Bundle size report

No bundle size changes (Toolpad)
No bundle size changes

Generated by 🚫 dangerJS against 0f34fc2

TransitionComponent?: React.JSXElementConstructor<
TransitionProps & { children?: React.ReactElement<any, any> }
TransitionComponent?: React.ElementType<
TransitionProps & { children: React.ReactElement<any, any> }
Copy link
Member Author

@aarongarciah aarongarciah Jul 19, 2024

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:

image

We could also improve the type by either:

  • Make children required in TransitionProps, but that would require more changes and there are places where children probably is not required.
  • Update transition components and make children optional. Again, that means more changes.

Copy link
Member

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?

Copy link
Member Author

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.

Copy link
Member

Choose a reason for hiding this comment

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

Thanks!

Copy link
Member Author

Choose a reason for hiding this comment

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

Updated: #40650

@aarongarciah aarongarciah requested a review from DiegoAndai July 19, 2024 15:53
@aarongarciah aarongarciah marked this pull request as ready for review July 19, 2024 15:53
@aarongarciah aarongarciah changed the title [material-ui][Accordion] Improve slots.transition and TransitionComponent type [material-ui][Accordion] Improve slots.transition type Jul 22, 2024
@aarongarciah aarongarciah force-pushed the accordion-transition-children-type branch from eebfa05 to 7663a21 Compare July 22, 2024 15:25
@aarongarciah aarongarciah force-pushed the accordion-transition-children-type branch from 7663a21 to 0f34fc2 Compare July 22, 2024 15:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change component: accordion This is the name of the generic UI component, not the React module! package: material-ui Specific to @mui/material typescript
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[material-ui] Improve slot.transition and TransitionProps types
3 participants