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

[DataGrid] Refactor: create base Checkbox #16445

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
+60 −29
Open

[DataGrid] Refactor: create base Checkbox #16445

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
79d1961
refactor: create base Checkbox
romgrk Feb 4, 2025
f988128
lint
romgrk Feb 5, 2025
e726b9d
Merge branch 'master' into refactor-agnostic-checkbox
romgrk Feb 6, 2025
6530e60
lint
romgrk Feb 6, 2025
ba08681
Merge branch 'master' into refactor-agnostic-checkbox
romgrk Feb 6, 2025
89ec973
lint
romgrk Feb 7, 2025
b0382f6
Merge branch 'master' into refactor-agnostic-checkbox
romgrk Feb 7, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion packages/x-data-grid/src/components/columnSelection/GridCellCheckboxRenderer.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,9 @@ const GridCellCheckboxForwardRef = forwardRef<HTMLInputElement, GridRenderCellPa
checked={isChecked && !isIndeterminate}
onChange={handleChange}
className={classes.root}
inputProps={{ 'aria-label': label, name: 'select_row' }}
slotProps={{
htmlInput: { 'aria-label': label, name: 'select_row' },
}}
onKeyDown={handleKeyDown}
indeterminate={isIndeterminate}
disabled={!isSelectable}
Expand Down
4 changes: 3 additions & 1 deletion packages/x-data-grid/src/components/columnSelection/GridHeaderCheckbox.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,9 @@ const GridHeaderCheckbox = forwardRef<HTMLButtonElement, GridColumnHeaderParams>
checked={isChecked && !isIndeterminate}
onChange={handleChange}
className={classes.root}
inputProps={{ 'aria-label': label, name: 'select_all_rows' }}
slotProps={{
htmlInput: { 'aria-label': label, name: 'select_all_rows' },
}}
tabIndex={tabIndex}
onKeyDown={handleKeyDown}
disabled={!isMultipleRowSelectionEnabled(rootProps)}
Expand Down
41 changes: 16 additions & 25 deletions packages/x-data-grid/src/components/columnsManagement/GridColumnsManagement.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
import * as React from 'react';
import PropTypes from 'prop-types';
import composeClasses from '@mui/utils/composeClasses';
import FormControlLabel from '@mui/material/FormControlLabel';
import { styled } from '@mui/material/styles';
import { inputBaseClasses } from '@mui/material/InputBase';
import { TextFieldProps } from '../../models/gridBaseSlots';
Expand Down Expand Up @@ -272,21 +271,17 @@ function GridColumnsManagement(props: GridColumnsManagementProps) {
</GridColumnsManagementHeader>
<GridColumnsManagementBody className={classes.root} ownerState={rootProps}>
{currentColumns.map((column) => (
<FormControlLabel
<rootProps.slots.baseCheckbox
key={column.field}
className={classes.row}
control={
<rootProps.slots.baseCheckbox
disabled={column.hideable === false}
checked={columnVisibilityModel[column.field] !== false}
onClick={toggleColumn}
name={column.field}
sx={{ p: 0.5 }}
inputRef={isFirstHideableColumn(column) ? firstSwitchRef : undefined}
{...rootProps.slotProps?.baseCheckbox}
/>
}
disabled={column.hideable === false}
checked={columnVisibilityModel[column.field] !== false}
onClick={toggleColumn}
name={column.field}
style={{ padding: 0.5 }}
Copy link
Member

@KenanYusuf KenanYusuf Feb 6, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

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?

Copy link
Member

Choose a reason for hiding this comment

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

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 🤔

Copy link
Member

Choose a reason for hiding this comment

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

How about adding density?: 'standard' | 'compact' prop to CheckboxProps and applying padding in BaseCheckbox for 'compact' density?

inputRef={isFirstHideableColumn(column) ? firstSwitchRef : undefined}
label={column.headerName || column.field}
{...rootProps.slotProps?.baseCheckbox}
/>
))}
{currentColumns.length === 0 && (
Expand All @@ -298,19 +293,15 @@ function GridColumnsManagement(props: GridColumnsManagementProps) {
{(!disableShowHideToggle || !disableResetButton) && currentColumns.length > 0 ? (
<GridColumnsManagementFooter ownerState={rootProps} className={classes.footer}>
{!disableShowHideToggle ? (
<FormControlLabel
control={
<rootProps.slots.baseCheckbox
disabled={hideableColumns.length === 0}
checked={allHideableColumnsVisible}
indeterminate={!allHideableColumnsVisible && !allHideableColumnsHidden}
onClick={() => toggleAllColumns(!allHideableColumnsVisible)}
name={apiRef.current.getLocaleText('columnsManagementShowHideAllText')}
sx={{ p: 0.5 }}
{...rootProps.slotProps?.baseCheckbox}
/>
}
<rootProps.slots.baseCheckbox
disabled={hideableColumns.length === 0}
checked={allHideableColumnsVisible}
indeterminate={!allHideableColumnsVisible && !allHideableColumnsHidden}
onClick={() => toggleAllColumns(!allHideableColumnsVisible)}
name={apiRef.current.getLocaleText('columnsManagementShowHideAllText')}
style={{ padding: 0.5 }}
label={apiRef.current.getLocaleText('columnsManagementShowHideAllText')}
{...rootProps.slotProps?.baseCheckbox}
/>
) : (
<span />
Expand Down
17 changes: 16 additions & 1 deletion packages/x-data-grid/src/material/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import MUIMenuList from '@mui/material/MenuList';
import MUIMenuItem from '@mui/material/MenuItem';
import MUITextField from '@mui/material/TextField';
import MUIFormControl from '@mui/material/FormControl';
import MUIFormControlLabel from '@mui/material/FormControlLabel';
import MUISelect from '@mui/material/Select';
import MUIButton from '@mui/material/Button';
import MUIIconButton from '@mui/material/IconButton';
Expand Down Expand Up @@ -93,7 +94,7 @@ const iconSlots: GridIconSlotsComponent = {

const baseSlots: GridBaseSlots = {
baseBadge: MUIBadge,
baseCheckbox: MUICheckbox,
baseCheckbox: BaseCheckbox,
baseCircularProgress: MUICircularProgress,
baseDivider: MUIDivider,
baseLinearProgress: MUILinearProgress,
Expand All @@ -120,6 +121,20 @@ const materialSlots: GridBaseSlots & GridIconSlotsComponent = {

export default materialSlots;

function BaseCheckbox(props: GridSlotProps['baseCheckbox']) {
const { label, slotProps, className, ...other } = props;
if (!label) {
return <MUICheckbox {...other} className={className} inputProps={slotProps?.htmlInput} />;
}
return (
<MUIFormControlLabel
className={className}
control={<MUICheckbox {...other} inputProps={slotProps?.htmlInput} />}
label={label}
/>
);
}

function BaseMenuItem(props: GridSlotProps['baseMenuItem']) {
const { inert, iconStart, iconEnd, children, ...other } = props;
if (inert) {
Expand Down
22 changes: 22 additions & 0 deletions packages/x-data-grid/src/models/gridBaseSlots.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,28 @@ export type ButtonProps = {
touchRippleRef?: any; // FIXME(v8:romgrk): find a way to remove
};

export type CheckboxProps = {
ref?: Ref<HTMLButtonElement>;
id?: string;
checked?: boolean;
className?: string;
disabled?: boolean;
indeterminate?: boolean;
inputRef?: React.Ref<HTMLInputElement>;
name?: string;
label?: React.ReactNode;
onClick?: React.MouseEventHandler;
onChange?: React.ChangeEventHandler;
onKeyDown?: React.KeyboardEventHandler;
size?: 'small' | 'medium';
slotProps?: {
htmlInput?: React.InputHTMLAttributes<HTMLInputElement>;
};
style?: React.CSSProperties;
tabIndex?: number;
touchRippleRef?: any; // FIXME(v8:romgrk): find a way to remove
Copy link
Member

Choose a reason for hiding this comment

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

How about moving this part to BaseCheckbox and passing hasFocus prop to it?

mui-x/packages/x-data-grid/src/components/columnSelection/GridCellCheckboxRenderer.tsx

Lines 72 to 80 in 5ed0b65

React.useEffect(() => {
if (hasFocus) {
const input = checkboxElement.current?.querySelector('input');
input?.focus({ preventScroll: true });
} else if (rippleRef.current) {
// Only available in @mui/material v5.4.1 or later
rippleRef.current.stop({});
}
}, [hasFocus]);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah that could work. I wonder if autoFocus would be better? It would match the similar prop on MenuList.

Copy link
Member

Choose a reason for hiding this comment

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

I wonder if autoFocus would be better?

Yeah, this makes sense 👍🏻

};

export type IconButtonProps = Omit<ButtonProps, 'startIcon'> & {
label?: string;
color?: 'default' | 'inherit' | 'primary';
Expand Down
2 changes: 1 addition & 1 deletion packages/x-data-grid/src/models/gridSlotsComponentsProps.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import * as React from 'react';
import type { BadgeProps as MUIBadgeProps } from '@mui/material/Badge';
import type { ButtonProps as MUIButtonProps } from '@mui/material/Button';
import type { CheckboxProps } from '@mui/material/Checkbox';
import type { CircularProgressProps as MUICircularProgressProps } from '@mui/material/CircularProgress';
import type { LinearProgressProps as MUILinearProgressProps } from '@mui/material/LinearProgress';
import type { MenuListProps } from '@mui/material/MenuList';
Expand Down Expand Up @@ -37,6 +36,7 @@ import type { GridColumnHeaderSortIconProps } from '../components/columnHeaders/
import type {
BadgeProps,
ButtonProps,
CheckboxProps,
CircularProgressProps,
DividerProps,
IconButtonProps,
Expand Down