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][utils] Merge sx instead of override when using mergeSlotProps #45062

Merged
merged 4 commits into from
Jan 24, 2025

Conversation

siriwatknp
Copy link
Member

@siriwatknp siriwatknp commented Jan 20, 2025

closes #45041

mergeSlotProps currently does not merge the sx prop (it will override instead).
This PR fix the logic from overriding to merging for sx prop (similar to className and style).

I think this is the correct expected behavior.

@mui-bot
Copy link

mui-bot commented Jan 20, 2025

Netlify deploy preview

Bundle size report

Details of bundle changes (Toolpad)
Details of bundle changes

Generated by 🚫 dangerJS against 8dfa551

Comment on lines +49 to +50
const typedDefaultSlotProps = defaultSlotProps as Record<string, any>;
const className = clsx(typedDefaultSlotProps?.className, externalSlotProps?.className);
Copy link
Member Author

Choose a reason for hiding this comment

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

Logic is the same, just types no remove individual type casting below.

@siriwatknp siriwatknp added the package: material-ui Specific to @mui/material label Jan 23, 2025
@siriwatknp siriwatknp changed the title merge sx [material-ui][utils] Merge sx instead of override when using mergeSlotProps Jan 23, 2025
@siriwatknp siriwatknp marked this pull request as ready for review January 23, 2025 03:08
Copy link
Member

@aarongarciah aarongarciah left a comment

Choose a reason for hiding this comment

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

@siriwatknp should we consider this a breaking change?

@siriwatknp
Copy link
Member Author

@siriwatknp should we consider this a breaking change?

I think this is a bug fix instead because the current logic does not meet the expected behavior (merging instead of override).

@aarongarciah
Copy link
Member

I think this is a bug fix instead because the current logic does not meet the expected behavior (merging instead of override).

👍

@siriwatknp siriwatknp merged commit bc2337f into mui:master Jan 24, 2025
22 checks passed
@DiegoAndai
Copy link
Member

@siriwatknp if we want this to be available in v6, we need to cherry pick it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
package: material-ui Specific to @mui/material
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[TextField] sx prop does not exist in slotProps.input
4 participants