-
-
Notifications
You must be signed in to change notification settings - Fork 1.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
[DataGrid] Fix calling onCellEditStop on error #12747
Conversation
Deploy preview: https://deploy-preview-12747--material-ui-x.netlify.app/ |
const { field, id, value, debounceMs, unstable_skipValueParser: skipValueParser } = args[0]; | ||
const column = apiRef.current.getColumn(field); | ||
const row = apiRef.current.getRow(id)!; | ||
|
||
let parsedValue = value; | ||
if (column.valueParser && !skipValueParser) { | ||
parsedValue = column.valueParser(value, row, column, apiRef); | ||
} | ||
|
||
const editingState = gridEditRowsStateSelector(apiRef.current.state); | ||
let newProps: GridEditCellProps = { | ||
...editingState[id][field], | ||
value: parsedValue, | ||
changeReason: debounceMs ? 'debouncedSetEditCellValue' : 'setEditCellValue', | ||
}; | ||
|
||
if (column.preProcessEditCellProps) { | ||
const hasChanged = value !== editingState[id][field].value; | ||
|
||
newProps = { ...newProps, isProcessingProps: true }; |
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.
This entire logic was copied from
mui-x/packages/x-data-grid/src/hooks/features/editing/useGridCellEditing.ts
Lines 449 to 478 in aed8aeb
const { id, field, value, debounceMs, unstable_skipValueParser: skipValueParser } = params; | |
throwIfNotEditable(id, field); | |
throwIfNotInMode(id, field, GridCellModes.Edit); | |
const column = apiRef.current.getColumn(field); | |
const row = apiRef.current.getRow(id)!; | |
let parsedValue = value; | |
if (column.valueParser && !skipValueParser) { | |
parsedValue = column.valueParser(value, row, column, apiRef); | |
} | |
let editingState = gridEditRowsStateSelector(apiRef.current.state); | |
let newProps: GridEditCellProps = { | |
...editingState[id][field], | |
value: parsedValue, | |
changeReason: debounceMs ? 'debouncedSetEditCellValue' : 'setEditCellValue', | |
}; | |
if (column.preProcessEditCellProps) { | |
const hasChanged = value !== editingState[id][field].value; | |
newProps = { ...newProps, isProcessingProps: true }; | |
updateOrDeleteFieldState(id, field, newProps); | |
newProps = await Promise.resolve( | |
column.preProcessEditCellProps({ id, row, props: newProps, hasChanged }), | |
); | |
} |
Thought of storing { id, row, props: newProps, hasChanged }
in ref, which are computed in below function, but wasn't sure if setCellEditingEditCellValue
runs always and runs before runIfNoPreProcessError
const setCellEditingEditCellValue = React.useCallback< |
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.
Thanks!
Did you consider following the same approach as in #11383
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.
Wasn't aware of that PR. Now I've updated code to use similar approach as in #11383.
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.
Looks good, thanks!
@@ -234,6 +234,19 @@ export const useGridCellEditing = ( | |||
[apiRef], | |||
); | |||
|
|||
const runIfNoFieldErrors = | |||
<Args extends any[]>(callback?: (...args: Args) => void) => |
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.
Nitpick: better type safety
<Args extends any[]>(callback?: (...args: Args) => void) => | |
<Args extends Parameters<GridEventListener<'cellEditStop'>>>(callback?: (...args: Args) => void) => |
…ting.ts Signed-off-by: Andrew Cherniavskii <andrew.cherniavskii@gmail.com>
Head branch was pushed to by a user without write access
Signed-off-by: Andrew Cherniavskii <andrew.cherniavskii@gmail.com> Co-authored-by: Andrew Cherniavskii <andrew.cherniavskii@gmail.com>
Signed-off-by: Andrew Cherniavskii <andrew.cherniavskii@gmail.com> Co-authored-by: Andrew Cherniavskii <andrew.cherniavskii@gmail.com>
closes: #11406