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

[docs][material-ui][theme] Add documentation on migrating JSS's alternative, array-based syntax to syntax supported by Emotion #42053

Open
wants to merge 2 commits into
base: next
Choose a base branch
from

Conversation

cjl750
Copy link

@cjl750 cjl750 commented Apr 28, 2024

Added a section to the page on breaking style and theme changes in v5 about migrating the alternative, array-based syntax for space- and comma-separated values to string-based values necessary for use with Emotion.

Since everything under the heading "Migrate theme styleOverrides to Emotion" was about refactoring local rule references, I opted to take the content in that section and put it under a new sub-header (so the existing h3s became h4s), then I added a second sub-header for the new section about migrating the alternative, array-based syntax.

This is to resolve #41971

… array-based syntax to syntax supported by Emotion
@mui-bot
Copy link

mui-bot commented Apr 28, 2024

Netlify deploy preview

Bundle size report

No bundle size changes (Toolpad)
No bundle size changes

Generated by 🚫 dangerJS against 4850582

@zannager zannager added package: material-ui Specific to @mui/material customization: theme Centered around the theming features labels Apr 29, 2024
Copy link
Member

@ZeeshanTamboli ZeeshanTamboli left a comment

Choose a reason for hiding this comment

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

@cjl750 Thanks for the pull request. It looks good to me! We need another review from @DiegoAndai to confirm whether we should consider documenting this, as it wasn't requested before and doesn't seem to be a common use case. However, I see no issue with documenting it.

@ZeeshanTamboli ZeeshanTamboli added the docs Improvements or additions to the documentation label May 11, 2024
@ZeeshanTamboli ZeeshanTamboli changed the title [material-ui][theme] Add documentation on migrating JSS's alternative, array-based syntax to syntax supported by Emotion [docs][material-ui][theme] Add documentation on migrating JSS's alternative, array-based syntax to syntax supported by Emotion May 11, 2024
Copy link
Member

@DiegoAndai DiegoAndai left a comment

Choose a reason for hiding this comment

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

Hey @cjl750! Thanks for working on this

Correct me if I'm wrong, but the usage in v4 would be:

const theme = createTheme({
  overrides: {
    MuiOutlinedInput: {
      root: {
        padding: [[20, 10]],
      },
    },
  },
});

(Codesandbox)

If so, we should include this as the starting point.

@cjl750
Copy link
Author

cjl750 commented May 15, 2024

Hi @DiegoAndai

Correct me if I'm wrong, but the usage in v4 would be:

const theme = createTheme({
  overrides: {
    MuiOutlinedInput: {
      root: {
        padding: [[20, 10]],
      },
    },
  },
});

You're right, that would be what you'd have in your v4 theme. I was keeping the pattern in the diff for the local rule overrides in that same section of the page, which shows the theme already converted to use styleOverrides. Now that you mention it, though, that does seem a bit out of order.

Looks like converting to styleOverrides is covered further down the page, under the heading "Theme structure." Perhaps it'd make more sense to rearrange the page a bit to move the "Theme structure" section toward the top and then move "Migrate theme styleOverrides to Emotion" below that? I can work that into this PR if you want, but rearranging seems like a good idea to me regardless of what happens with this PR, so maybe you all would prefer to do it separately. Let me know what you think.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
customization: theme Centered around the theming features docs Improvements or additions to the documentation package: material-ui Specific to @mui/material
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[material-ui][theme] Array syntax doesn't work for values in styleOverrides in theme
5 participants