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] Convert Material UI components to support Pigment CSS #41273

Closed
62 tasks done
siriwatknp opened this issue Feb 26, 2024 · 26 comments · Fixed by #42693
Closed
62 tasks done

[material-ui] Convert Material UI components to support Pigment CSS #41273

siriwatknp opened this issue Feb 26, 2024 · 26 comments · Fixed by #42693
Assignees
Labels
package: material-ui Specific to @mui/material umbrella For grouping multiple issues to provide a holistic view v6.x

Comments

@siriwatknp
Copy link
Member

siriwatknp commented Feb 26, 2024

Left over components

Thank you for past contributions

Contribution is welcome

The focus of Material UI v6 is to support static CSS extraction, and we are excited to invite the community to be a part of it!

For short context, the static extraction is done by our in-house styling-engine package, aka Pigment CSS. You can think of it as a replacement for Emotion/Styled-components. We must add an intermediate path to let the components support both Pigment CSS and Emotion.

The goal of this issue is to track the progress of the work with guidance on how to contribute and check the result. Explanation about Pigment CSS is out-of-scope. But you can visit the README for more info.

Contributing

  1. Pick a component from the Ready-to-take section below. Tag @siriwatknp in the comment to assign to you (for example, if you take Accordion, Accordion* must be included in your PR).

  2. Fork the repo (if you are a new contributor, please check the contributing first) and open the component implementation, e.g. packages/mui-material/src/Avatar/Avatar.js.

  3. Change the path import of these APIs, styled, useThemeProps, keyframes to ../zero-styled:

    - import styled from '../styles/styled';
    - import useThemeProps from '../styles/useThemeProps';
    + import { styled, createUseThemeProps } from '../zero-styled';
    …the rest of the imports
    
    const useThemeProps = createUseThemeProps('MuiAvatar');
    
    …

    💡 For useThemeProps, replace it with createUseThemeProps and call the function with a string that has the same value as useThemeProps({ props: inProps, name: <string> }) in the component implementation. Take a look at the Alert PR for example.

  4. Ensure that the Component.propTypes is followed by /* remove-proptypes */ directive.

  5. Check the component before attaching properties, e.g. in Divider:

    // packages/mui-material/src/Divider/Divider.js:222
    - Divider.muiSkipListHighlight = true;
    + if (Divider) {
    +   Divider.muiSkipListHighlight = true;
    + }
  6. Update styles implementation, see Converting styles below

Converting styles

Most of the time, you will have to convert styles interpolation to variants.

Move ownerState from the root style argument callback to variants

Before:

const AccordionRoot = styled(Paper, {})({ theme, ownerState }) => ({
    ...(!ownerState.square && {
      borderRadius: 0,
      '&:first-of-type': {
        borderTopLeftRadius: (theme.vars || theme).shape.borderRadius,
        borderTopRightRadius: (theme.vars || theme).shape.borderRadius,
      },
      '&:last-of-type': {
        borderBottomLeftRadius: (theme.vars || theme).shape.borderRadius,
        borderBottomRightRadius: (theme.vars || theme).shape.borderRadius,
        // Fix a rendering issue on Edge
        '@supports (-ms-ime-align: auto)': {
          borderBottomLeftRadius: 0,
          borderBottomRightRadius: 0,
        },
      },
    }),
    ...(!ownerState.disableGutters && {
      [`&.${accordionClasses.expanded}`]: {
        margin: '16px 0',
      },
    }),
  })

After:

const AccordionRoot = styled(Paper, {})({ theme }) => ({ // there must be NO `ownerState` here.
    variants: [
      {
        props: { square: false },
        style: {
          borderRadius: 0,
          '&:first-of-type': {
            borderTopLeftRadius: (theme.vars || theme).shape.borderRadius,
            borderTopRightRadius: (theme.vars || theme).shape.borderRadius,
          },
          '&:last-of-type': {
            borderBottomLeftRadius: (theme.vars || theme).shape.borderRadius,
            borderBottomRightRadius: (theme.vars || theme).shape.borderRadius,
            // Fix a rendering issue on Edge
            '@supports (-ms-ime-align: auto)': {
              borderBottomLeftRadius: 0,
              borderBottomRightRadius: 0,
            },
          },
        },
      },
      {
        props: { disableGutters: false },
        style: {
          [`&.${accordionClasses.expanded}`]: {
            margin: '16px 0',
          }
        }
      }
    ]
  })
Use Object.entries(theme.palette) to populate colors

Before:

({ theme, ownerState }) => {
  return {
    ...theme.typography.body2,
    backgroundColor: 'transparent',
    display: 'flex',
    padding: '6px 16px',
    ...(ownerState.color &&
      ownerState.variant === 'standard' && {
        color: theme.vars
          ? theme.vars.palette.Alert[`${color}Color`]
          : getColor(theme.palette[color].light, 0.6),
        backgroundColor: theme.vars
          ? theme.vars.palette.Alert[`${color}StandardBg`]
          : getBackgroundColor(theme.palette[color].light, 0.9),
        [`& .${alertClasses.icon}`]: theme.vars
          ? { color: theme.vars.palette.Alert[`${color}IconColor`] }
          : {
              color: theme.palette[color].main,
            },
      }),
}

After:

({ theme }) => {
  return {
    ...theme.typography.body2,
    backgroundColor: 'transparent',
    display: 'flex',
    padding: '6px 16px',
    variants: [
      ...Object.entries(theme.palette)
        .filter(([, value]) => value.main && value.light) // check all the used fields in the style below
        .map(([color]) => ({
          props: { colorSeverity: color, variant: 'standard' },
          style: {
            color: theme.vars
              ? theme.vars.palette.Alert[`${color}Color`]
              : getColor(theme.palette[color].light, 0.6),
            backgroundColor: theme.vars
              ? theme.vars.palette.Alert[`${color}StandardBg`]
              : getBackgroundColor(theme.palette[color].light, 0.9),
            [`& .${alertClasses.icon}`]: theme.vars
              ? { color: theme.vars.palette.Alert[`${color}IconColor`] }
              : {
                  color: theme.palette[color].main,
                },
          },
        })),
    ]
}

Example: https://github.com/mui/material-ui/pull/41230/files?diff=unified&w=0#diff-c2a97485bf897f10a4ae2116d86ea7d6eaed078be9781d4181b1a5bbf02ae170R60-R77

Render demos

  1. pnpm install once
  2. Run the script using node scripts/pigmentcss-render-mui-demos.mjs react-alert (replace react-alert with the component you are working on; the react-* must be one of https://mui.com/material-ui/*
  3. Update the component and build all packages once with pnpm build. (If you update the component again, you only need to build mui-material package with pnpm --filter @mui/material run build)
  4. cd apps/pigment-css-next-app && pnpm dev
  5. Open localhost:3000/material-ui/react-<component> to check the result
  6. Attach the screenshot or recording to the PR.
  7. If you encounter any errors, please attach a screenshot of the error in the PR comment. (Feel free to open the PR even if you got an error)

Open a PR

  1. using a title [material-ui][<Component>] Convert to support CSS extraction
  2. Tag @siriwatknp to review
  3. The argos CI should be green (this ensures that it still works with emotion/styled-components)

Ready-to-take Components

Will be done by a Codemod

#41743

#42001

  • Icon (has .muiName attached)
  • Dialog (contains useTheme())
    • DialogActions
    • DialogContent
    • DialogContentText
    • DialogTitle
  • Drawer (contains RTL logic)
  • Input (has .muiName attached)
  • Menu (contains useTheme())
    • MenuItem
    • MenuList
  • LinearProgress (wait for POC from CircularProgress) [material-ui] Convert LinearProgress to support Pigment CSS #41816
  • Paper - uses useTheme
  • Skeleton (wait for POC from CircularProgress)
  • Snackbar (contains useTheme())
    • SnackbarContent
  • SpeedDial (contains useTheme())
    • SpeedDialAction
    • SpeedDialIcon
  • Tabs (contains RTL logic)
    • TabScrollButton
    • Tab
  • Tooltip (contains RTL logic)

Waiting for 👍

  • Link
    • Need solution for extendSxProp
  • Typography
    • Need solution for extendSxProp
  • Box
  • SwipeableDrawer
@siriwatknp siriwatknp added umbrella For grouping multiple issues to provide a holistic view package: material-ui Specific to @mui/material v6.x labels Feb 26, 2024
@siriwatknp siriwatknp added this to the Material UI: v6 milestone Feb 26, 2024
@siriwatknp siriwatknp moved this to In progress in Pigment CSS Feb 29, 2024
@siriwatknp siriwatknp removed this from Pigment CSS Feb 29, 2024
@siriwatknp siriwatknp changed the title [WIP][material-ui][zero] Convert Material UI components to support Zero [material-ui][zero] Convert Material UI components to support Zero Mar 1, 2024
@siriwatknp siriwatknp changed the title [material-ui][zero] Convert Material UI components to support Zero [material-ui][zero] Convert Material UI to support static CSS extraction Mar 2, 2024
@siriwatknp siriwatknp changed the title [material-ui][zero] Convert Material UI to support static CSS extraction [material-ui][zero] Convert Material UI to support CSS extraction Mar 2, 2024
@danilo-leal danilo-leal moved this to In progress in Material UI Mar 5, 2024
@danilo-leal danilo-leal changed the title [material-ui][zero] Convert Material UI to support CSS extraction [material-ui] Convert Material UI to support CSS extraction Mar 5, 2024
@danilo-leal danilo-leal changed the title [material-ui] Convert Material UI to support CSS extraction [material-ui] Convert Material UI components to support CSS extraction Mar 5, 2024
@zanivan
Copy link
Contributor

zanivan commented Mar 13, 2024

Hey @siriwatknp on step 4 of Render demos, shouldn't it be cd apps/pigment-css-next-app && pnpm install && pnpm dev instead of cd apps/pigment-next-app && pnpm install && pnpm dev?

@cu8code
Copy link

cu8code commented Mar 15, 2024

@siriwatknp would like to work on Checkbox

@DiegoAndai
Copy link
Member

DiegoAndai commented Mar 18, 2024

I assigned the Breadcrumbs component to myself to work on them this week 👍🏼

It uses Typography though 🤔

const BreadcrumbsRoot = styled(Typography, {
  name: 'MuiBreadcrumbs',
  slot: 'Root',
  overridesResolver: (props, styles) => {
    return [{ [`& .${breadcrumbsClasses.li}`]: styles.li }, styles.root];
  },
})({});

Is that a blocker given that the Typography component is on hold?

@siriwatknp
Copy link
Member Author

I assigned the Breadcrumbs component to myself to work on them this week 👍🏼

@DiegoAndai the Breadcrumbs was taken by aacevski a while ago #41496

@siriwatknp
Copy link
Member Author

@siriwatknp would like to work on Checkbox

Assigned, thanks!

@DiegoAndai
Copy link
Member

@DiegoAndai the Breadcrumbs was taken by aacevski a while ago #41496

😅

@siriwatknp is the Chip taken? If not, I can take it.

@aacevski
Copy link
Contributor

aacevski commented Mar 19, 2024

I'd like to take Popover. I have opened a PR: #41564

Next up, I'd like to work on the Card component.

@siriwatknp
Copy link
Member Author

is the Chip taken? If not, I can take it.

Assigned.

Next up, I'd like to work on the Card component.

Assigned.

@siriwatknp
Copy link
Member Author

Can I pick up toolbar? @siriwatknp

Please do, thanks!

@piotrkrajewskicn
Copy link

Hi @siriwatknp
I'm working on a project dependent on MUI and we would need the zero runtime CSS for a next step in the project.
We're happy to contribute with our time to the MUI repo with the rewrite if it can help to speed up the works and the new version release.
But basically wondering if us spending some extra time will actually make things faster or is there a fixed release schedule anyway?

@lhilgert9
Copy link
Contributor

lhilgert9 commented Apr 2, 2024

@siriwatknp I'll take the Select-component. Should I also change the NativeSelectInput or is that something for the Input-component.

@siriwatknp
Copy link
Member Author

@siriwatknp I'll take the Select-component. Should I also change the NativeSelectInput or is that something for the Input-component.

The Input needs a GlobalStyles (not done yet). Can you pick one from the "Ready-to-take Component".

@lhilgert9
Copy link
Contributor

@siriwatknp I was wondering myself why the Select component was in the ready to take section. Then just assign me a component and I can work through it. Thanks

@siriwatknp
Copy link
Member Author

Hi @siriwatknp I'm working on a project dependent on MUI and we would need the zero runtime CSS for a next step in the project. We're happy to contribute with our time to the MUI repo with the rewrite if it can help to speed up the works and the new version release. But basically wondering if us spending some extra time will actually make things faster or is there a fixed release schedule anyway?

Thanks! We aim for May but it's not a fixed schedule. The faster we finish this issue, the faster we can do the stable release.

@danilo-leal danilo-leal changed the title [material-ui] Convert Material UI components to support CSS extraction [material-ui] Convert Material UI components to support Pigment CSS Apr 4, 2024
@lhilgert9
Copy link
Contributor

@siriwatknp I'll work on the ToggleButton component.

@siriwatknp
Copy link
Member Author

@siriwatknp I'll work on the ToggleButton component.

Thanks! assigned.

@lhilgert9
Copy link
Contributor

@siriwatknp I would like to work on the Radio next.

@gijsbotje
Copy link
Contributor

gijsbotje commented Apr 10, 2024

@siriwatknp I've taken the liberty of converting the PaginationItem (#41848), IconButton (#41850) and Fab (#41851)

@lhilgert9
Copy link
Contributor

@siriwatknp I would like to work on the Icon next.

@siriwatknp
Copy link
Member Author

@siriwatknp I would like to work on the Icon next.

Hey, thanks for asking but the rest will be done by the codemod. Thank you so much!

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 umbrella For grouping multiple issues to provide a holistic view v6.x
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

10 participants