[Collapse] Migrate to emotion

[queengooborg]

This PR migrates the Collapse component for v5 as a part of #24405.

Notice: this is my very first pull request to this repository. If I have made any mistakes, please do not hesitate to let me know and I will fix it right away! Feel free to give any and all critique -- I'm a big girl, I can handle it. 😛

• I have followed (at least) the PR section of the contributing guide.

This contribution, and all other contributions I make to Material UI, are performed under Gooborg Studios, LLC on behalf of Fox and Geese, LLC. Both companies agree to surrender all contributions under MUI's MIT license.

[mui-pr-bot]

@material-ui/core: parsed: +0.26% , gzip: +0.20%
@material-ui/lab: parsed: +0.25% , gzip: +0.29%

Details of bundle changes

Generated by 🚫 dangerJS against 9e22c49

[queengooborg]

There's two tests that I'm not really sure how to fix at this moment, nor do I feel that disabling the tests is the right call. The failing tests are the following:

  1) <StepContent />
       renders children inside an Collapse component:
     TypeError: Cannot read property 'root' of null
      at Context.<anonymous> (packages/material-ui/src/StepContent/StepContent.test.js:46:66)
      at processImmediate (node:internal/timers:463:21)

  2) <StepContent />
       prop: transitionDuration
         should use default Collapse component:
     TypeError: Cannot read property 'root' of null
      at Context.<anonymous> (packages/material-ui/src/StepContent/StepContent.test.js:66:68)
      at processImmediate (node:internal/timers:463:21)

Would love some help on this one!

[mnajdova]

Really great job so far, especially that this is your first PR on the repo 🚀 This component is a bit tricky 😄 I've tried to leave comments for everything I found. Let's address these comments and test the component, and then we can move to do fixes for the tests.

Again, great job!

[queengooborg]

Thank you very much for the quick review, @mnajdova! I've committed all of the suggested changes (they were very educational, thank you), and will soon be addressing the remaining review comments. 😄

[mnajdova]

Thank you very much for the quick review, @mnajdova! I've committed all of the suggested changes (they were very educational, thank you), and will soon be addressing the remaining review comments. 😄

Sure thing! If you have some doubt, take the Button component as a reference, or take a look at the migration guide. I'll check the PR again after all comments are resolved

[queengooborg]

Alright, all review comments have been addressed, thank you very much! All that remains now are just those issues with the tests for StepContent.

[mnajdova]

Alright, all review comments have been addressed, thank you very much! All that remains now are just those issues with the tests for StepContent.

We need to change how the collapseClasses are being used. I believe this diff should fix the test issues:

diff --git a/packages/material-ui/src/StepContent/StepContent.test.js b/packages/material-ui/src/StepContent/StepContent.test.js
index 9266055ee4..dabdfe9a0d 100644
--- a/packages/material-ui/src/StepContent/StepContent.test.js
+++ b/packages/material-ui/src/StepContent/StepContent.test.js
@@ -1,20 +1,18 @@
 import * as React from 'react';
 import { expect } from 'chai';
 import { getClasses, createClientRender, createMount, describeConformance } from 'test/utils';
-import Collapse from '../Collapse';
+import Collapse, { collapseClasses } from '../Collapse';
 import Stepper from '../Stepper';
 import Step from '../Step';
 import StepContent from './StepContent';

 describe('<StepContent />', () => {
   let classes;
-  let collapseClasses;
   const mount = createMount({ strict: true });
   const render = createClientRender();

   before(() => {
     classes = getClasses(<StepContent />);
-    collapseClasses = getClasses(<Collapse />);
   });

   describeConformance(<StepContent />, () => ({

[queengooborg]

Oh durr, that makes total sense... 🤦‍♀️ Patch applied, and the tests are now passing, thank you!

[mnajdova]

I believe this line is not correct https://github.com/mui-org/material-ui/pull/24501/files#diff-0b07fe6763d1a29535d5239ffd35dd227da26c69e664e0d01c0619414b0e86a0R81 it should probably be

      const collapse = container.querySelector(`.${collapseClasses.root}`);

@oliviertassinari what do you think?

[mnajdova]

Great first PR on Material-UI 🚀