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

Jump to bottom

[DataGrid] Fix support for singleSelect with numeric values #2112

Merged
merged 5 commits into from Jul 23, 2021
Merged

[DataGrid] Fix support for singleSelect with numeric values #2112

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
e9a3622
[DataGrid] Fix support for singleSelect with numeric values
m4theushw Jul 9, 2021
f6da95a
Small detail
oliviertassinari Jul 17, 2021
82b8223
Any is too broad
m4theushw Jul 22, 2021
028001c
prettier
m4theushw Jul 22, 2021
2307a19
Update packages/grid/data-grid/src/tests/filtering.DataGrid.test.tsx
m4theushw Jul 22, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion docs/pages/api-docs/data-grid/grid-col-def.md
View file
Open in desktop
Expand Up @@ -38,6 +38,6 @@ import { GridColDef } from '@material-ui/data-grid';
| <span class="prop-name optional">type<sup><abbr title="optional">?</abbr></sup></span> | <span class="prop-type">string</span> | <span class="prop-default">'string'<br /></span> | Type allows to merge this object with a default definition [GridColDef](/api/data-grid/grid-col-def/). |
| <span class="prop-name optional">valueFormatter<sup><abbr title="optional">?</abbr></sup></span> | <span class="prop-type">(params: GridValueFormatterParams) =&gt; GridCellValue</span> | | Function that allows to apply a formatter before rendering its value. |
| <span class="prop-name optional">valueGetter<sup><abbr title="optional">?</abbr></sup></span> | <span class="prop-type">(params: GridValueGetterParams) =&gt; GridCellValue</span> | | Function that allows to get a specific data instead of field to render in the cell. |
| <span class="prop-name optional">valueOptions<sup><abbr title="optional">?</abbr></sup></span> | <span class="prop-type">(string \| { label: string; value: any })[]</span> | | To be used in combination with `type: 'singleSelect'`. This is an array of the possible cell values and labels. |
| <span class="prop-name optional">valueOptions<sup><abbr title="optional">?</abbr></sup></span> | <span class="prop-type">(string \| number \| { label: string; value: any })[]</span> | | To be used in combination with `type: 'singleSelect'`. This is an array of the possible cell values and labels. |
| <span class="prop-name optional">valueParser<sup><abbr title="optional">?</abbr></sup></span> | <span class="prop-type">(value: GridCellValue, params?: GridCellParams) =&gt; GridCellValue</span> | | Function that takes the user-entered value and converts it to a value used internally. |
| <span class="prop-name optional">width<sup><abbr title="optional">?</abbr></sup></span> | <span class="prop-type">number</span> | <span class="prop-default">100<br /></span> | Set the width of the column. |
10 changes: 5 additions & 5 deletions packages/grid/_modules_/grid/components/cell/GridEditSingleSelectCell.tsx
View file
Open in desktop
Expand Up @@ -5,14 +5,14 @@ import { GridCellParams } from '../../models/params/gridCellParams';
import { isEscapeKey } from '../../utils/keyboardUtils';

const renderSingleSelectOptions = (option) =>
typeof option === 'string' ? (
<MenuItem key={option} value={option}>
{option}
</MenuItem>
) : (
typeof option === 'object' ? (
<MenuItem key={option.value} value={option.value}>
{option.label}
</MenuItem>
) : (
<MenuItem key={option} value={option}>
{option}
</MenuItem>
);

export function GridEditSingleSelectCell(props: GridCellParams & SelectProps) {
Expand Down
22 changes: 15 additions & 7 deletions packages/grid/_modules_/grid/components/panel/filterPanel/GridFilterInputValue.tsx
View file
Open in desktop
Expand Up @@ -8,14 +8,14 @@ import { GridColDef } from '../../../models/colDef/gridColDef';

const renderSingleSelectOptions = ({ valueOptions }: GridColDef) =>
['', ...valueOptions!].map((option) =>
typeof option === 'string' ? (
<option key={option} value={option}>
{option}
</option>
) : (
typeof option === 'object' ? (
<option key={option.value} value={option.value}>
{option.label}
</option>
) : (
<option key={option} value={option}>
{option}
</option>
),
);

Expand Down Expand Up @@ -44,16 +44,24 @@ export function GridFilterInputValue(props: GridTypeFilterInputValueProps & Text

const onFilterChange = React.useCallback(
(event) => {
let value = event.target.value;
// NativeSelect casts the value to a string.
if (type === 'singleSelect') {
oliviertassinari marked this conversation as resolved.
Show resolved Hide resolved
const column = apiRef.current.getColumn(item.columnField);
value = column.valueOptions
.map((option) => (typeof option === 'object' ? option.value : option))
.find((optionValue) => String(optionValue) === value);
}

clearTimeout(filterTimeout.current);
const value = event.target.value;
setFilterValueState(value);
setIsApplying(true);
filterTimeout.current = setTimeout(() => {
applyValue({ ...item, value });
setIsApplying(false);
}, SUBMIT_FILTER_STROKE_TIME);
},
[applyValue, item],
[apiRef, applyValue, item, type],
);

React.useEffect(() => {
Expand Down
2 changes: 1 addition & 1 deletion packages/grid/_modules_/grid/models/colDef/gridColDef.ts
View file
Open in desktop
Expand Up @@ -74,7 +74,7 @@ export interface GridColDef {
/**
* To be used in combination with `type: 'singleSelect'`. This is an array of the possible cell values and labels.
*/
valueOptions?: Array<string | { value: any; label: string }>;
valueOptions?: Array<string | number | { value: any; label: string }>;
Copy link
Member

Choose a reason for hiding this comment

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

Can we use GridCellValue?

Copy link
Member

Choose a reason for hiding this comment

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

I don't think we currently support options of type Date (which is included in GridCellValue)

It will be considered as an object here https://github.com/mui-org/material-ui-x/pull/2112/files#diff-e03ef6ac8071c6dbcd6ceddc294a6ceb9692826571ae45332126f34b953d35efR8

Copy link
Member

Choose a reason for hiding this comment

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

How would I do a single select for a list of country for example, with the flag and country name?
I think the select options should consider renderCell, valueFormatter or getter...

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 following you

Copy link
Member

Choose a reason for hiding this comment

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

Well it's off topic: but I think this feature is a bit too basic at this stage and should consider renderCell, valueGetter, valueFormatter...

Copy link
Member

Choose a reason for hiding this comment

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

This PR add the support of number, it's a step forward but I do agree that we still don't support everything we should

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 singleSelect is only a column type, like the others. The user can provide a custom cell or a different data structures.

/**
* Allows to align the column values in cells.
*/
Expand Down
19 changes: 10 additions & 9 deletions packages/grid/_modules_/grid/models/colDef/gridSingleSelectOperators.ts
View file
Open in desktop
Expand Up @@ -6,13 +6,14 @@ export const getGridSingleSelectOperators: () => GridFilterOperator[] = () => [
{
value: 'is',
getApplyFilterFn: (filterItem: GridFilterItem) => {
if (!filterItem.columnField || !filterItem.value || !filterItem.operatorValue) {
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're already validating if there's a columnField and operatorValue before calling the filter function. Related to #1961 (comment)

if (filterItem.value == null || filterItem.value === '') {
return null;
}
return ({ value }): boolean => {
return typeof value === 'string'
? filterItem.value === value
: filterItem.value === (value as { value: any; label: string }).value;
if (typeof value === 'object') {
return filterItem.value === (value as { value: any; label: string }).value;
}
return filterItem.value === value;
};
},
InputComponent: GridFilterInputValue,
Expand All @@ -21,14 +22,14 @@ export const getGridSingleSelectOperators: () => GridFilterOperator[] = () => [
{
value: 'not',
getApplyFilterFn: (filterItem: GridFilterItem) => {
if (!filterItem.columnField || !filterItem.value || !filterItem.operatorValue) {
if (filterItem.value == null || filterItem.value === '') {
return null;
}

return ({ value }): boolean => {
return typeof value === 'string'
? filterItem.value !== value
: filterItem.value !== (value as { value: any; label: string }).value;
if (typeof value === 'object') {
return filterItem.value !== (value as { value: any; label: string }).value;
}
return filterItem.value !== value;
};
},
InputComponent: GridFilterInputValue,
Expand Down
205 changes: 168 additions & 37 deletions packages/grid/data-grid/src/tests/filtering.DataGrid.test.tsx
View file
Open in desktop
Expand Up @@ -2,9 +2,17 @@ import * as React from 'react';
import {
createClientRenderStrictMode, // @ts-expect-error need to migrate helpers to TypeScript
screen,
// @ts-expect-error need to migrate helpers to TypeScript
fireEvent,
} from 'test/utils';
import { expect } from 'chai';
import { DataGrid, GridToolbar } from '@material-ui/data-grid';
import { useFakeTimers } from 'sinon';
import {
DataGrid,
GridToolbar,
GridPreferencePanelsValue,
GridLinkOperator,
} from '@material-ui/data-grid';
import { getColumnValues } from 'test/utils/helperFn';

const isJSDOM = /jsdom/.test(window.navigator.userAgent);
Expand All @@ -21,18 +29,21 @@ describe('<DataGrid /> - Filter', () => {
brand: 'Nike',
isPublished: false,
country: 'United States',
status: 0,
},
{
id: 1,
brand: 'Adidas',
isPublished: true,
country: 'Germany',
status: 0,
},
{
id: 2,
brand: 'Puma',
isPublished: true,
country: 'Germany',
status: 2,
},
],
columns: [
Expand All @@ -43,6 +54,15 @@ describe('<DataGrid /> - Filter', () => {
type: 'singleSelect',
valueOptions: ['United States', 'Germany', 'France'],
},
{
field: 'status',
type: 'singleSelect',
valueOptions: [
{ value: 0, label: 'Payment Pending' },
{ value: 1, label: 'Shipped' },
{ value: 2, label: 'Delivered' },
],
},
],
};

Expand All @@ -52,8 +72,9 @@ describe('<DataGrid /> - Filter', () => {
operatorValue?: string;
value?: any;
field?: string;
state?: any;
}) => {
const { operatorValue, value, rows, columns, field = 'brand' } = props;
const { operatorValue, value, rows, columns, field = 'brand', ...other } = props;
return (
<div style={{ width: 300, height: 300 }}>
<DataGrid
Expand All @@ -70,6 +91,7 @@ describe('<DataGrid /> - Filter', () => {
],
}}
disableColumnFilter={false}
{...other}
/>
</div>
);
Expand Down Expand Up @@ -626,49 +648,158 @@ describe('<DataGrid /> - Filter', () => {
});
});

describe('select operators', () => {
it('should allow operator is', () => {
const { setProps } = render(<TestCase value="a" operatorValue="contains" />);
setProps({
field: 'country',
operatorValue: 'is',
value: 'United States',
describe('singleSelect operators', () => {
let clock;

beforeEach(() => {
clock = useFakeTimers();
});

afterEach(() => {
clock.restore();
});

describe('simple options', () => {
it('should allow operator is', () => {
const { setProps } = render(<TestCase value="a" operatorValue="contains" />);
setProps({
field: 'country',
operatorValue: 'is',
value: 'United States',
});
expect(getColumnValues()).to.deep.equal(['Nike']);
setProps({
field: 'country',
operatorValue: 'is',
value: 'Germany',
});
expect(getColumnValues()).to.deep.equal(['Adidas', 'Puma']);
setProps({
field: 'country',
operatorValue: 'is',
value: '',
});
expect(getColumnValues()).to.deep.equal(['Nike', 'Adidas', 'Puma']);
});
expect(getColumnValues()).to.deep.equal(['Nike']);
setProps({
field: 'country',
operatorValue: 'is',
value: 'Germany',

it('should allow operator not', () => {
const { setProps } = render(<TestCase value="a" operatorValue="contains" />);
setProps({
field: 'country',
operatorValue: 'not',
value: 'United States',
});
expect(getColumnValues()).to.deep.equal(['Adidas', 'Puma']);
setProps({
field: 'country',
operatorValue: 'not',
value: 'Germany',
});
expect(getColumnValues()).to.deep.equal(['Nike']);
setProps({
field: 'country',
operatorValue: 'not',
value: '',
});
expect(getColumnValues()).to.deep.equal(['Nike', 'Adidas', 'Puma']);
});
expect(getColumnValues()).to.deep.equal(['Adidas', 'Puma']);
setProps({
field: 'country',
operatorValue: 'is',
value: '',

it('should work with numeric values', () => {
render(
<TestCase
rows={[
{ id: 1, name: 'Hair Dryer', voltage: 220 },
{ id: 2, name: 'Dishwasher', voltage: 110 },
{ id: 3, name: 'Microwave', voltage: 220 },
]}
columns={[
{ field: 'name' },
{ field: 'voltage', type: 'singleSelect', valueOptions: [220, 110] },
]}
state={{
preferencePanel: {
open: true,
openedPanelValue: GridPreferencePanelsValue.filters,
},
filter: {
items: [{ id: 123, columnField: 'voltage', value: '', operatorValue: 'is' }],
linkOperator: GridLinkOperator.And,
},
}}
/>,
);
expect(getColumnValues()).to.deep.equal(['Hair Dryer', 'Dishwasher', 'Microwave']);
fireEvent.change(screen.getByLabelText('Value'), { target: { value: '220' } });
clock.tick(600);
m4theushw marked this conversation as resolved.
Show resolved Hide resolved
m4theushw marked this conversation as resolved.
Show resolved Hide resolved
expect(getColumnValues()).to.deep.equal(['Hair Dryer', 'Microwave']);
});
expect(getColumnValues()).to.deep.equal(['Nike', 'Adidas', 'Puma']);
});

it('should allow operator not', () => {
const { setProps } = render(<TestCase value="a" operatorValue="contains" />);
setProps({
field: 'country',
operatorValue: 'not',
value: 'United States',
describe('complex options', () => {
it('should allow operator is', () => {
const { setProps } = render(<TestCase value="a" operatorValue="contains" />);
setProps({
field: 'status',
operatorValue: 'is',
value: 0,
});
expect(getColumnValues()).to.deep.equal(['Nike', 'Adidas']);
setProps({
field: 'status',
operatorValue: 'is',
value: 2,
});
expect(getColumnValues()).to.deep.equal(['Puma']);
setProps({
field: 'status',
operatorValue: 'is',
value: '',
});
expect(getColumnValues()).to.deep.equal(['Nike', 'Adidas', 'Puma']);
});
expect(getColumnValues()).to.deep.equal(['Adidas', 'Puma']);
setProps({
field: 'country',
operatorValue: 'not',
value: 'Germany',

it('should allow operator not', () => {
const { setProps } = render(<TestCase value="a" operatorValue="contains" />);
setProps({
field: 'status',
operatorValue: 'not',
value: 0,
});
expect(getColumnValues()).to.deep.equal(['Puma']);
setProps({
field: 'status',
operatorValue: 'not',
value: 2,
});
expect(getColumnValues()).to.deep.equal(['Nike', 'Adidas']);
setProps({
field: 'status',
operatorValue: 'not',
value: '',
});
expect(getColumnValues()).to.deep.equal(['Nike', 'Adidas', 'Puma']);
});
expect(getColumnValues()).to.deep.equal(['Nike']);
setProps({
field: 'country',
operatorValue: 'not',
value: '',

it('should work with numeric values', () => {
render(
<TestCase
state={{
preferencePanel: {
open: true,
openedPanelValue: GridPreferencePanelsValue.filters,
},
filter: {
items: [{ id: 123, columnField: 'status', value: '', operatorValue: 'is' }],
linkOperator: GridLinkOperator.And,
},
}}
/>,
);
expect(getColumnValues()).to.deep.equal(['Nike', 'Adidas', 'Puma']);
fireEvent.change(screen.getByLabelText('Value'), { target: { value: '2' } });
clock.tick(600);
expect(getColumnValues()).to.deep.equal(['Puma']);
});
expect(getColumnValues()).to.deep.equal(['Nike', 'Adidas', 'Puma']);
});
});

Expand Down