Delete Grid component #1785
Delete Grid component #1785
Conversation
| import { GridProps } from './Grid'; | ||
| import { CalculatedColumn, Position, ScrollPosition, SubRowDetails, RowRenderer, RowRendererProps, RowData, ColumnMetrics, CellMetaData, InteractionMasksMetaData } from './common/types'; | ||
| import { ReactDataGridProps } from './ReactDataGrid'; | ||
| import EventBus from './EventBus'; |
There was a problem hiding this comment.
Just a general discussion regarding this EventBus, looks like it can be replaced by Context now? After several layers being removed, we directly render the Canvas in RDG.tsx in this PR, it seems we start to not need the eventBus anymore?
There was a problem hiding this comment.
How would you use a context to trigger scrollToColumn?
There was a problem hiding this comment.
For scrollToColumn case, can't we move it one level up? Then we can move canvas ref in scrollToColumn in RDG level like this.
function scrollToColumn(idx: number, columns: CalculatedColumn<R>[]) {
const { current } = canvas.current.ref;
...
}This evenBus subscribe and unsubscribe makes me feel like we are going back and forth to avoid passing functions as props. With the layers being removed, maybe the eventBus can be removed gradually, or did I miss something?
There was a problem hiding this comment.
The idea behind event bus was to avoid rendering all the cells when a cell is clicked or selection is changed. Previously {rowidx, cellIx } was being passed to each cell and it resulted in a really slow cell navigation. That was the whole idea behind adding interaction mask
We can delete InteractionMask in the future now that we have improved rendering so much. I am sure that will make grid much simpler now
| import { GridProps } from './Grid'; | ||
| import { CalculatedColumn, Position, ScrollPosition, SubRowDetails, RowRenderer, RowRendererProps, RowData, ColumnMetrics, CellMetaData, InteractionMasksMetaData } from './common/types'; | ||
| import { ReactDataGridProps } from './ReactDataGrid'; | ||
| import EventBus from './EventBus'; |
There was a problem hiding this comment.
The idea behind event bus was to avoid rendering all the cells when a cell is clicked or selection is changed. Previously {rowidx, cellIx } was being passed to each cell and it resulted in a really slow cell navigation. That was the whole idea behind adding interaction mask
We can delete InteractionMask in the future now that we have improved rendering so much. I am sure that will make grid much simpler now
Gridis an unnecessary component layer, so I've moved its stuff into the root component.