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.9k stars 1.19k forks source link

[data grid] Valueparser not called when clearing cell #13628

Open louisaaron opened 3 days ago

louisaaron commented 3 days ago

The problem in depth

Here's the current flow when you edit a selected cell (single click not double click) with a custom cell editor then start typing a value:

  1. DataGrid calls colDef.valueSetter with empty string (even if the colDef value type IS NOT STRING) to "clear" the cell
  2. It opens the cell editor and forwards the key press event.
  3. Generally, the renderer's onChange function is called, which calls startCellEditMode, setEditCellValue, and stopCellEditMode.
  4. valueParser gets called with the value in setEditCellValue
  5. valueSetter gets called with the parsed value and then finally processRowUpdate gets called with the updated row.

What should happen:

  1. DataGrid calls valueParser with empty string to "clear" the cell.
  2. DataGrid calls valueSetter with the parsedValue (which will be whatever the colDef decides the default/no value should be typed as)
  3. It opens the cell editor and forwards the key press event.
  4. Generally, the renderer's onChange function is called, which calls startCellEditMode, setEditCellValue, and stopCellEditMode.
  5. valueParser gets called with the value in setEditCellValue
  6. valueSetter gets called with the parsed value and then finally processRowUpdate gets called with the updated row.

Your environment

`npx @mui/envinfo` ``` System: OS: macOS 13.6.4 Binaries: Node: 20.14.0 - ~/.nvm/versions/node/v20.14.0/bin/node npm: 10.7.0 - ~/.nvm/versions/node/v20.14.0/bin/npm pnpm: Not Found Browsers: Chrome: 126.0.6478.116 Edge: Not Found Safari: 16.6 npmPackages: @emotion/react: ^11.11.1 => 11.11.1 @emotion/styled: ^11.8.1 => 11.11.0 @mui/base: 5.0.0-beta.18 @mui/core-downloads-tracker: 5.14.12 @mui/icons-material: ^5.8.0 => 5.14.12 @mui/lab: ^5.0.0-alpha.86 => 5.0.0-alpha.147 @mui/material: ^5.8.1 => 5.14.12 @mui/private-theming: 5.14.12 @mui/styled-engine: 5.14.12 @mui/system: 5.14.12 @mui/types: 7.2.5 @mui/utils: 5.14.12 @mui/x-data-grid: 6.19.10 @mui/x-data-grid-premium: 6.19.10 => 6.19.10 @mui/x-data-grid-pro: 6.19.10 @mui/x-date-pickers: 6.19.9 @mui/x-date-pickers-pro: ^6.19.3 => 6.19.9 @mui/x-license: ^7.0.0-beta.2 => 7.0.0-beta.2 @mui/x-license-pro: ^6.10.2 => 6.10.2 @mui/x-tree-view: 6.0.0-alpha.1 @types/react: ^18.0.6 => 18.2.25 react: ^18.1.0 => 18.2.0 react-dom: ^18.1.0 => 18.2.0 styled-components: 5.3.11 typescript: ^5.1.6 => 5.2.2 ```

Search keywords: valueparser

github-actions[bot] commented 3 days ago

You have created a support request under the "Priority Support" terms, which is a paid add-on to MUI X Premium ⏰. Please validate your support key using the link below:

https://tools-public.mui.com/prod/pages/jyhs86t?repo=mui-x&issueId=13628

Do not share your support key in this issue!

Priority Support is only provided to verified customers. Once you have verified your support key, we will remove the support: unknown label and add the support: priority label to this issue. Only then the time for the SLA will start counting.

michelengelen commented 2 days ago

Hey @louisaaron ... this should only happen in row edit mode. We did adjust this in #12216 for cell editing, but missed this for row editing.

Here is a diff that gets a potential fix started:

diff --git a/packages/x-data-grid/src/hooks/features/editing/useGridRowEditing.ts b/packages/x-data-grid/src/hooks/features/editing/useGridRowEditing.ts
index 33932d806..41c33fceb 100644
--- a/packages/x-data-grid/src/hooks/features/editing/useGridRowEditing.ts
+++ b/packages/x-data-grid/src/hooks/features/editing/useGridRowEditing.ts
@@ -442,7 +442,28 @@ export const useGridRowEditing = (

         let newValue = apiRef.current.getCellValue(id, field);
         if (fieldToFocus === field && (deleteValue || initialValue)) {
-          newValue = deleteValue ? '' : initialValue;
+          if (deleteValue) {
+            const fieldType = apiRef.current.getColumn(field).type;
+            switch (fieldType) {
+              case 'boolean':
+                newValue = false;
+                break;
+              case 'date':
+              case 'dateTime':
+              case 'number':
+                newValue = undefined;
+                break;
+              case 'singleSelect':
+                newValue = null;
+                break;
+              case 'string':
+              default:
+                newValue = '';
+                break;
+            }
+          } else if (initialValue) {
+            newValue = initialValue;
+          }
         }

         acc[field] = {

I'll open this up as a good first issue. 👍🏼