[DataGrid] Allow `valueOptions` to accept a function #2850

alexfauquette · 2021-10-12T13:53:27Z

For columns with type singleSelect, the valueOptions can now be a function. This function takes an argument row containing the row values and returns an array of options.

valueOptions are also used to filter the column. In that case, the function is called without argument. The user can choose what to return when row === undefenied.

The functionality can be tested here: https://deploy-preview-2850--material-ui-x.netlify.app/components/data-grid/columns/#column-types

DanailH · 2021-10-13T08:03:07Z

packages/grid/_modules_/grid/models/colDef/gridColDef.ts

@@ -83,7 +85,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 | number | { value: any; label: string }>;
+  valueOptions?: ValueOptionsArray | ((params?: GridRowModel) => ValueOptionsArray);


I see we are passing the GridRowModel but I think it may be also valuable to pass in the current column, as in the column field. What do you think?

I do not think it is usefull to pass the column attributes, because valueOptions is defined for a specific function. So it will be used for only one column.

Not necessarily - if I want to pull in the value options from an API for example I would need to know for which column I need to fetch them.

So are we adding the field or proceeding without it?

We could create the GridValueOptionsParams interface with the id, field and row. It's more to keep the same pattern used by the other props. The column we don't need to pass, as it would also include the value options which, if called, could start an infinite loop.

It's done. I'm wondering if we should let id and row undefined in the header (no row profited) or if we should add a boolean property isFilteringOptions which is more explicit than checking if the row is defined.

We can keep id and row undefined when calling from the filter panel. That's how the value formatter works.

https://github.com/mui-org/material-ui-x/blob/6ce542ba7a3e04cb68eb281002630cd97b39795d/packages/grid/_modules_/grid/models/params/gridCellParams.ts#L90-L92

DanailH · 2021-10-13T08:04:48Z

packages/grid/_modules_/grid/components/panel/filterPanel/GridFilterInputValue.tsx

@@ -54,7 +55,9 @@ function GridFilterInputValue(props: GridTypeFilterInputValueProps & TextFieldPr
      // NativeSelect casts the value to a string.
      if (type === 'singleSelect') {
        const column = apiRef.current.getColumn(item.columnField);
-        value = column.valueOptions
+        const columnValueOptions =
+          typeof column.valueOptions === 'function' ? column.valueOptions() : column.valueOptions;


If we also pass in the column field we can provide it here so that devs can know which column they are currently working with.

DanailH · 2021-10-13T08:07:37Z

Looks nice! A general rule of thumb - I would wait on the docs a bit. If you need to do changes to the implementation it will be double work to also fix the docs. You can complete the docs once we nail the implementation.

Also, we would need to add tests for the new functionality. I would apply the same for the tests as with the docs 😉

flaviendelangle · 2021-10-13T12:47:07Z

Somebody knows why the CI do not work on @alexfauquette PRs ?

DanailH · 2021-10-13T14:47:37Z

Somebody knows why the CI do not work on @alexfauquette PRs ?

No idea, it's on both of his PRs !!?

flaviendelangle · 2021-10-13T15:01:35Z

No idea, it's on both of his PRs !!?

Yeah, so I don't think it will solve on next one :/

DanailH · 2021-10-19T08:46:34Z

@alexdanilowicz if Argos is failing you can check the new screenshot and approve it if you think it is not relevant.

alexfauquette · 2021-10-19T08:53:49Z

Done, I think you tagged the wrong guy @DanailH

alexfauquette · 2021-10-20T09:16:42Z

packages/grid/_modules_/grid/models/params/gridValueOptionsParams.ts

+export interface GridValueOptionsParams {
+  /**
+   * The field of the column to which options will be provided
+   */
+  field: string;
+  /**
+   * The grid row id.
+   */
+  id?: GridRowId;
+  /**
+   * The row model of the row that the current cell belongs to.
+   */
+  row?: GridRowModel;
+}


Here is the parameter pass as an argument to the valueOptions. id and row are not passed if it is on the header.

github-actions · 2021-10-20T14:50:58Z

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

m4theushw

It looks nice. We only miss a couple of tests for the edit cell and the filter:

https://github.com/mui-org/material-ui-x/blob/3adb29b16170453b9b3eec0a60f749ff5b2d3d95/packages/grid/x-grid/src/tests/editRows.DataGridPro.test.tsx#L523

https://github.com/mui-org/material-ui-x/blob/3adb29b16170453b9b3eec0a60f749ff5b2d3d95/packages/grid/data-grid/src/tests/filtering.DataGrid.test.tsx#L801

m4theushw · 2021-10-22T12:08:05Z

packages/grid/_modules_/grid/models/colDef/gridColDef.ts

   */
-  valueOptions?: Array<string | number | { value: any; label: string }>;
+  valueOptions?:
+    | Array<string | number | { value: any; label: string }>


Suggested change

| Array<string | number | { value: any; label: string }>

| Array<ValueOptions>

About this, I'm a bit concerned by the types in the doc API. The signature of this props does not seems to be clear.

Before, it was

(string | number | { label: string; value: any })[]

So ok, I know I can pass a value or a specific type of objects to the prop. Now the type is

ValueOptions[] | ((params: GridValueOptionsParams) => ValueOptions[])

It is not possible to find what ValueOptions and GridValueOptionsParams are in the doc API.

Yeah, the docs is not great yet. We have been iterating it for a long time. In the future, there will proper docs for these small type definitions. See #1529 (comment). For now, we can keep ValueOptions.

docs/src/pages/components/data-grid/columns/ColumnTypesGrid.tsx

m4theushw

Looks good to me. I merged with next to fix one the the failing screenshots. You can approve the other one in argos.

alexfauquette self-assigned this Oct 13, 2021

DanailH reviewed Oct 13, 2021

View reviewed changes

DanailH changed the title ~~[DataGrid] allows valueOptions to be a function~~ [DataGrid] Allow valueOptions to accept a function Oct 13, 2021

alexfauquette force-pushed the adapt-optionValues-to-row branch from 1d0d319 to 6f21838 Compare October 20, 2021 09:13

alexfauquette commented Oct 20, 2021

View reviewed changes

github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Oct 20, 2021

DanailH approved these changes Oct 22, 2021

View reviewed changes

m4theushw reviewed Oct 22, 2021

View reviewed changes

alexfauquette and others added 13 commits October 22, 2021 17:10

allows valueOptions to be a function

48393a2

add a column example in the doc

184f6db

rerun ci

3a7061c

proptype

2829fa4

docs:api

b2be092

pass GridValueOptionsParams as parameter to valueOptions function

e857f7b

rerun ci

1445898

docs:api

200915c

prettier

c6f79ef

PR feedbacks

5b5051b

add tests

26f687e

prettier

ffc1d65

run ci scripts

a9628be

alexfauquette force-pushed the adapt-optionValues-to-row branch from bc3aeab to a9628be Compare October 22, 2021 15:32

github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Oct 22, 2021

alexfauquette requested a review from DanailH October 27, 2021 08:05

Merge branch 'next' into HEAD

6ed001d

m4theushw approved these changes Oct 28, 2021

View reviewed changes

alexfauquette merged commit c7c80bc into mui:next Oct 28, 2021

alexfauquette deleted the adapt-optionValues-to-row branch October 28, 2021 09:19

oliviertassinari added the component: data grid This is the name of the generic UI component, not the React module! label Oct 28, 2021

oliviertassinari added the new feature New feature or request label Jan 20, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[DataGrid] Allow `valueOptions` to accept a function #2850

[DataGrid] Allow `valueOptions` to accept a function #2850

alexfauquette commented Oct 12, 2021 •

edited

Loading

DanailH Oct 13, 2021

alexfauquette Oct 13, 2021

DanailH Oct 13, 2021

DanailH Oct 19, 2021

m4theushw Oct 20, 2021

alexfauquette Oct 20, 2021

m4theushw Oct 20, 2021

DanailH Oct 13, 2021

DanailH commented Oct 13, 2021

flaviendelangle commented Oct 13, 2021

DanailH commented Oct 13, 2021

flaviendelangle commented Oct 13, 2021

DanailH commented Oct 19, 2021

alexfauquette commented Oct 19, 2021

alexfauquette Oct 20, 2021

github-actions bot commented Oct 20, 2021

m4theushw left a comment

m4theushw Oct 22, 2021

alexfauquette Oct 22, 2021 •

edited

Loading

m4theushw Oct 28, 2021

m4theushw left a comment

	\| Array<string \| number \| { value: any; label: string }>
	\| Array<ValueOptions>

[DataGrid] Allow valueOptions to accept a function #2850

[DataGrid] Allow valueOptions to accept a function #2850

Conversation

alexfauquette commented Oct 12, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DanailH commented Oct 13, 2021

flaviendelangle commented Oct 13, 2021

DanailH commented Oct 13, 2021

flaviendelangle commented Oct 13, 2021

DanailH commented Oct 19, 2021

alexfauquette commented Oct 19, 2021

Choose a reason for hiding this comment

github-actions bot commented Oct 20, 2021

m4theushw left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alexfauquette Oct 22, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

m4theushw left a comment

Choose a reason for hiding this comment

[DataGrid] Allow `valueOptions` to accept a function #2850

[DataGrid] Allow `valueOptions` to accept a function #2850

alexfauquette commented Oct 12, 2021 •

edited

Loading

alexfauquette Oct 22, 2021 •

edited

Loading