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

[core] Copy getClasses from the core (removed in v5) #2140

Merged
merged 12 commits into from
Jul 14, 2021

Conversation

flaviendelangle
Copy link
Member

@flaviendelangle flaviendelangle commented Jul 13, 2021

#1994 starts using getClasses that was recently removed from the core (due to the migration away from enzyme)
See mui/material-ui@2569eb9#diff-f075d81b14df1f31fb0699c4516fe3d78490351867feeb4fb869d5dbe8c6128e

@oliviertassinari

@flaviendelangle
Copy link
Member Author

@flaviendelangle flaviendelangle added core Infrastructure work going on behind the scenes test labels Jul 13, 2021
@oliviertassinari
Copy link
Member

Ok, so if this quick-win doesn't work. I would propose even simpler. We introduce the xxxClasses pattern for this component, it will help us migrate to v5, and avoid investing in legacy test utils.

@oliviertassinari
Copy link
Member

oliviertassinari commented Jul 13, 2021

Here is what we do in v5 (in the idea):

diff --git a/packages/grid/_modules_/grid/components/panel/GridPanel.test.tsx b/packages/grid/_modules_/grid/components/panel/GridPanel.test.tsx
index bf5027df..b8d6e468 100644
--- a/packages/grid/_modules_/grid/components/panel/GridPanel.test.tsx
+++ b/packages/grid/_modules_/grid/components/panel/GridPanel.test.tsx
@@ -4,11 +4,9 @@ import {
   // @ts-expect-error JS
   createMount,
   // @ts-expect-error JS
-  getClasses,
-  // @ts-expect-error JS
   describeConformance,
 } from 'test/utils';
-import { GridPanel, useGridApiRef, GridApiContext } from '@material-ui/data-grid';
+import { GridPanel, gridPanelClasses as classes, useGridApiRef, GridApiContext } from '@material-ui/data-grid';
 import { Popper } from '@material-ui/core';

 describe('<GridPanel />', () => {
@@ -16,8 +14,6 @@ describe('<GridPanel />', () => {
   // TODO v5: replace with createClientRender
   const render = createClientRenderStrictMode();

-  let classes;
-
   function Wrapper(props) {
     const apiRef = useGridApiRef();
     apiRef.current.columnHeadersContainerElementRef = {
@@ -28,10 +24,6 @@ describe('<GridPanel />', () => {
     return <GridApiContext.Provider value={apiRef} {...props} />;
   }

-  before(() => {
-    classes = getClasses(<GridPanel open={false} />);
-  });
-
   describeConformance(<GridPanel disablePortal open />, () => ({
     classes,
     inheritComponent: Popper,
diff --git a/packages/grid/_modules_/grid/components/panel/GridPanel.tsx b/packages/grid/_modules_/grid/components/panel/GridPanel.tsx
index 7f5416ed..ac458b4a 100644
--- a/packages/grid/_modules_/grid/components/panel/GridPanel.tsx
+++ b/packages/grid/_modules_/grid/components/panel/GridPanel.tsx
@@ -41,6 +41,11 @@ const useStyles = makeStyles(
   { name: 'MuiGridPanel', defaultTheme },
 );

+export const gridPanelClasses = {
+  root: 'MuiGridPanel-root',
+  paper: 'MuiGridPanel-paper',
+};
+
 export const GridPanel = React.forwardRef<HTMLDivElement, GridPanelProps>(function GridPanel(
   props,
   ref,
@@ -103,6 +108,3 @@ export const GridPanel = React.forwardRef<HTMLDivElement, GridPanelProps>(functi
     </Popper>
   );
 }) as (props: GridPanelProps) => JSX.Element;
-
-// @ts-ignore TODO migrate to v5 gridPanelClasses pattern, this is only for tests
-GridPanel.useStyles = useStyles;

If you want to push it further, to match v5 exactly, feel free to.

@oliviertassinari
Copy link
Member

@mui-org/x think that we need to take over this effort as Flavien is off until next Monday.

@m4theushw
Copy link
Member

@oliviertassinari I can take it as it's breaking master and blocking all new PRs.

@m4theushw
Copy link
Member

@oliviertassinari Do you have any suggestion to fix the Netlify deploy? I think it fails because it's using a node_modules cache.

@oliviertassinari
Copy link
Member

oliviertassinari commented Jul 14, 2021

@m4theushw It's flaky, I don't know why. I have rerun it.

tsconfig.json Outdated Show resolved Hide resolved
Copy link
Member

@oliviertassinari oliviertassinari left a comment

Choose a reason for hiding this comment

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

It's green 🎉

@m4theushw m4theushw merged commit ac5b29a into mui:master Jul 14, 2021
@oliviertassinari
Copy link
Member

@m4theushw Thanks. I'm rebasing my other PR now :)

@flaviendelangle flaviendelangle deleted the getClasses branch October 9, 2021 07:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Infrastructure work going on behind the scenes test
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants