-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
[DataGridPro] Add new option hideDescendantCount to Tree Data #3368
[DataGridPro] Add new option hideDescendantCount to Tree Data #3368
Conversation
a27978c
to
76df4bb
Compare
@@ -1,5 +1,4 @@ | |||
/// <reference types="next" /> | |||
/// <reference types="next/types/global" /> |
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.
Do you know what caused this modification?
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.
Probably this one
I checked the NextJS example and they have this line
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.
https://github.com/vercel/next.js/pull/28316/files
I think I found the PR
|
||
const commonProperties: Omit<GridColDef, 'field' | 'editable'> = { | ||
...GRID_TREE_DATA_GROUP_COL_DEF, | ||
renderCell: (params) => ( |
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.
Why do you not hideDescendantCount
to the colDef
and use this parameter in the <GridTreeDataGroupingCell />
component like colDef.valueOptions
for <GridEditSingleSelectCell />
?
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 wanted to avoid adding properties on the colDef
that are only useful for the grouping column.
But if the team feels like it's better to pass it to the colDef
then I just have to move hideDescendantCount
from GridGroupingColDefOverride
to GridColDef
and access it through the colDef
prop in the cell component.
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.
Make sense, but when defining groupingColDef
developers will probably expect their props to be in the colDef
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.
For column grouping I have other properties only usefull for the grouping column.
Putting everything on the colDef is not great I think.
I agree with you on the doc. The component API should be enough |
Extracted from #3277
Will be properly documented and tested in the docs of the grouping columns, I don't think it's worth creating an example for the Tree Data.