[DataGrid] Do not publish rowEditStop event if row has fields with errors

[cherniavskii]

Fixes #11263

• Before: https://codesandbox.io/p/sandbox/inspiring-dijkstra-g2h52p?file=%2Fsrc%2Fdemo.tsx%3A123%2C25-123%2C43
• After: https://codesandbox.io/p/sandbox/gracious-babycat-8y68c9?file=%2Fsrc%2Fdemo.tsx%3A123%2C25-123%2C43

TODO:

• Add unit tests

[mui-bot]

Deploy preview: https://deploy-preview-11383--material-ui-x.netlify.app/

Generated by 🚫 dangerJS against 863db74

[lauri865]

Does this prevent processRowUpdate from being called as well? If so, it'll break a lot of things in our custom validator implementation that shows validation errors also in viewing mode, and I'm not sure if there's a good alternative to that.

[michelengelen]

super solid! LGTM!

[michelengelen]

@lauri865 this will only prevent the rowEditStop event to be fired, when there are fields with errors present. Correct @cherniavskii ?

[cherniavskii]

@lauri865 processRowUpdate is not being called if the row has fields in error state - see https://codesandbox.io/p/sandbox/nostalgic-paper-wyyv28?file=%2Fsrc%2FDemo.tsx%3A19%2C21

If you're not using preProcessEditCellProps and only rely on processRowUpdate for validation - it should be OK.
The server-side validation demo works as before: https://deploy-preview-11383--material-ui-x.netlify.app/x/react-data-grid/editing/#server-side-validation

[cherniavskii]

Let's wait a bit for @lauri865's feedback on this to make sure we're not breaking anything 🙂
@lauri865 Could you confirm my points in #11383 (comment)?

[lauri865]

You are correct.

We are using preProcessEditCellProps, but have sidestepped using the error prop and replaced it with custom props like errorMessage to make sure processRowUpdate gets called regardless of whether there is an error or not. So, thanks to that it will not break things for us luckily.

The problem is that both onCellEditStop and onRowEditStop don't include the latest value, and the only way we've found to be able to show validation errors in view mode (see example below) is through the above workaround by side-stepping the error prop and getting datagrid to call processRowUpdate regardless. Just rejecting invalid inputs, which is the default behaviour, is often too restricting of an UX. Imagine a use case where the user uploads a CSV with 25 rows, and there's missing data you want to highlight (validation needs to be shown outside editing mode), and maybe erroneous rows that the user will want to skip altogether (not get stuck in edit mode every time you enter an invalid field). That requires validation to be less blocking, more informative. Just wish there was an easier way to achieve this behaviour by default, but we have managed to find a way that works for us for now at least, albeit I realise it may not be the most future-proof way.

Example of showcasing a tooltip with validation errors in view mode:

[cherniavskii]

@lauri865

So, thanks to that it will not break things for us luckily.

Great, I'll push this PR forward then!

The problem is that both onCellEditStop and onRowEditStop don't include the latest value

Could you open a new issue for this discussion? A minimal reproduction example would be great!

[github-actions]

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

[github-actions]

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