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

[Joy] Refine componentsProps for all components #34077

Merged
merged 49 commits into from
Aug 28, 2022

Conversation

siriwatknp
Copy link
Member

@siriwatknp siriwatknp commented Aug 26, 2022

a continuation of #34022 which apply to all components in this PR.

  • all components (except TextField) follow MUI Base componentsProps which can be a callback.
  • exported {Component}OwnerState types to be used in styleOverrides.
  • add theme components spec to ensure that the ownerState inside the callback is typed.
  • add a custom variant test to describeConformance
  • fixed some components that have wrong variant and color proptypes.

@siriwatknp siriwatknp added the package: joy-ui Specific to @mui/joy label Aug 26, 2022
@mui-bot
Copy link

mui-bot commented Aug 26, 2022

Details of bundle changes

@mui/joy: parsed: +0.34% , gzip: -0.16% 😍

Generated by 🚫 dangerJS against eb4d19e

@siriwatknp siriwatknp marked this pull request as draft August 26, 2022 07:29
@siriwatknp siriwatknp marked this pull request as ready for review August 28, 2022 13:39
@siriwatknp siriwatknp merged commit 179cc07 into mui:master Aug 28, 2022
@@ -49,13 +49,15 @@ export default function ExampleChoiceChipCheckbox() {
setValue((val) => val.filter((text) => text !== item));
}
}}
sx={{
Copy link
Member

@oliviertassinari oliviertassinari Aug 28, 2022

Choose a reason for hiding this comment

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

While improving the support for componentsProps in all the Joy UI components sounds great, for adding custom attributes, and events. I wonder if using it for customization over the CSS selectors is a step forward. Personally, I found the previous approach better. We teach for customizations that are not one-off to use the styled() API, which was easier to migrate to with the sx demo.

Another way to look at it, SASS is still the most popular way to style: https://2021.stateofcss.com/en-US/technologies/#scatterplot_overview. How would somebody using SASS would figure out how to migrate the customization?

Copy link
Member Author

Choose a reason for hiding this comment

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

I think this is not up to Joy and I don't even think which one is better. It's all up to the developers to choose the way they prefer. I will ensure that both ways are documented.

Copy link
Member

@oliviertassinari oliviertassinari Aug 29, 2022

Choose a reason for hiding this comment

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

It's all up to the developers to choose the way they prefer.

@siriwatknp It's an interesting perspective. I had tried this path in the past (my initial intuition of how it should be done) and stopped once I saw too many developers struggling with deciding which option they should use. I saw them ask for the documentation to give them "the way". For example, in our last developer survey: https://www.notion.so/mui-org/Raw-data-aa374141dcb3453dbfea301dcc437126#8830e74c200946dcb29ce385684f1dc4

10 | more opinionated
2 | less opinionated

Maybe it would make sense to first decide on which approach is better, and then stick to it everywhere in the docs 😁

daniel-rabe pushed a commit to daniel-rabe/material-ui that referenced this pull request Nov 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
package: joy-ui Specific to @mui/joy
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants