Fix/default editable grid #3526
Conversation
| @@ -479,11 +479,11 @@ function getNarrowedColType(type?: string): GridColType | undefined { | |||
| return (type && type in DEFAULT_COLUMN_TYPES ? type : undefined) as GridColType | undefined; | |||
| } | |||
|
|
|||
| export function parseColumns(columns: SerializableGridColumns): GridColDef[] { | |||
| export function parseColumns(columns: SerializableGridColumns, isEditable?: boolean): GridColDef[] { | |||
There was a problem hiding this comment.
parseColumns acts as a conversion between "serializable columns" and "X grid columns". It's being reused across the codebase for this purpose. I prefer to keep this function single purpose.
How do you feel about solving this downstream instead? Where we iterate a second time to create the rendered columns:
https://github.com/mui/mui-toolpad/blob/02cc9a97b76098d51651eb48cf10c12ccdc7c8fe/packages/toolpad-studio-components/src/DataGrid.tsx#L1264-L1279
There was a problem hiding this comment.
@Janpot , say we're having id column it shouldn't be editable at all right, should I move the isIdColumn logic to this function?
If we update the editable here id column may probably get override right?
There was a problem hiding this comment.
How about we only set isEditable if it isn't already defined? And remove isEditable: true from the baseColumn in parseColumns.
There was a problem hiding this comment.
Yea, that's a good approach. Will update accordingly.
b4s36t4
force-pushed
the
fix/default-editable-grid
branch
from
2797c76
to
825b970
Compare
|
@Janpot I have updated the changes you suggested, can you please verify. |
Janpot
added
the
component: data grid
| return column; | ||
| } | ||
|
|
||
| column.editable = isRowUpdateModelAvailable; |
There was a problem hiding this comment.
I think we should avoid mutating items in .map. Perhaps something like the following would work?
return column.editable === undefined ? { ...column, editable: isRowUpdateModelAvailable } : column;
Can you look into why the tests fail? |
Closes #3524