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/
3.79k stars 1.12k forks source link

[data grid] `formattedValue` not based on edited value in renderEditCell #12860

Closed AdamBeWell closed 4 days ago

AdamBeWell commented 2 weeks ago

Steps to reproduce

Link to live example: latest - https://stackblitz.com/edit/react-5zo6n1?file=Demo.tsx v6 - https://stackblitz.com/edit/react-p2uh2s?file=Demo.tsx

Steps:

  1. Try to edit a cell in column Full name

Current behavior

If using value from props as the value for the edit field in renderEditCell, everything works fine. If using formattedValue from props, the formattedValue is always based on the original value and not on the edited value.

Expected behavior

The formattedValue should be updated based on the new edited value after it is parsed and formatted.

Context

If I want the values in my array to be different from the value displayed in the datagrid, valueFormatter cannot be used in conjunction with editing.

In the examples above (derived from examples from the documentation), if I want the values in my array to be capitalized but the values in the datagrid to be lower case, even when editing, the field cannot be edited when using formattedValue in renderEditCell. The default edit cell component always uses value and not formattedValue.

Very niche use case and arguably not a bug but I think that formattedValue in renderEditCell should be based on the edited value and not the original value.

Your environment

No response

Search keywords: formattedvalue rendereditcell Order ID: 71270

michelengelen commented 1 week ago

Hey @AdamBeWell ... columns with a valueGetter are not meant to be editable. The example you gave is actually a good one: The value in the fullName column is being generated from 2 other columns and therefore we are not able to determine which of the 2 columns should be updated when editing the value there.

There is also a difference between valueGetter and valueFormatter. You can have a field that uses valueFormatter and still be editable (this is a good example of this):

valueFormatter: (value) => {
  if (value == null) {
    return '';
  }
  return `${value.toLocaleString()} %`;
},

Here we format the stored value (a number) and add a % to the end to show that this is a percentage value.

@cherniavskii we could make this clearer with a warning in the valueGetter section. Should we add a recipe for this?

AdamBeWell commented 1 week ago

From my understanding, an editable column with a valueGetter must also have a valueSetter. I see no problem with that and that's how it is in my example and everywhere else in my code. I actually didn't know that you could omit valueGetter, I guess it falls back to the field name at that point.

The value in the fullName column is being generated from 2 other columns and therefore we are not able to determine which of the 2 columns should be updated when editing the value there.

That shouldn't matter here because of valueSetter. After calling setEditCellValue I would expected the edited value to go through valueParser, then valueSetter and then the GridRenderEditCellParams of renderEditCell to be updated with value from valueGetter and formattedValuefrom valueFormatter with the new edited value. And this is how it seems to work, except for formattedValue in GridRenderEditCellParams is seemingly not updated or is always based on the original value.

In short, formattedValuein GridRenderEditCellParams is basically useless as it is. Looking into it a bit more, I see that the formattedValue I'm using in GridRenderEditCellParams is extended from GridCellParams and probably not meant no be updated unlike value which is extended from both GridEditCellProps and GridCellParams.

My actual use case was for a column with persisted user ids but with displayed user names, even when editing. valueGetter gets the ids, valueFormattertranslates those ids to their names, valueParser translates the new names from the new value back to ids and valueSetter sets the new ids. And that works for the first render but once you try to edit the value, no change is recorded, like in my example.

There is also a difference between valueGetter and valueFormatter. You can have a field that uses valueFormatter and still be editable (this is a good example of this): Here we format the stored value (a number) and add a % to the end to show that this is a percentage value.

But if you want to display the % sign when the cell is being edited, you can't. You could clean up the value/remove the sign after in valueParser. It doesn't really make sense in this case but I think you should be able to.

cherniavskii commented 1 week ago

Hey @AdamBeWell This is a very interesting and unusual use case, thanks for the detailed explanation! I'll see what we can do on our side!

cherniavskii commented 1 week ago

@AdamBeWell Can you take a look at this demo? Is this what you're looking for?

AdamBeWell commented 1 week ago

@cherniavskii Yes, that is exactly the behavior I was expecting and the way I was trying to achieve it. The value is capitalized but the formattedValue is lowercase and updates properly when editing.

cherniavskii commented 1 week ago

I've opened https://github.com/mui/mui-x/pull/12870, and I'll check with the team if there are any objections to merging it 👍🏻

github-actions[bot] commented 4 days ago

:warning: This issue has been closed. If you have a similar problem but not exactly the same, please open a new issue. Now, if you have additional information related to this issue or things that could help future readers, feel free to leave a comment.

@AdamBeWell: How did we do? Your experience with our support team matters to us. If you have a moment, please share your thoughts in this short Support Satisfaction survey.