Reduce dom elements - Approach #2 #681
Conversation
| shouldComponentUpdate(nextProps) { | ||
| // we need to render the cell to hide/show it |
There was a problem hiding this comment.
This is the shouldComponentUpdate react LifeCycleMethod .It will tell that when the class component should re-render .In this case the cell component should re-render whenever our scrollOffsetLeft, rowIndex, columnIndex, visibility or any other cell props get changed .
| @@ -193,57 +192,39 @@ class FixedDataTableCell extends React.Component { | |||
| isHeaderOrFooter, | |||
| visible, | |||
| zIndex, | |||
| left1, | |||
| // position, | |||
| scrollOffsetLeft, | |||
There was a problem hiding this comment.
scrollOffsetLeft tells us that by how much amount we have scrolled it .It will be useful for fixing the position of a particular cell
| width, | ||
| zIndex, | ||
| visibility: visible ? 'visible' : 'hidden', | ||
| }; | ||
|
|
There was a problem hiding this comment.
The variable which differentiate the different types of Cell groups (fixed,scrollable,fixed-right) is zIndex .It will be set to 2 for fixed and set to 0 for scrollable .Earlier we were providing styling to the whole cell group so we were using zIndex in FixedDataTableCellGroup .But now to maintain those styles we have passed zIndex to each and every cell to provide styles to it .
| this.props.initialRender, | ||
| this.props.isRTL | ||
| ); | ||
|
|
There was a problem hiding this comment.
This function is used for fixing the position of the particular sell .OffsetLeft is the position of a particular cell group from the left .After fixing the position of a cell here we will add left (distance respective to a particular cell group) to it .
src/FixedDataTableCellGroup.js
Outdated
| } else { | ||
| style.left = props.offsetLeft; | ||
| } | ||
|
|
There was a problem hiding this comment.
As we have provided all the styling to a particular cell .So, now there is no need to provide styles to whole cell group .
| {/* </> */} | ||
| </div> | ||
| ); | ||
| return <>{sortedCells}</>; |
There was a problem hiding this comment.
We can remove this div now .I have also included this class name to the cell for maintaining the CSS associated with it
| // var Archit=( | ||
| // < | ||
| // ) | ||
|
|
||
| var fixedColumns = ( | ||
| <FixedDataTableCellGroup |
There was a problem hiding this comment.
Can we remove this FixedDataTableCellGroup component at all ?, I think we may directly render cell from row.
Please investigate this as well and see if it may lead to any performance improvement.
Bonus, if we could also remove FixedDataTableCell and just directly render cells supplied by client.
src/FixedDataTableRow.js
Outdated
| @@ -216,26 +216,6 @@ class FixedDataTableRow extends React.Component { | |||
| ); | |||
| } | |||
|
|
|||
| // shouldComponentUpdate(nextProps) { | |||
There was a problem hiding this comment.
Why you removed this shouldComponentUpdate ?
There was a problem hiding this comment.
When I combined FixedDataTableRow and FixedDataTableRowImpl at that time I have to also take care of this shouldComponentUpdate .So I updated it and removed the old shouldComponentUpdate .
|
Could you change base branch to column-virtualization |
pradeepnschrodinger
changed the title
Remove Cell Group div
Description
Earlier after div removal branch the structure of table to render any text in cell was table->row->CellGroup->Cell->Text where CellGroup will be either fixed, scrollable or fixed-right .In this Final div removal PR I changed the table structure to be Table->row->Cell->Text and remove one more div which was used for the styling of cell group .I provided props such as zIndex, scrollOffsetLeft (How much you have scrolled), offsetLeft and left to FixedDataTableCell so that I can directly style the cell and place it to the correct position .
Motivation and Context
For optimising the performance of fixed-data-table-2
How Has This Been Tested?
Tested using our local examples and performance dev tools
Screenshots (if appropriate):
Types of changes
Checklist: