Skip to content
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

[DataGrid] Removal of gridEditRowsStateSelector downgrades usability #11133

Closed
jvfabri opened this issue Nov 21, 2023 · 3 comments · Fixed by #13877
Closed

[DataGrid] Removal of gridEditRowsStateSelector downgrades usability #11133

jvfabri opened this issue Nov 21, 2023 · 3 comments · Fixed by #13877
Assignees
Labels
component: data grid This is the name of the generic UI component, not the React module! dx Related to developers' experience feature: Editing Related to the data grid Editing feature

Comments

@jvfabri
Copy link

jvfabri commented Nov 21, 2023

Steps to reproduce

Migrating from DataGrid's v5 to v6 offers no substitute to gridEditRowsStateSelector which is essential in conditionally validating actions in a more elevated way.

Further details are included in Context section and can be seen in other issues such as #4983.

Current behavior

In v5, we could use the gridEditRowsStateSelector to retrieve current information about the cells being edited in a row, to validate the Save action.

In v6 this selector was removed and migrating involves breaking validation of row editing and conditional render on Actions column.

Expected behavior

Migrating to v6 should offer at least a valid substitute to the removed selector to some degree, in this case by providing a way to conditionally render actions based on current editing state of a row.

Context

As mentioned in this #4983 (comment) comment, I believe removing gridEditRowsStateSelector downgraded usability of DataGrid as there seems to be no clear substitute for functionalities this selector allowed to be developed.

To be more specific, in our current use of DataGrid, we needed a way to disable the save action on row editing conditionally, in such way that invalid inputs on the row cannot be submitted to the API. For v5 we solved this issue (in short) like this:

// the save component
function SaveButton({ id, getErrorFromEditState, ...props }) {
 const apiRef = useGridApiContext()
 const editState = gridEditRowsStateSelector(apiRef.current.state);
  
  const isFormValid = getErrorFromEditState(editState[id])
  
  return (
    <GridActionsCellItem
      disabled={!isFormValid}
      icon={<CheckIcon />}
      color="primary"
      {...rest}
    />
  )
}

...

// getActions in column definition
getActions: ({ id }) => {
  // ...
  return [<SaveButton onClick={handleSaveClick(id)} getErrorFromEditState{handleError} id={id} />]
}

// and for each time we create a new table, we also define the handleError somewhat like this:
function handleError (editRow) {
  if (editRow.description.error) return "descEmpty";
  else if (editRow.start.error) return "startInvalid";
  else if (editRow.end.error) return "endInvalid";
  else return false;
}

//one of the validations is:
preProcessEditCellProps: (params) => {
   const hasError =
      params.props.value == null ||
      params.props.value?.isAfter(
          params.otherFieldsProps.end.value
      );
   return { ...params.props, error: hasError };
},

What we want to achieve is a way to abstract the "custom" datagrid, allowing for the developer to set preProcessEditCellProps for a column and a "handleError" function to the "custom" DataGrid, instead of redefining actions for every use case.

Your environment

npx @mui/envinfo
System:
    OS: Windows 11 10.0.22621
  Binaries:
    Node: 18.16.1 - C:\Program Files\nodejs\node.EXE
    Yarn: Not Found
    npm: 9.8.1 - C:\dev\UI\frontend\node_modules\.bin\npm.CMD
  Browsers:
    Chrome: 119.0.6045.160
    Edge: Chromium (119.0.2151.72)
  npmPackages:
    @emotion/react: ^11.4.1 => 11.10.6
    @emotion/styled: ^11.3.0 => 11.10.6
    @mui/base:  5.0.0-beta.12
    @mui/core-downloads-tracker:  5.14.6
    @mui/icons-material: ^5.14.3 => 5.14.6
    @mui/lab: ^5.0.0-alpha.127 => 5.0.0-alpha.127
    @mui/material: ^5.14.3 => 5.14.6
    @mui/private-theming:  5.14.6
    @mui/styled-engine:  5.14.6
    @mui/system:  5.14.6
    @mui/types:  7.2.4
    @mui/utils:  5.14.6
    @mui/x-data-grid: ^5.17.19 => 5.17.26
    @mui/x-date-pickers: ^5.0.14 => 5.0.20
    @types/react:  18.0.28
    react: ^18.2.0 => 18.2.0
    react-dom: ^18.2.0 => 18.2.0
    typescript:  4.9.5

Search keywords: datagrid edit state

@jvfabri jvfabri added the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Nov 21, 2023
@cherniavskii
Copy link
Member

Hi @jvfabri
Thanks for opening the issue! There's no substitute for it indeed.

I couldn't find much context behind removing the selector, but I think it was done to keep most of the grid state private so we can change it without breaking changes.
In this specific case though, the editing state mostly consists of GridEditCellProps, which we already expose publicly.

I think we can bring the selector back.
@mui/x any objections?

@cherniavskii cherniavskii added component: data grid This is the name of the generic UI component, not the React module! feature: Editing Related to the data grid Editing feature dx Related to developers' experience and removed status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels Nov 22, 2023
@github-project-automation github-project-automation bot moved this to 🆕 Needs refinement in MUI X Data Grid Nov 22, 2023
@cherniavskii cherniavskii moved this from 🆕 Needs refinement to 🔖 Ready in MUI X Data Grid Nov 22, 2023
@jvfabri
Copy link
Author

jvfabri commented Nov 23, 2023

The issue is also that getActions / Action cells do not really have a convenient way to interact with other cells in the row/grid, or if there is, it is not very clear in the docs or even in the code itself. It is indeed much easier to do the same thing in EditCells. Either way, thanks for the super fast reply 😄

Copy link

⚠️ This issue has been closed. If you have a similar problem but not exactly the same, please open a new issue.
Now, if you have additional information related to this issue or things that could help future readers, feel free to leave a comment.

@jvfabri: How did we do? Your experience with our support team matters to us. If you have a moment, please share your thoughts in this short Support Satisfaction survey.

@romgrk romgrk moved this from 🔖 Ready to ✅ Done in MUI X Data Grid Jul 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: data grid This is the name of the generic UI component, not the React module! dx Related to developers' experience feature: Editing Related to the data grid Editing feature
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

3 participants