Skip to content

[DataGrid] Fix cell editing of computed columns with data source #17684

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
May 6, 2025

Conversation

ptuukkan
Copy link
Contributor

@ptuukkan ptuukkan commented May 5, 2025

I noticed that if there is a column which does not have a respective property in row data and using dataSource and cell editing, dataSource.editRow does not fire. This can happen with computed columns, for example.

This is due to the code checking if the row[field] has changed. In this case it would be always undefined and the updateStateToStopCellEditMode would return early without calling dataSource.editRow. useGridRowEditing uses isDeepEqual instead to check if something in the row has been changed. I thought it might be reasonable to use it in useGridCellEditing as well. Perhaps it wouldn't even be necessary to do that checking, but I will leave that decision to you.

Here is a sandbox inspired by the following demo: https://mui.com/x/react-data-grid/editing/#value-parser-and-value-setter
https://stackblitz.com/edit/tswwgcr5-5zr3q5b1?file=src%2FDemo.tsx

Editing the Computed column causes no updates to the grid nor it fires the dataSource.updateRow.

Here is the same sandbox with this PR's changes:
https://stackblitz.com/edit/uouznmzp?file=src%2FDemo.tsx

Copy link

github-actions bot commented May 5, 2025

Thanks for adding a type label to the PR! 👍

@mui-bot
Copy link

mui-bot commented May 5, 2025

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

Generated by 🚫 dangerJS against ff42466

@MBilalShafi MBilalShafi self-requested a review May 5, 2025 11:54
@MBilalShafi MBilalShafi added type: bug A bug or unintended behavior in the components. scope: data grid Changes or issues related to the data grid product feature: Editing Related to the data grid Editing feature feature: Server integration Better integration with backends, e.g. data source labels May 5, 2025
Copy link
Member

@MBilalShafi MBilalShafi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you, @ptuukkan, for the PR. The change makes sense to me. It could indeed be possible to sync computed columns on the server. I added a test to make sure it keeps working as expected.

Perhaps it wouldn't even be necessary to do that checking

I would keep it to eliminate false positive runs of editRow, which would mean unnecessary API calls and cache resets.

@arminmeh I'd love another pair of eyes, though feel free to ignore the review request if you wish.

@MBilalShafi MBilalShafi requested a review from arminmeh May 6, 2025 05:55
@arminmeh arminmeh enabled auto-merge (squash) May 6, 2025 10:50
@arminmeh arminmeh merged commit cbda8b5 into mui:master May 6, 2025
19 checks passed
@ptuukkan ptuukkan deleted the data-source-computed-field-edit branch May 6, 2025 11:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature: Editing Related to the data grid Editing feature feature: Server integration Better integration with backends, e.g. data source scope: data grid Changes or issues related to the data grid product type: bug A bug or unintended behavior in the components.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants