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] Fix cell value type in quick filtering v7 #10884

Merged
merged 5 commits into from
Nov 6, 2023
Merged

[DataGrid] Fix cell value type in quick filtering v7 #10884

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
456018f
fix cell value type in quick filtering v7
cherniavskii Nov 3, 2023
c89d525
make ignoreDiacritics work in quick filtering
cherniavskii Nov 4, 2023
a768f31
avoid exposing private apiRef
cherniavskii Nov 4, 2023
f41c87e
reduce the number of getPublicApiRef calls
cherniavskii Nov 6, 2023
decaba4
Merge branch 'master' into fix-cellValue-type-in-quick-filter-v7
cherniavskii Nov 6, 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
1 change: 1 addition & 0 deletions docs/pages/x/api/data-grid/grid-api.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ import { GridApi } from '@mui/x-data-grid';
| <span class="prop-name">hideFilterPanel</span> | <span class="prop-type">() =&gt; void</span> | Hides the filter panel. |
| <span class="prop-name">hideHeaderFilterMenu</span> | <span class="prop-type">() =&gt; void</span> | Hides the header filter menu. |
| <span class="prop-name">hidePreferences</span> | <span class="prop-type">() =&gt; void</span> | Hides the preferences panel. |
| <span class="prop-name">ignoreDiacritics</span> | <span class="prop-type">DataGridProcessedProps['ignoreDiacritics']</span> | Returns the value of the `ignoreDiacritics` prop. |
| <span class="prop-name">isCellEditable</span> | <span class="prop-type">(params: GridCellParams) =&gt; boolean</span> | Controls if a cell is editable. |
| <span class="prop-name">isColumnPinned [<span class="plan-pro" title="Pro plan"></span>](/x/introduction/licensing/#pro-plan)</span> | <span class="prop-type">(field: string) =&gt; GridPinnedPosition \| false</span> | Returns which side a column is pinned to. |
| <span class="prop-name">isRowSelectable</span> | <span class="prop-type">(id: GridRowId) =&gt; boolean</span> | Determines if a row can be selected or not. |
Expand Down
5 changes: 5 additions & 0 deletions docs/pages/x/api/data-grid/grid-filter-api.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,11 @@
"type": "(item: GridFilterItem) => void"
},
{ "name": "hideFilterPanel", "description": "Hides the filter panel.", "type": "() => void" },
{
"name": "ignoreDiacritics",
"description": "Returns the value of the <code>ignoreDiacritics</code> prop.",
"type": "DataGridProcessedProps['ignoreDiacritics']"
},
{
"name": "setFilterLogicOperator",
"description": "Changes the GridLogicOperator used to connect the filters.",
Expand Down
3 changes: 2 additions & 1 deletion packages/grid/x-data-grid-premium/src/hooks/features/clipboard/useGridClipboardImport.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import {
getActiveElement,
GridPipeProcessor,
useGridRegisterPipeProcessor,
getPublicApiRef,
} from '@mui/x-data-grid/internals';
import { GRID_DETAIL_PANEL_TOGGLE_FIELD, GRID_REORDER_COL_DEF } from '@mui/x-data-grid-pro';
import { unstable_debounce as debounce } from '@mui/utils';
Expand Down Expand Up @@ -379,7 +380,7 @@ export const useGridClipboardImport = (

defaultPasteResolver({
pastedData,
apiRef: { current: apiRef.current.getPublicApi() },
apiRef: getPublicApiRef(apiRef),
updateCell: (...args) => {
cellUpdater.updateCell(...args);
},
Expand Down
7 changes: 6 additions & 1 deletion packages/grid/x-data-grid/src/colDef/gridStringOperators.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,19 @@ import { GridFilterItem } from '../models/gridFilterItem';
import { GridFilterOperator } from '../models/gridFilterOperator';
import { GridFilterInputMultipleValue } from '../components/panel/filterPanel/GridFilterInputMultipleValue';
import { convertLegacyOperators, tagInternalFilter } from './utils';
import { removeDiacritics } from '../hooks/features/filter/gridFilterUtils';

export const getGridStringQuickFilterFn = tagInternalFilter(
(value: any): GridApplyQuickFilterV7 | null => {
if (!value) {
return null;
}
const filterRegex = new RegExp(escapeRegExp(value), 'i');
return (columnValue): boolean => {
return (_, row, column, apiRef): boolean => {
let columnValue = apiRef.current.getRowFormattedValue(row, column);
if (apiRef.current.ignoreDiacritics) {
columnValue = removeDiacritics(columnValue);
}
Comment on lines +17 to +20
Copy link
Member Author

Choose a reason for hiding this comment

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

This is a disadvantage compared to v6 filtering, where it was possible to overwrite the cellParams.formattedValue.

return columnValue != null ? filterRegex.test(columnValue.toString()) : false;
};
},
Expand Down
13 changes: 7 additions & 6 deletions packages/grid/x-data-grid/src/hooks/features/filter/gridFilterUtils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import {
GridQuickFilterValueResult,
} from './gridFilterState';
import { buildWarning } from '../../../utils/warning';
import { getPublicApiRef } from '../../../utils/getPublicApiRef';
import {
gridColumnFieldsSelector,
gridColumnLookupSelector,
Expand Down Expand Up @@ -152,7 +153,7 @@ export const mergeStateWithFilterModel =
filterModel: sanitizeFilterModel(filterModel, disableMultipleColumnsFiltering, apiRef),
});

const removeDiacritics = (value: unknown) => {
export const removeDiacritics = (value: unknown) => {
if (typeof value === 'string') {
return value.normalize('NFD').replace(/[\u0300-\u036f]/g, '');
}
Expand Down Expand Up @@ -220,7 +221,7 @@ const getFilterCallbackFromItem = (
if (ignoreDiacritics) {
value = removeDiacritics(value);
}
return applyFilterOnRow(value, row, column, apiRef);
return applyFilterOnRow(value, row, column, getPublicApiRef(apiRef));
Copy link
Member Author

Choose a reason for hiding this comment

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

We were passing private apiRef here

Copy link
Member

Choose a reason for hiding this comment

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

Oops, nice catch 👍

Copy link
Contributor

Choose a reason for hiding this comment

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

Can we create the API ref outside this function? I think this one is inside the filter loop, so we're doing O(n) allocations if we call getPublicApiRef here.

Copy link
Contributor

Choose a reason for hiding this comment

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

Same comment applies below, line 420.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for catching it, updated it!

},
};
}
Expand Down Expand Up @@ -371,7 +372,7 @@ const buildAggregatedQuickFilterApplier = (
const value = ignoreDiacritics ? removeDiacritics(quickFilterValue) : quickFilterValue;
return {
v7: true,
fn: getApplyQuickFilterFnV7(value, column, apiRef),
fn: getApplyQuickFilterFnV7(value, column, getPublicApiRef(apiRef)),
};
}),
});
Expand All @@ -382,7 +383,7 @@ const buildAggregatedQuickFilterApplier = (
const value = ignoreDiacritics ? removeDiacritics(quickFilterValue) : quickFilterValue;
return {
v7: false,
fn: getApplyQuickFilterFn(value, column, apiRef),
fn: getApplyQuickFilterFn(value, column, getPublicApiRef(apiRef)),
};
}),
});
Expand All @@ -406,7 +407,7 @@ const buildAggregatedQuickFilterApplier = (
}

