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/
4.53k stars 1.32k forks source link

[data grid] clipboard paste validation #11874

Open sd-danhill opened 9 months ago

sd-danhill commented 9 months ago

The problem in depth

We use preProcessEditCellProps on the column for validation as suggested by the docs, however with clipboard paste, preProcessEditCellProps isn't called

We're using processRowUpdate which gets correctly called on every update

My question is, should we be doing validation in both preProcessEditCellProps and processRowUpdate? It seems if you're editing the cell manually then it would need to validate twice. Given processRowUpdate passes the entire new/old row, we'd need to determine what's changed and then validate on that

Would be great to get some feedback on how to best approach validating clipboard paste values.

Thanks

Your environment

`npx @mui/envinfo` ``` Using Chrome System: OS: macOS 14.1.1 Binaries: Node: 20.7.0 - ~/.nvm/versions/node/v20.7.0/bin/node npm: 10.1.0 - ~/.nvm/versions/node/v20.7.0/bin/npm pnpm: 7.24.2 - ~/Library/pnpm/pnpm Browsers: Chrome: 121.0.6167.85 Edge: Not Found Safari: 17.1 npmPackages: @emotion/react: ^11.10.4 => 11.10.4 @emotion/styled: ^11.10.4 => 11.10.4 @mui/base: 5.0.0-beta.6 @mui/core-downloads-tracker: 5.15.6 @mui/icons-material: ^5.10.6 => 5.10.6 @mui/lab: ^5.0.0-alpha.135 => 5.0.0-alpha.135 @mui/material: ^5.14.19 => 5.15.6 @mui/private-theming: 5.15.6 @mui/styled-engine: 5.15.6 @mui/system: 5.15.6 @mui/types: 7.2.13 @mui/utils: 5.15.6 @mui/x-data-grid: 6.19.2 @mui/x-data-grid-premium: ^6.19.2 => 6.19.2 @mui/x-data-grid-pro: 6.19.2 @mui/x-date-pickers: ^6.11.1 => 6.11.1 @mui/x-license-pro: ^6.10.2 => 6.10.2 @types/react: ^18.2.0 => 18.2.0 react: ^18.2.0 => 18.2.0 react-dom: ^18.2.0 => 18.2.0 typescript: ^4.8.4 => 4.8.4 ```

Search keywords: clipboard paste validation Order ID: 78997

michelengelen commented 9 months ago

@romgrk not sure about this one. I know we have a few events that get fired during pasting, but are those suitable for cell processing?

romgrk commented 9 months ago

I haven't touched the clipboard pasting code yet, maybe @cherniavskii can answer better which of those options is best?

cherniavskii commented 9 months ago

Hi @sd-danhill Could you share more details about your use case? What would you expect to happen if validation fails for a pasted value?

sd-danhill commented 9 months ago

@cherniavskii Different columns with different formats, e.g. currency, times, text values, numbers. At the moment with single cell editing we block the user from exiting edit mode should it not pass validation, e.g. time not in HH:MM format.

On what I'd expect to happen, a solution could be that every cell gets a chance to validate it's new value (runs preProcessEditCellProps on the column) and if all successful only then commits the changes. On failure it could use a callback onClipboardPasteValidationFailure which could give us the opportunity to show an error toast?

I guess the solution depends on the paradigm: Should the user be able to enter bad data and fix it, or flat out reject bad data from being entered

If I allow users to enter bad data (and then for example block the user saving) rather than rejecting it, I'd still need a mechanism to validate the cell on change

cherniavskii commented 9 months ago

@sd-danhill

Thanks for your feedback!

The reason preProcessEditCellProps is not being called on clipboard paste is that cells never enter editing mode. But I'm considering calling it for every pasted cell, even though technically they're not in the editing mode.

As for what happens when preProcessEditCellProps fails on at least one cell – it makes sense to reject the whole paste operation. What kind of validations do you use in preProcessEditCellProps? Do you use async validation?

sd-danhill commented 9 months ago

@cherniavskii

Agreed on rejecting the whole paste. That's the workaround I've put in place right now within processRowUpdate. I perform a diff between old/new row, check what fields have changed, find the column and run preProcessEditCellProps on each field. On success return new row, on failure throw an error w/ a toast message.

In terms of what validations we currently run, it's mainly pattern matching w/ regex. HH:mm for timings, currency validation, date of birth. Nothing async

michelengelen commented 9 months ago

I have added it to the board to investigate the proposal from @cherniavskii

sd-danhill commented 8 months ago

To add another issue - isCellEditable isn't called either, allowing values to be pasted into uneditable cells

anilcngz commented 2 months ago

@cherniavskii

While editing one by one as below, I can validate each cell by using preProcessEditCellProps which has async validations for phone number etc.

manual-editing

But when copy paste data, as you mentioned earlier preProcessEditCellProps is not being called because the rows are not in editing mode.

Screenshot 2024-09-14 at 00 53 16

I tried to use onBeforeClipboardPasteStart event to set rows mode to edit, but then the paste is not working as expected, it pastes all data into selected cell

before-paste-set-edit-mode

Do you have any suggestion for my case?