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.

Sign up for GitHub

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

Jump to bottom

Migrate all components to emotion #24405

Closed
mnajdova opened this issue Jan 14, 2021 · 74 comments · Fixed by #26389
Closed

Migrate all components to emotion #24405

mnajdova opened this issue Jan 14, 2021 · 74 comments · Fixed by #26389
Labels
priority: important This change can make a difference
Milestone
v5-beta

Comments

@mnajdova
Copy link
Member

mnajdova commented Jan 14, 2021 •
edited by siriwatknp
Loading

As decided in #22342 we need to migrate all the components to the @mateiral-ui/styled-engine (emotion by default) API. We've already converted a dozen components. We are now confident with the approach. We have fixed most of the challenges it creates. This issue will help track the progress of the migration.

For converting a component, you can use as a template any of the already converted components, e.g #24397. We have also this migration guide in case you want to prepare yourself before starting, or in case if something is not working as expected.

This is a useful checklist to have when migrating a component:

General checklist:

  • Update sx prop types
  • Update tests (classes & describeConformanceV5 usage)
  • Add componentClasses files
  • Export componentClasses from index.js & index.d.ts

Component checklist:

  • useThemeProps, useUtilityClasses are used and correct (styleProps need to be created)
  • slots styles are converted to separate styled() components (example ButtonLabel)
  • styleProps and classes are spread on root and all slots correctly
  • if there is component prop, it is used like as={component} on the Root slot
  • $slot are converted to [& .${componentClasses.slot}]

Here is a list of all components with their status:

Note: if you see that the components depends on some other components, then those components need to be converted first, before the component itself can be migrated.

@material-ui/core

@material-ui/lab

Once the migration of the core components is done, we can then migrate all the demos of the documentation to the sx prop (preferred) and styled API, removing all imports of withStyles and makeStyles.

Current progress 169/169 
Array.from(document.querySelectorAll('.contains-task-list')[2].children)
.concat(Array.from(document.querySelectorAll('.contains-task-list')[3].children)).reduce((acc, item) => {
  if (item.querySelector('input[type="checkbox"]:checked')) {
    acc.done += 1;
  }
  acc.total += 1;
  return acc;
}, { total: 0, done: 0 });
@mnajdova mnajdova added the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Jan 14, 2021
@oliviertassinari oliviertassinari removed the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Jan 14, 2021
@oliviertassinari oliviertassinari added this to the v5 milestone Jan 14, 2021
@oliviertassinari oliviertassinari changed the title [RFC] Migrate all core components to emotion Migrate all core components to emotion Jan 14, 2021
@oliviertassinari oliviertassinari added priority: important This change can make a difference ready to take Help wanted. Guidance available. There is a high chance the change will be accepted labels Jan 14, 2021
@mbrookes mbrookes pinned this issue Jan 14, 2021
@TheRealBenJones
Copy link

Gunna take a shot at Icon for my first commit

This was referenced Jan 15, 2021
Closed
Merged
@kutnickclose kutnickclose mentioned this issue Jan 16, 2021
Merged
1 task
Merged
1 task
@oliviertassinari oliviertassinari mentioned this issue Jan 17, 2021
Merged
1 task
@mnajdova mnajdova mentioned this issue Jan 18, 2021
Closed
@queengooborg queengooborg mentioned this issue Jan 20, 2021
Merged
1 task
@oliviertassinari oliviertassinari mentioned this issue Jan 20, 2021
Merged
This was referenced Jan 20, 2021
Merged
Merged
Merged
@oliviertassinari oliviertassinari mentioned this issue Jan 20, 2021
Closed
@queengooborg queengooborg mentioned this issue Jan 21, 2021
Merged
1 task
Merged
1 task
This was referenced Jan 21, 2021
Merged
Merged
Merged
Merged
Merged
@xs9627 xs9627 mentioned this issue Jan 22, 2021
Merged
1 task
This was referenced Jan 22, 2021
Merged
Merged
@oliviertassinari
Copy link
Member

TypeScript, unless there is a good reason not too.