const applier = appliers[v];
let value = apiRef.current.getRowFormattedValue(row, column);
let value = apiRef.current.getRowValue(row, column);

if (applier.fn === null) {
continue;
Expand All @@ -416,7 +417,7 @@ const buildAggregatedQuickFilterApplier = (
if (ignoreDiacritics) {
value = removeDiacritics(value);
}
const isMatching = applier.fn(value, row, column, apiRef);
const isMatching = applier.fn(value, row, column, getPublicApiRef(apiRef));
if (isMatching) {
result[filterValue] = true;
continue outer;
Expand Down
2 changes: 2 additions & 0 deletions packages/grid/x-data-grid/src/hooks/features/filter/useGridFilter.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@ export const useGridFilter = (
| 'slotProps'
| 'disableColumnFilter'
| 'disableEval'
| 'ignoreDiacritics'
>,
): void => {
const logger = useGridLogger(apiRef, 'useGridFilter');
Expand Down Expand Up @@ -329,6 +330,7 @@ export const useGridFilter = (
showFilterPanel,
hideFilterPanel,
setQuickFilterValues,
ignoreDiacritics: props.ignoreDiacritics,
};

useGridApiMethod(apiRef, filterApi, 'public');
Expand Down
1 change: 1 addition & 0 deletions packages/grid/x-data-grid/src/internals/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,7 @@ export { isNavigationKey } from '../utils/keyboardUtils';
export { clamp, isDeepEqual, isNumber, isFunction, isObject } from '../utils/utils';
export { buildWarning } from '../utils/warning';
export { exportAs } from '../utils/exportAs';
export * from '../utils/getPublicApiRef';
export type { GridPrivateOnlyApiCommon } from '../models/api/gridApiCommon';
export { useGridPrivateApiContext } from '../hooks/utils/useGridPrivateApiContext';
export * from '../hooks/utils/useOnMount';
Expand Down
5 changes: 5 additions & 0 deletions packages/grid/x-data-grid/src/models/api/gridFilterApi.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { GridFilterModel } from '../gridFilterModel';
import { GridFilterItem, GridLogicOperator } from '../gridFilterItem';
import { GridControlledStateReasonLookup } from '../events';
import { DataGridProcessedProps } from '../props/DataGridProps';

/**
* The filter API interface that is available in the grid [[apiRef]].
Expand Down Expand Up @@ -56,4 +57,8 @@ export interface GridFilterApi {
* @param {any[]} values The list of element to quick filter
*/
setQuickFilterValues: (values: any[]) => void;
/**
* Returns the value of the `ignoreDiacritics` prop.
*/
ignoreDiacritics: DataGridProcessedProps['ignoreDiacritics'];
}
9 changes: 9 additions & 0 deletions packages/grid/x-data-grid/src/utils/getPublicApiRef.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
import type { GridPrivateApiCommunity } from '../models/api/gridApiCommunity';

export function getPublicApiRef<PrivateApi extends GridPrivateApiCommunity>(
apiRef: React.MutableRefObject<PrivateApi>,
) {
return { current: apiRef.current.getPublicApi() } as React.MutableRefObject<
ReturnType<PrivateApi['getPublicApi']>
>;
}