Remove legacy grouping props #2012
Conversation
src/RowRenderer.tsx
Outdated
| }: IRowRendererProps<R, SR>) { | ||
| const { __metaData } = row as RowData; | ||
| onRowClick | ||
| }: RowRendererProps<R, SR> & SharedDataGridProps<R, SR>) { |
There was a problem hiding this comment.
I wonder if we should use memo on the Row component and delete RowRenderer. Thoughts?
There was a problem hiding this comment.
We still want to have the ability to customize the rowrenderer, right? I believe that's the reason why we kept this layer.
There was a problem hiding this comment.
Let's just delete this module, memoize Row, and use the Row/rowRenderer directly.
Custom row renderers can memo themselves.
src/RowRenderer.tsx
Outdated
| eventBus: EventBus; | ||
| isRowSelected: boolean; | ||
| } | ||
| type IRowRendererProps<R, SR> = RowRendererProps<R, SR> & SharedDataGridProps<R, SR>; |
There was a problem hiding this comment.
Do we still need this? Or we should use it at ln#23
stories/demos/LegacyGrouping.tsx
Outdated
|
|
||
| function groupByColumn(rows: Row[], columnKeys: string[], expandedGroups: Set<string>, treeDepth = 0, parentKey = '') { | ||
| if (columnKeys.length === 0) return rows; | ||
| const gridRows: any = []; |
There was a problem hiding this comment.
Is it due to the expanded data so that we cannot put Row[] here?
src/RowRenderer.tsx
Outdated
| }: IRowRendererProps<R, SR>) { | ||
| const { __metaData } = row as RowData; | ||
| onRowClick | ||
| }: RowRendererProps<R, SR> & SharedDataGridProps<R, SR>) { |
There was a problem hiding this comment.
We still want to have the ability to customize the rowrenderer, right? I believe that's the reason why we kept this layer.
| height: var(--row-height); | ||
| line-height: var(--row-height); | ||
| padding: 0 8px; | ||
| border-bottom: 1px solid #ddd; |
There was a problem hiding this comment.
border-bottom seems like a common variable, I'm wondering if we want to create a css variable for this.
src/RowRenderer.tsx
Outdated
| }: IRowRendererProps<R, SR>) { | ||
| const { __metaData } = row as RowData; | ||
| onRowClick | ||
| }: RowRendererProps<R, SR> & SharedDataGridProps<R, SR>) { |
There was a problem hiding this comment.
Let's just delete this module, memoize Row, and use the Row/rowRenderer directly.
Custom row renderers can memo themselves.
| const rowGroupHeader = { | ||
| groupKey, | ||
| name: key, | ||
| __metaData: { |
There was a problem hiding this comment.
I think it'd be better to refactor this to a more realistic data structure, maybe something like
interface RowGroup {
type: 'group';
// ...
}
interface RowData {
type: 'data';
// ...
}
type Row = RowGroup | RowData;Not worth our time right now though.
There was a problem hiding this comment.
Simplified types a bit
Cell selection on the grouped row is off on the grouped row but that is how legacy worked so I am not changing it. Let's revisit when we add the new grouping API