Conversation
| } | ||
|
|
||
| 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. |
| 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? |
github-actions
bot
added
the
PR: out-of-date
3 participants
Closes #3524