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

[DataGrid] Customize FilterPanel with props #3497

Merged
merged 40 commits into from
Feb 10, 2022

Conversation

alexfauquette
Copy link
Member

@alexfauquette alexfauquette commented Dec 22, 2021

Fix #3082 and fix #3386

Allows modifying the filter panel by using props. It should improve the DX. I let you provide any feedback on the interface. I will add test, after agreeing on how dev should customize this pannel :)

Docs: https://deploy-preview-3497--material-ui-x.netlify.app/components/data-grid/filtering/#customize-the-filter-panel-content

@alexfauquette alexfauquette self-assigned this Dec 22, 2021
@alexfauquette alexfauquette requested review from DanailH, flaviendelangle and m4theushw and removed request for DanailH and flaviendelangle December 22, 2021 18:20
Comment on lines 233 to 238
sx={[
{ flexShrink: 0, justifyContent: 'flex-end', marginRight: 0.5, marginBottom: 0.2 },
...(Array.isArray(deleteIconContainerSx)
? deleteIconContainerSx
: [deleteIconContainerSx]),
]}
Copy link
Member

@m4theushw m4theushw Dec 27, 2021

Choose a reason for hiding this comment

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

As we discussed, the prop could be renamed to deleteIconContainerProps and allow to override other props of the FormControl.

Suggested change
sx={[
{ flexShrink: 0, justifyContent: 'flex-end', marginRight: 0.5, marginBottom: 0.2 },
...(Array.isArray(deleteIconContainerSx)
? deleteIconContainerSx
: [deleteIconContainerSx]),
]}
{...deleteIconContainerProps}
sx={{
flexShrink: 0,
justifyContent: 'flex-end',
marginRight: 0.5,
marginBottom: 0.2,
...deleteIconContainerProps.sx
}}

Another alternative is to make the FormControl a styled component, then you don't need to care about spreading deleteIconContainerProps.sx, since it's done automatically.

const StyledFormControl = styled(FormControl)({ flexShrink: 0 });

<StyledFormControl {...deleteIconContainerProps} />

<div style={{ height: 400, width: '100%' }}>
<DataGridPro
{...data}
// components={{
Copy link
Member

Choose a reason for hiding this comment

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

Remove commented-out code (or maybe it's still a WIP).

Copy link
Member Author

Choose a reason for hiding this comment

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

In fact, I was looking for a way to show all the customization options in the same code, and comment on those which are not used in the demo. But yes, that's not explicit enough.

hasLinkOperatorColumn?: boolean;
linkOperators?: (GridLinkOperator.And | GridLinkOperator.Or)[];
columnsSort?: 'asc' | 'desc';
deleteIconContainerSx?: SxProps<Theme>;
Copy link
Member

Choose a reason for hiding this comment

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

@mnajdova in the core how to you handle when you want the user to easily customize the style of nested elements with sx ?

Do you pass down the sx function like this or do you expose the classes and let the user nest inside the root sx ?

Copy link
Member

Choose a reason for hiding this comment

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

I would recommend using components and componentsProps for this. Then developers can do:

<GridFilterForm
  componentsProps={{ deleteIconContainer: { sx: /* ... */ } }}
  ...
/>

At least this is what we do in the @mui/base.

Copy link
Member Author

@alexfauquette alexfauquette Jan 11, 2022

Choose a reason for hiding this comment

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

@mnajdova Do you mean using components props to let dev use a custom filter as follow?

const CustomFilterForm = (props) => <GridFilterForm
	componentsProps={{ deleteIconContainer: { sx: /* ... */ } }}
	{...props}
/>

<DataGrid
	...
	components={{
		filterForm: CustomFilterForm
	}}

For now, I tried to use nesting classes as follows. Considering that <FilterPanel /> is the only component user can have access to. The other components are considered as nested ones, and can only be customized by CSS selectors.

<DataGrid
	...
	componentsProps={{
		filterPanel: {
			sx: {
				"& .MuiDataGrid-filterForm": { ... },
				"& .MuiDataGrid-closeIconController": { ... },
				...
			}
		}
	}}

Copy link
Member

Choose a reason for hiding this comment

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

Sorry, for some reason I have missed this comment. If the other nested components can only be customized by CSS selectors, why are you adding this new prop deleteIconContainerSx. It contradicts with the previous statement.

My point was that, we don't add first class props on the component for prop that should be passed on some nested elements, we prefer using componentsProps for this use-case.

@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Jan 10, 2022
@github-actions
Copy link

This pull request has conflicts, please resolve those before we can evaluate the pull request.

@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Jan 11, 2022
{...baseFormControlProps}
{...valueInputProps}
className={clsx(
classes.valueInput,
Copy link
Member

Choose a reason for hiding this comment

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

Maybe this has been already answered but what about when the FilterFormValueInput is the select or the multi-select, will it always has that classes.valueInput class or should we change the class based on the component?

Copy link
Member Author

Choose a reason for hiding this comment

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

Not already answered. Modifying the class according to the input seems tricky.
We should add a property currentOperator.valueFormControlClass

In my opinion, it is overkilled compared to what most of the dev will probably do with the customization:

  • removing some option
  • modifying input size

About the size, classes.valueInput is useful to remove default one. To have customer size depending on the input component, I see two cases:

  • it is their own input, so they can do whatever they want
  • it is one of the provided input. They can distinguish single and multiple values with CSS controller, depending on MuiFormControl-root and MuiInput-root

Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure if I'm on the same page here but classes.valueInput is applied to the FormControl that wraps the custom filter input component. In this context, FilterFormValueInput will always be a FormControl, unless the user changes it through the slots.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, we are on the same page

For now, the FormControl is responsible for the width. So to customize the classes.valueInput must be used to add width:'unset'

},
valueInputProps: {
required: true,
},
Copy link
Member

@joserodolfofreitas joserodolfofreitas Feb 4, 2022

Choose a reason for hiding this comment

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

Not sure this is the best way to customize the deleteIcon color.
But I think we could add a simple example for deleteIconProps too.

Suggested change
},
deleteIconProps: {
sx: {
"& .MuiSvgIcon-root": { color: "#f00" }
}
},

Copy link
Member Author

@alexfauquette alexfauquette Feb 7, 2022

Choose a reason for hiding this comment

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

Yes, it's better than my reorder which was a bad accessibility practice

@github-actions
Copy link

github-actions bot commented Feb 8, 2022

This pull request has conflicts, please resolve those before we can evaluate the pull request.

@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Feb 8, 2022
@siriwatknp
Copy link
Member

siriwatknp commented Feb 8, 2022

👋 The migration PR has been merged.

Please follow these steps to make sure that the contents are all updated. (Sorry for the inconvenience)

  1. pull latest master from upstream to your branch
  2. if your PR has changes on the *.md or demo files, you are likely to encounter conflict because all of them have been moved to the new folder.
    2.1 revert the change on those markdown files you did
    2.2 pull latest master from upstream (you should not see conflict)
    2.3 make the changes again at docs/data/data-grid/*
  3. run yarn docs:api
    • you might see the changes in docs/pages/x/api/data-grid/* if your PR touches some of the components
    • it is okay if there is no changes

If you are struggle with the steps above, feel free to tag @siriwatknp

@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Feb 8, 2022
Copy link
Member

@m4theushw m4theushw left a comment

Choose a reason for hiding this comment

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

I only found some small details to fix. Well done!

variant="standard"
{...baseFormControlProps}
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
{...baseFormControlProps}
as={rootProps.components.BaseFormControl}
{...baseFormControlProps}

The customization of the filter panel content can be performed by passing props to the default `<GridFilterPanel />` component.
The available props allow to override:

- The available `linkOperators` (can contains `GridLinkOperator.And` and `GridLinkOperator.Or`)
Copy link
Member

Choose a reason for hiding this comment

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

There's already "available" in the previous line.

Suggested change
- The available `linkOperators` (can contains `GridLinkOperator.And` and `GridLinkOperator.Or`)
- The `linkOperators` (can contains `GridLinkOperator.And` and `GridLinkOperator.Or`)

- Any prop of the input components

Input components can be [customized](/customization/how-to-customize/) by using two approaches.
You can pass a `sx` prop to any input container or you can use css selectors on nested components of the filter panel.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
You can pass a `sx` prop to any input container or you can use css selectors on nested components of the filter panel.
You can pass a `sx` prop to any input container or you can use CSS selectors on nested components of the filter panel.

You can pass a `sx` prop to any input container or you can use css selectors on nested components of the filter panel.
More details are available in the demo.

| Props | Class |
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
| Props | Class |
| Props | CSS class |

React.useEffect(() => {
if (items.length > 0) {
lastFilterRef.current!.focus();
}
}, [items.length]);

return (
<GridPanelWrapper>
<GridPanelWrapper sx={sx}>
Copy link
Member

Choose a reason for hiding this comment

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

We usually pass to the root all the remaining props.

@alexfauquette alexfauquette merged commit 02213e8 into mui:master Feb 10, 2022
@alexfauquette alexfauquette changed the title [DataGrid] Customize FilterPanel with props [DataGrid] Customize FilterPanel with props Feb 10, 2022
@alexfauquette alexfauquette deleted the customize-filters branch February 10, 2022 13:27
@cherniavskii cherniavskii mentioned this pull request Feb 10, 2022
13 tasks
@oliviertassinari oliviertassinari added the component: data grid This is the name of the generic UI component, not the React module! label Jan 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: data grid This is the name of the generic UI component, not the React module!
Projects
None yet
10 participants