-
Notifications
You must be signed in to change notification settings - Fork 291
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
Reduce dom elements - Approach #3 #682
base: column-virtualization
Are you sure you want to change the base?
Conversation
</div> | ||
); | ||
} | ||
export default CellGroup; |
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.
This is the CellGroup Function which is the non-legacy component
</div> | ||
); | ||
} | ||
export default CellGroupLegacy; |
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.
This is the legacy component .Only the difference between legacy and non-legacy is of styling and rendering of the divs
/> | ||
); | ||
} | ||
} | ||
//} |
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.
We have to pass shouldUseLegacyComponents to wherever needed
const CellGroupComponent = this.props.shouldUseLegacyComponents | ||
? CellGroupLegacy | ||
: CellGroup; | ||
|
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.
Here we will check that either I have to use the legacy components or the new ones
{...props} | ||
_initialRender={this._initialRender} | ||
sortedCells={sortedCells} | ||
/> | ||
); |
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.
Then we have to render CellGroupComponent
'public/fixedDataTableRow/highlighted': this.props.index % 2 === 1, | ||
'public/fixedDataTableRow/odd': this.props.index % 2 === 1, | ||
'public/fixedDataTableRow/even': this.props.index % 2 === 0, | ||
}); | ||
var fixedColumnsWidth = this.props.fixedColumnsWidth; |
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.
We will be adding all this styling and className in the function call (either of legacy or non-legacy)
rowExpanded={rowExpanded} | ||
rowExpandedStyle={rowExpandedStyle} | ||
columnsRightShadow={columnsRightShadow} | ||
/> |
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.
We have to pass all the functions also as props to the RowComponent
Choice on user
Description
In this PR we have given the choice to user that whether he wants to use the old code of column-virtualization or the new code which is of our div-removal branch (after removing div) .We have used higher Order components to maintain the reusability of code .We have passed one more prop named as shouldUseLegacyComponents (boolean) .On the basis of this prop we will decide which function we have to render either the legacy one or the new one .The main difference of code is in row and the cellGroup components so we have seperated the uncommon part into different functions (HOC) and then decided on the basis of if condition that which part we have to render .
Motivation and Context
To provide the functionality to user that whether he wants to use the old code or the new code (backward compatibilty)
How Has This Been Tested?
On the existing examples and then also checked the performance
Screenshots (if appropriate):
Types of changes
Checklist: