[core] Reduce styles complexity #3012
Conversation
DanailH
added
core
|
Related to #2995 (comment). If we make this change, I think that we should make sure it has no performance implications. |
DanailH
changed the title
github-actions
bot
added
the
PR: out-of-date
|
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
DanailH
changed the title
github-actions
bot
removed
the
PR: out-of-date
|
Looks good to me |
packages/grid/_modules_/grid/components/cell/GridEditInputCell.tsx
Outdated
Show resolved
Hide resolved
| @@ -1,6 +1,7 @@ | |||
| import * as React from 'react'; | |||
| import clsx from 'clsx'; | |||
| import { unstable_composeClasses as composeClasses } from '@mui/material'; | |||
There was a problem hiding this comment.
This looks problematic. We bundle the whole @mui/material dependency in dev mode. Could we import the util only?
| export const GridFooterContainer = React.forwardRef<HTMLDivElement, GridFooterContainerProps>( | ||
| function GridFooterContainer(props: GridFooterContainerProps, ref) { | ||
| const { className, ...other } = props; | ||
| const rootProps = useGridRootProps(); | ||
| const ownerState = { classes: rootProps.classes }; | ||
| const classes = useUtilityClasses(ownerState); | ||
|
|
||
| return <div ref={ref} className={clsx(classes.root, className)} {...other} />; |
There was a problem hiding this comment.
Something is off with the classes.
gridClasses has footerContainer and yet, in this file, we create the class name a second time. I think that we should either:
- have
<DataGrid classes={{ footerContainer: 'foo' }} />to actually work and usegridClasses.footerContainerfor the class name, not duplicating its creation. - or change the name of the styled component:
name: 'MuiDataGrid',->name: 'GridFooterContainer',
slot: 'FooterContainer',->slot: 'Root',, remove footerContainer fromgridClassesand expose agridFooterContainerClassesmodule.
There was a problem hiding this comment.
I would stick to the first option since that is the convention we are currently following. IMO it will look off to have GridFooterContainer because that way we would need to go through the other components and update them as well.
There was a problem hiding this comment.
@oliviertassinari I didn't understand the problem and solution proposed in the first option.
-
The value of
gridClasses.footerContainerisMuiDataGrid-footerContainerwhich breakscomposeClasses. -
<DataGrid classes={{ footerContainer: 'foo' }} />works correctly:
There was a problem hiding this comment.
Ok, it seems that we are good.
Reorganizing the grid styles. Due to performance reasons the styles for the column headers, cells and rows are kept in the
GridRootStyles. All other styles that aren't nested in one of those components have been moved out of this file and into their respective component.This PR doesn't have any breaking changes.