-
-
Couldn't load subscription status.
- Fork 1.7k
[DataGrid] New editing API #3963
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
Conversation
|
These are the results for the performance tests:
|
|
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
2b6764a to
2e7f536
Compare
|
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
114e9e0 to
1b82f63
Compare
packages/grid/x-data-grid-pro/src/tests/detailPanel.DataGridPro.test.tsx
Outdated
Show resolved
Hide resolved
| const isValid = await api.setEditCellValue({ id, field, value: event.target.value }, event); | ||
|
|
||
| if (rootProps.experimentalFeatures?.enableNewEditingAPI) { | ||
| return; |
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.
The edit cell in the current singleSelect column automatically commits the value after selecting an option. This doesn't work well with preProcessEditCellProps because if doing a AJAX validation it freeze the UI while waiting for the server to respond. Now we don't automatically commit the value. The user has to click outside as in any other column.
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.
If preProcessEditCellProps is not being used and/or the desired UX is that once a user has made a MenuItem selection the singleSelect will automatically commit, is there a prescribed method for doing so? Or, is there a workaround to make that happen? Thank you.
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.
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.
Based on the auto-stop example, my understanding is this cannot be done with the singleSelect column type but rather must be accomplished with a custom column type. While the custom column type may be a Select, it would not be a singleSelect. Thank you.
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.
I did not test the behavior yet
Overall, the new API is a lot easier to read.
packages/grid/x-data-grid/src/internals/models/api/gridEditingApi.ts
Outdated
Show resolved
Hide resolved
packages/grid/x-data-grid/src/internals/models/params/gridCellParams.ts
Outdated
Show resolved
Hide resolved
|
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
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.
Nothing to add
|
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
| */ | ||
| experimentalFeatures?: Partial<GridExperimentalFeatures>; | ||
| /** | ||
| * Callback called before updating a row with new values in the row and cell editing. |
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.
Users not looking at the doc but only at there IDE autocomplete will probably use it without thinking to enable the flag.
| * Callback called before updating a row with new values in the row and cell editing. | |
| * Callback called before updating a row with new values in the row and cell editing. | |
| * Only applied if `props.experimentalFeatures.newEditingApi: true` |
This PR proposes a new API for cell and row editing. To avoid breaking changes, it's only available if
experimentalFeatures.enableNewEditingAPIistrue. Since we switch hooks, developers can't change which API to use in runtime.After working a lot with the current API it seems to me that during development the listeners were created before the API methods. This caused weird things like publishing events and also calling other API methods.
mui-x/packages/grid/x-data-grid/src/internals/hooks/features/editRows/useGridCellEditing.ts
Lines 228 to 230 in 3101d8d
But these published events also call API methods. Their only purpose is to allow developers to override the default behavior.
mui-x/packages/grid/x-data-grid/src/internals/hooks/features/editRows/useGridCellEditing.ts
Lines 299 to 301 in 3101d8d
This was making the editing hooks a mess to maintain. To fix these problems I took a different direction here. First I created the API methods, then binded them to events. Generic listeners, like
cellKeyDown, are dumb now and they will only be used to publish a specific editing event, likecellEditStart. The specific event listeners will also be dumb, only calling a method from the API with the correct params. All the complex logic will be inside the API method.Methods for the cell editing API
isCellEditabledetermines if a cell is editablegetCellModereturns if a cell is in edit or view modestartCellEditModeputs a cell into edit modestopCellEditModeputs a cell into view mode and updates the original row with the value storedsetEditCellValueupdates the value of a cellMethods for the row editing API
getRowModereturns if a cell is in edit or view modestartRowEditModeputs row into edit modestopRowEditModeputs a row into view mode and updates the original row with the values storedsetEditCellValueupdates the value of a cellNew props
processRowUpdate(name open for discussion) is called when putting a cell/row back to view mode. It's meant to be used to send the new row to the server. Since it's called BEFOREupdateRowsit allows developers to also pre-process the row that will be saved. If the promise is rejected, then we don't update the row and the row doesn't goes to view mode. Previously, to send the row to the server we taught developers to use therowEditStopevent but this had a problem that it updated the row even when the server threw an error, it was optimistic. Now we use a pessimistic approach, which IHMO is what users want for a CRUD application. See https://marmelab.com/react-admin/CreateEdit.html#undoableMethods to be removed in v6
setRowModestartRowEditModeandstopRowEditModesetCellModestartCellEditModeandstopCellEditModecommitRowChangestopRowEditModecommitCellChangestopCellEditModeDemo with server-side validation: https://codesandbox.io/s/validateservernamegrid-material-demo-forked-5iflei?file=/demo.tsx
Demo with server-side persistence: https://codesandbox.io/s/celleditserversidepersistence-material-demo-forked-7xcpgg?file=/demo.tsx (it doesn't save if the name is empty)
Demo with row editing, validation and with the default start/stop editing behaviors disabled https://codesandbox.io/s/starteditbuttongrid-material-demo-forked-y84q5e?file=/demo.tsx
The docs will be done in a follow-up PR.