mui / mui-x

MUI X: Build complex and data-rich applications using a growing list of advanced React components, like the Data Grid, Date and Time Pickers, Charts, and more!
https://mui.com/x/
4.04k stars 1.25k forks source link

[data grid] Inconsistent filter model update #8107

Open m-konkov opened 1 year ago

m-konkov commented 1 year ago

Steps to reproduce 🕹

Link to live example: https://codesandbox.io/s/why-filters-update-forked-9dzoq4

Steps:

  1. Set filter for "category" column and select one or more values
  2. Change filter column on "some-random-column" which has the same multi-filter select for value
  3. Check console, there is incosistent value for filter model (onFilterModelChange): "some-random-column" has value: [0] for a moment, however this column has different valueOptions. image

Current behavior 😯

When I change filters they use the previous values and then cleaning is going on. I looked at the code, it looks like this is happening in the component for filtering (in useEffect). For example: packages\grid\x-data-grid\src\components\panel\filterPanel\GridFilterInputMultipleSingleSelect.tsx image

Expected behavior 🤔

Expected:

  1. onFilterModelChange provides consistent values
  2. There is a way (via prop or something) to control the clearing of values for custom InputComponent. For example, if I want to save some values according to some condition I can check it somehow or at least I can always get fresh value for filter after changing column.

Context 🔦

I ran into this while trying to develop a custom component to set a filter value. When switching between columns with a multiselect (my custom component) I got old value for multiselect which is wrong considering different valueOptions for columns.

I found a small workaround: InputComponent: (params) => <MultiselectFilterValue {...params} />

Using this code, I created a unique component for each filter and the values ​​began to be updated as I needed. But it still doesn't look like a good solution.

Your environment 🌎

npx @mui/envinfo ``` Don't forget to mention which browser you used. Output from `npx @mui/envinfo` goes here. ```

Order ID or Support key 💳 (optional)

58079

m-konkov commented 1 year ago

I think it can be possible to reset the values ​​when rendering with useRef and applyValue in condition:

 const refColumnField = useRef(item.columnField);

 if (refColumnField.current !== item.columnField) {
     applyValue({ ....item, value: emptyValue }); // reset filter value on change column field
     refColumnField.current = item.columnField
 }

 return ... // InputComponent

And one more thing: Why is useEffect and useState is used to sync input state in packages\grid\x-data-grid\src\components\panel\filterPanel\GridFilterInputMultipleSingleSelect.tsx and other default InputComponents? Although you can directly use the values from the props InputComponent props.

m4theushw commented 1 year ago

We should remove this effect and try to move the logic to inside the component that is rendering each item. To not add operator-specific logic inside it we could add callback to the operator definition to get the initial values for a operator. Something like that:

diff --git a/packages/grid/x-data-grid/src/components/panel/filterPanel/GridFilterForm.tsx b/packages/grid/x-data-grid/src/components/panel/filterPanel/GridFilterForm.tsx
index 1e13b9b5e..26811b829 100644
--- a/packages/grid/x-data-grid/src/components/panel/filterPanel/GridFilterForm.tsx
+++ b/packages/grid/x-data-grid/src/components/panel/filterPanel/GridFilterForm.tsx
@@ -301,11 +301,16 @@ const GridFilterForm = React.forwardRef<HTMLDivElement, GridFilterFormProps>(
           !newOperator.InputComponent ||
           newOperator.InputComponent !== currentOperator?.InputComponent;

+        let initialValue = eraseItemValue ? undefined : item.value;
+        if (newOperator.getInitialValue) {
+          initialValue = newOperator.getInitialValue(item.value);
+        }
+
         applyFilterChanges({
           ...item,
           field,
           operator: newOperator.value,
-          value: eraseItemValue ? undefined : item.value,
+          value: initialValue,
         });
       },
       [apiRef, applyFilterChanges, item, currentColumn, currentOperator],

I found a small workaround

It's not a workaround. If the InputComponent is different we erase the value.

https://github.com/mui/mui-x/blob/ea2f35d69bd005280766f74fd526c702990bb021/packages/grid/x-data-grid/src/components/panel/filterPanel/GridFilterForm.tsx#L299-L302

Why is useEffect and useState is used to sync input state in packages\grid\x-data-grid\src\components\panel\filterPanel\GridFilterInputMultipleSingleSelect.tsx and other default InputComponents?

We do that way to separate the concerns. The useGridFilter hook doesn't contain specific logic for a particular operator, everything has to go inside the input component.

garyjohnson commented 1 year ago

I'm encountering this issue in our project as well, with two additional wrinkles that make this more severe for us:

1) We're using valueOptions, and the previous value is rendered as a filter on the newly selected column if it is the same type but even when it isn't a valid option on the new column. 2) We're using the filterModel to drive params of a web request (results are paginated server-side). This results in an invalid param being sent to the server, causing unnecessary 500 errors.

`npx @mui/envinfo` ``` System: OS: macOS 13.5.1 Binaries: Node: 18.15.0 - ~/.nvm/versions/node/v18.15.0/bin/node Yarn: Not Found npm: 9.5.0 - ~/.nvm/versions/node/v18.15.0/bin/npm Browsers: Chrome: 116.0.5845.140 Edge: 116.0.1938.62 Safari: 16.6 npmPackages: @emotion/react: 11.10.5 @emotion/styled: 11.10.5 @mui/base: 5.0.0-beta.5 @mui/core-downloads-tracker: 5.13.4 @mui/lab: 5.0.0-alpha.140 @mui/material: 5.13.6 @mui/private-theming: 5.14.5 @mui/styled-engine: 5.13.2 @mui/system: 5.14.5 @mui/types: 7.2.4 @mui/utils: 5.14.5 @mui/x-data-grid: 6.9.0 @mui/x-data-grid-pro: 6.9.0 @mui/x-date-pickers: 6.9.0 @mui/x-date-pickers-pro: 6.9.0 @mui/x-license-pro: 6.9.0 @types/react: ^18.2.4 => 18.2.20 react: ^18.2.0 => 18.2.0 react-dom: ^18.2.0 => 18.2.0 typescript: ~5.1.6 => 5.1.6 ```

Order ID 72484