[DataGrid] Refactor: create base Checkbox #16445
Conversation
romgrk
added
the
component: data grid
|
Deploy preview: https://deploy-preview-16445--material-ui-x.netlify.app/ |
| checked={columnVisibilityModel[column.field] !== false} | ||
| onClick={toggleColumn} | ||
| name={column.field} | ||
| style={{ padding: 0.5 }} |
There was a problem hiding this comment.
When translating padding/margin from sx to style, the value needs to change also— the value was 4px, now it is 0.5px.
Ideally, padding/margin styles should be calculated using the spacing value in the theme, perhaps we should wrap this in a styled to apply the padding?
There was a problem hiding this comment.
Looks like there is an issue with this approach https://app.argos-ci.com/mui/mui-x/builds/30524/135954282
It is applying the padding to the label rather than the checkbox. Not sure how we can target the checkbox here without using Material UI classes 🤔
There was a problem hiding this comment.
How about adding density?: 'standard' | 'compact' prop to CheckboxProps and applying padding in BaseCheckbox for 'compact' density?
|
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
github-actions
bot
added
the
PR: out-of-date
github-actions
bot
removed
the
PR: out-of-date
| }; | ||
| style?: React.CSSProperties; | ||
| tabIndex?: number; | ||
| touchRippleRef?: any; // FIXME(v8:romgrk): find a way to remove |
There was a problem hiding this comment.
How about moving this part to BaseCheckbox and passing hasFocus prop to it?
There was a problem hiding this comment.
Yeah that could work. I wonder if autoFocus would be better? It would match the similar prop on MenuList.
There was a problem hiding this comment.
I wonder if autoFocus would be better?
Yeah, this makes sense 👍🏻
| checked={columnVisibilityModel[column.field] !== false} | ||
| onClick={toggleColumn} | ||
| name={column.field} | ||
| style={{ padding: 0.5 }} |
There was a problem hiding this comment.
How about adding density?: 'standard' | 'compact' prop to CheckboxProps and applying padding in BaseCheckbox for 'compact' density?
Part of the design-system agnostic work.
Create the base Checkbox. Unlike the material-ui one, this one handles its own label the same way the base TextField does.