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

[mui-codemod][AccordionSummary] Add contentGutters deprecation codemods #41006

Merged
merged 18 commits into from
Feb 15, 2024

Conversation

DiegoAndai
Copy link
Member

@DiegoAndai DiegoAndai commented Feb 8, 2024

Part of: #40417

Summary

This PR adds codemods for the AccordionSummary's contentGutters composed class deprecation. It covers the customization use cases on our docs (https://mui.com/material-ui/customization/how-to-customize/): the sx prop, the theme, the styled function, and plain CSS.

CSS transform support

Previously, we only had JS transforms. This PR adds support for CSS transforms using postcss and postcss-cli. It was necessary to use postcss in an unusual (but supported) way to keep the same codemod CLI API. The way it works is:

  • A codemod must have JS transforms, and CSS transforms are optional
  • To add a CSS transform to a codemod:
    • Add a <codemod>/postcss-plugin.js file that implements and exports the postcss plugin
    • Add a <codemod>/postcss.config.js file that imports the plugin and exports the config with only that plugin. This is the only way I could find to load local plugins.
  • For deprecations/all, we have a separate postcss.config.js that imports all the plugins (which is only one for now)

If the user runs a specific codemod, then <codemod>/postcss.config.js is used so only that codemod's plugin is applied. If the users runs the deprecations/all codemod, then deprecations/all/postcss.config.js is used so all codemod plugins are applied.

Other styling options

In theory, we could leverage postcss to support other style approaches, like Sass or Less. This could be done via the codemod's CLI. This PR doesn't implement this, but we can do if the community asks for it.

Child class

The contentGutters class is applied to the content child, not the AccordionSummary root. The content class is also applied to the content child, but the gutters class is applied to the root. Because of this, we only cover contentGutters used with a child selector ( .contentGutters ). The gutters class is applied without the child selector, and the content class keeps the child selector: .gutters .content.

@DiegoAndai DiegoAndai added the package: codemod Specific to @mui/codemod label Feb 8, 2024
@DiegoAndai DiegoAndai self-assigned this Feb 8, 2024
@mui-bot
Copy link

mui-bot commented Feb 8, 2024

@DiegoAndai DiegoAndai changed the title [mui-codemod] Add AccordionSummary contentGutters deprecation codemods [mui-codemod][AccordionSummary] Add contentGutters deprecation codemods Feb 8, 2024
@DiegoAndai DiegoAndai added the component: accordion This is the name of the generic UI component, not the React module! label Feb 8, 2024
@DiegoAndai DiegoAndai marked this pull request as ready for review February 8, 2024 17:53
@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Feb 12, 2024
Signed-off-by: Diego Andai <diego@mui.com>
@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Feb 12, 2024
Copy link
Member

@siriwatknp siriwatknp left a comment

Choose a reason for hiding this comment

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

Please see my comments about the styleOverrides and overridesResolver. The rest looks good 👍

@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Feb 13, 2024
@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Feb 13, 2024
@DiegoAndai
Copy link
Member Author

Thanks for the review @siriwatknp! I implemented the fixes

Also, I reduced the scope of the codemods to only pickup the contentGutters class when used with a child selector. Otherwise, it would be difficult to determine how to introduce the gutters class. This covers less, but I think it's enough, and if it isn't, we should wait until a user requests it before diving into that complex case.

@siriwatknp
Copy link
Member

Thanks for the review @siriwatknp! I implemented the fixes

Also, I reduced the scope of the codemods to only pickup the contentGutters class when used with a child selector. Otherwise, it would be difficult to determine how to introduce the gutters class. This covers less, but I think it's enough, and if it isn't, we should wait until a user requests it before diving into that complex case.

Agree!

Copy link
Member

@siriwatknp siriwatknp left a comment

Choose a reason for hiding this comment

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

👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: accordion This is the name of the generic UI component, not the React module! package: codemod Specific to @mui/codemod
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants