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] Lower filter debounce delay #9712

Merged
merged 23 commits into from
Aug 14, 2023
Merged

[DataGrid] Lower filter debounce delay #9712

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
66c12f4
feat: make SUBMIT_FILTER_STROKE_TIME into a prop
romgrk Jul 18, 2023
3bdf00c
feat
romgrk Jul 18, 2023
9795860
feat
romgrk Jul 18, 2023
0d8babc
lint
romgrk Jul 18, 2023
776062d
lint
romgrk Jul 18, 2023
67fe5b4
build: docs:api
romgrk Jul 18, 2023
db2c5d9
refactor
romgrk Jul 18, 2023
8b6e63e
lint
romgrk Jul 18, 2023
40ae023
Merge branch 'master' into feat-quick-filter-delay
romgrk Jul 18, 2023
01f92ac
refactor
romgrk Jul 18, 2023
810a028
lint
romgrk Jul 18, 2023
5d3c395
lint
romgrk Jul 18, 2023
6fe923f
lint
romgrk Jul 18, 2023
6a1ad56
fix: input re-type bug
romgrk Jul 26, 2023
a6cdda2
ci: run
romgrk Jul 26, 2023
9185ab2
fix: add effect back
romgrk Jul 26, 2023
08315bf
lint
romgrk Jul 26, 2023
5b52b74
Merge branch 'master' into feat-quick-filter-delay
romgrk Aug 1, 2023
9bd9dee
Merge branch 'master' into feat-quick-filter-delay
romgrk Aug 1, 2023
5db6cbb
build
romgrk Aug 1, 2023
a42ae12
fix: filter panel re-type bug
romgrk Aug 3, 2023
f1d05aa
fix
romgrk Aug 11, 2023
e6abe96
lint
romgrk Aug 14, 2023
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
5 changes: 0 additions & 5 deletions packages/grid/x-data-grid/src/components/panel/filterPanel/GridFilterInputValue.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,11 +51,6 @@ function GridFilterInputValue(props: GridTypeFilterInputValueProps) {
[applyValue, item, rootProps.filterDebounceMs, filterTimeout],
);

React.useEffect(() => {
const itemValue = item.value ?? '';
setFilterValueState(String(itemValue));
}, [item.value]);
Copy link
Member

Choose a reason for hiding this comment

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

I believe this effect was covering the use case with a controlled filter model, where the filter model could change while the user has the filter panel open.
Without this effect, the filter input will be out of sync with the actual filter model:

With effect: https://codesandbox.io/s/gracious-khayyam-2xqw3n?file=/demo.tsx
Without effect: https://codesandbox.io/s/dreamy-glitter-h8h4yj?file=/demo.tsx

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've adapted the effect to respond to the model changes specifically, what do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll apply to the other equivalent components if it LGTY.

Copy link
Member

@MBilalShafi MBilalShafi Aug 10, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

The effect was also making sure the value is correctly synced with header filter (which is another rendered instance of GridFilterInputValue), removing it also impacted it too, as it's actually fromInput but it still needs to be synced 😄

Before:
before-hf

After:
after-hf

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ooh good catch, there's so many parts to this thing ^^

I've pushed a fix for that.

Copy link
Member

Choose a reason for hiding this comment

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

Thanks, works as expected now!

Copy link
Member

Choose a reason for hiding this comment

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

@romgrk It had another aspect which I missed catching 😄
Since the rendering of GridFilterInputValue is being done from GridHeaderFilterCell, the item change is incorrectly reflected.
I am thinking of disabling this optimization (fromInput) for the headerFilters, let me know if you have better ideas.


return (
<rootProps.slots.baseTextField
id={id}
Expand Down
7 changes: 1 addition & 6 deletions packages/grid/x-data-grid/src/components/panel/filterPanel/GridFilterPanel.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,12 +87,7 @@ const GridFilterPanel = React.forwardRef<HTMLDivElement, GridFilterPanelProps>(
...other
} = props;

const applyFilter = React.useCallback(
(item: GridFilterItem) => {
apiRef.current.upsertFilterItem(item);
},
[apiRef],
);
const applyFilter = apiRef.current.upsertFilterItem;

romgrk marked this conversation as resolved.
Show resolved Hide resolved
const applyFilterLogicOperator = React.useCallback(
(operator: GridLogicOperator) => {
Expand Down