-
-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
feat(mui): editable Data Grid #5744
Conversation
🦋 Changeset detectedLatest commit: 09f8f2d The changes in this PR will be included in the next version bump. Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Some problems occurred because my fork was not completely synced. Resolved it now. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @beg1c thanks for the PR! Added some comments.
@BatuhanW Added tests and some changes. I'd appreciate it if you could take a look when you have a moment. I plan to update the documentation later since there might be more changes before merging. Additionally, it seems we're lacking functionality where the user can call |
@beg1c appreciate the effort! Looks good initially, I'll review more in detail later. Also cc @aliemir @alicanerdurmaz |
Co-authored-by: Alican Erdurmaz <alicanerdurmaz@gmail.com>
@BatuhanW @alicanerdurmaz updated docs. Do you think we should accept boolean For further usage I don't think we need that prop, but we could break older versions if we don't accept it. If some users have their own implementation of editable datagrid, and they didn't declare So for example if someone defined their data grid as:
which is wrong as I don't know, what do you think? |
I believe you are right. We should accept something like this: useForm({
queryOptions: {
enabled: editable,
},
});
processRowUpdate: async (newRow, oldRow) => {
if(!editable) return
/* .... */
}
|
Hey @beg1c we've merged |
PR Checklist
Please check if your PR fulfills the following requirements:
Bugs / Features
What is the new behavior?
Data Grid can now be edited by specifying
editable
property on DataGrid column definition, just as in MUI Data Grid documentation.useDataGrid
will now utilize existing Data Grid propsprocessRowUpdate
,onRowEditStart
andonCellEditStart
to update resource wheneverprocessRowUpdate
is triggered.fixes #5656 (issue)
Notes for reviewers
This is lightweight implementation of this functionality, using only existing Data Grid props. This functionality can be later extended for handling multiline editing.
processRowUpdate
must return value to Data Grid, that is why we must return oldRow if mutation does not succeed.I've set title column on
examples/table-material-ui-use-data-grid
as editable.