@vicasas vicasas mentioned this issue Apr 18, 2021
Merged
1 task
This was referenced Apr 19, 2021
Merged
Merged
@mnajdova mnajdova mentioned this issue Apr 21, 2021
Closed
1 task
@mnajdova
Copy link
Member Author

Looks like we finished the migration of all core components 🥳 We have around 2/3 left in the lab and we will be done with those too ✔️ Thanks a lot to everyone that contributed 🙏

@siriwatknp
Copy link
Member

@mnajdova are the pickers ready to be picked up?

@oliviertassinari
Copy link
Member

are the pickers ready to be picked up?

This sounds like a question for @eps1lon. Without the context, I would assume the answer is yes (so we only bundle one CSS-in-JS engine) unless there is a planned change that could happen before, making the migration to emotion more efficient?

@eps1lon
Copy link
Member

eps1lon commented Apr 23, 2021

Pickers should be good to go.

This was referenced Apr 25, 2021
Merged
Merged
Merged
Merged
@vicasas
Copy link
Member

vicasas commented Apr 27, 2021 •
edited
Loading

For some unknown reason, some Backdrop component files were lost post-migration (#24523).

The files are:

[component]Classes.d.ts
[component]Classes.js

@mnajdova
Copy link
Member Author

@vicasas they were moved to the @material-ui/unstyled package

@oliviertassinari oliviertassinari changed the title Migrate all core components to emotion Migrate all components to emotion May 10, 2021
This was referenced May 11, 2021
Merged
Merged
Merged
@anurodhmohapatra
Copy link

anurodhmohapatra commented May 19, 2021 •
edited by oliviertassinari
Loading

I have one query. I was reading v5 beta documentation and was looking at some code. As MUI migrating to emotion. In documentation are you going to use backticks or object method for styling?

const Main = styled('main', { shouldForwardProp: (prop) => prop !== 'open' })(
  ({ theme, open }) => ({
    flexGrow: 1,
    padding: theme.spacing(3),
    transition: theme.transitions.create('margin', {
      easing: theme.transitions.easing.sharp,
      duration: theme.transitions.duration.leavingScreen,
    }),
    marginLeft: `-${drawerWidth}px`,
    ...(open && {
      transition: theme.transitions.create('margin', {
        easing: theme.transitions.easing.easeOut,
        duration: theme.transitions.duration.enteringScreen,
      }),
      marginLeft: 0,
    }),
  }),
);

@oliviertassinari oliviertassinari mentioned this issue May 19, 2021
Merged
1 task
@oliviertassinari
Copy link
Member

oliviertassinari commented May 19, 2021 •
edited
Loading

In documentation are you going to use backticks or object method for styling?

@anurodhmohapatra We use the JavaScript object syntax to one exception. We use the CSS template for the customization demos with the unstyled components.

@natac13
Copy link
Contributor

natac13 commented May 19, 2021

In documentation are you going to use backticks or object method for styling?

@anurodhmohapatra We use the JavaScript object syntax to one exception. We use the CSS template for the customization demos with the unstyled components.

And correct me if I am wrong but when there are key frames involved? Such as circular progress?

@anurodhmohapatra
Copy link

anurodhmohapatra commented May 19, 2021 •
edited
Loading

In documentation are you going to use backticks or object method for styling?

@anurodhmohapatra We use the JavaScript object syntax to one exception. We use the CSS template for the customization demos with the unstyled components.

@oliviertassinari This means we will be getting JavaScript object syntax out of the box which we can modify to customise it. If there is any element which is unstyled we will use tagged template literals(backticks) for styling. Am I right? Or is there any other best practice?

This was referenced May 19, 2021
Merged
Merged
@oliviertassinari oliviertassinari unpinned this issue May 21, 2021
@oliviertassinari
Copy link
Member

Well done everybody in the migration! 👏

@siriwatknp siriwatknp mentioned this issue Mar 30, 2022
Merged
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
priority: important This change can make a difference
Projects
None yet
Milestone
v5-beta
Development

Successfully merging a pull request may close this issue.

[TimePicker] Migrate ClockPicker to emotion siriwatknp/material-ui