-
Notifications
You must be signed in to change notification settings - Fork 290
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 #1 #680
base: column-virtualization
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -15,13 +15,13 @@ | |
import React from 'react'; | ||
import PropTypes from 'prop-types'; | ||
|
||
import cx from './vendor_upstream/stubs/cx'; | ||
import FixedDataTableCell from './FixedDataTableCell'; | ||
import FixedDataTableTranslateDOMPosition from './FixedDataTableTranslateDOMPosition'; | ||
import _ from 'lodash'; | ||
import inRange from 'lodash/inRange'; | ||
import cx from './vendor_upstream/stubs/cx'; | ||
|
||
class FixedDataTableCellGroupImpl extends React.Component { | ||
class FixedDataTableCellGroup extends React.Component { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We will pass all the props from FixedDataTableRow to FixedDataTableCellGroup .So, I changed the name of FixedDataTableCellGroupImpl to FixedDataTableCellGroup |
||
/** | ||
* PropTypes are disabled in this component, because having them on slows | ||
* down the FixedDataTable hugely in DEV mode. You can enable them back for | ||
|
@@ -59,6 +59,8 @@ class FixedDataTableCellGroupImpl extends React.Component { | |
|
||
isHeaderOrFooter: PropTypes.bool, | ||
|
||
offsetLeft: PropTypes.number, | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. To remove the wrapper div of CellGroup, we have to place the groups (fixed,scrollable,fixedright) according to the offsetLeft. Earlier we were seperating offsetLeft in below FixedDataTableCellGroup class component and adding one div to position the groups there . |
||
isRTL: PropTypes.bool, | ||
|
||
/** | ||
|
@@ -114,6 +116,20 @@ class FixedDataTableCellGroupImpl extends React.Component { | |
componentDidMount() { | ||
this._initialRender = false; | ||
} | ||
shouldComponentUpdate(/*object*/ nextProps) /*boolean*/ { | ||
/// if offsets haven't changed for the same cell group while scrolling, then skip update | ||
return !( | ||
nextProps.isScrolling && | ||
this.props.rowIndex === nextProps.rowIndex && | ||
this.props.left === nextProps.left && | ||
this.props.offsetLeft === nextProps.offsetLeft | ||
); | ||
} | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This LifeCycleMethod of React will check if there is changes in any of the following props than it will re-render the class component . |
||
static defaultProps = /*object*/ { | ||
left: 0, | ||
offsetLeft: 0, | ||
}; | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. To place any particular cell we need these two thing one is left and the other one is offsetLeft . The left will be relative to the cell group and offsetLeft will be the position of that group from the left . |
||
render() /*object*/ { | ||
var props = this.props; | ||
|
@@ -148,11 +164,12 @@ class FixedDataTableCellGroupImpl extends React.Component { | |
} | ||
|
||
var style = { | ||
height: props.height, | ||
height: props.cellGroupWrapperHeight || props.height, | ||
position: 'absolute', | ||
width: props.contentWidth, | ||
zIndex: props.zIndex, | ||
}; | ||
|
||
FixedDataTableTranslateDOMPosition( | ||
style, | ||
-1 * props.left, | ||
|
@@ -161,6 +178,12 @@ class FixedDataTableCellGroupImpl extends React.Component { | |
this.props.isRTL | ||
); | ||
|
||
if (this.props.isRTL) { | ||
style.right = props.offsetLeft; | ||
} else { | ||
style.left = props.offsetLeft; | ||
} | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. FixedDataTableTranslateDOMPosition will fix the cell group corresponding to how much we have scrolled .props.left is the amount by which we have scrolled .Than to fix this group from the left or right(in case of isRTL) we have to give an additional offsetLeft to the cell group. |
||
// NOTE (pradeep): Sort the cells by column index so that they appear with the right order in the DOM (see #221) | ||
const sortedCells = _.sortBy(this._staticCells, (cell) => | ||
_.get(cell, 'props.columnIndex', Infinity) | ||
|
@@ -183,10 +206,12 @@ class FixedDataTableCellGroupImpl extends React.Component { | |
this.props.endViewportColumnIndex | ||
); | ||
const columnProps = this.props.columns[columnIndex].props; | ||
|
||
const cellTemplate = | ||
this.props.columns[columnIndex].templates[this.props.template]; | ||
|
||
var className = columnProps.cellClassName; | ||
|
||
var pureRendering = columnProps.pureRendering || false; | ||
|
||
const onColumnReorderEndCallback = columnProps.isReorderable | ||
|
@@ -207,7 +232,6 @@ class FixedDataTableCellGroupImpl extends React.Component { | |
className={className} | ||
height={this.props.rowHeight} | ||
key={key} | ||
columnIndex={columnIndex} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. columnIndex was already mentioned at top |
||
maxWidth={columnProps.maxWidth} | ||
minWidth={columnProps.minWidth} | ||
touchEnabled={this.props.touchEnabled} | ||
|
@@ -227,66 +251,4 @@ class FixedDataTableCellGroupImpl extends React.Component { | |
}; | ||
} | ||
|
||
class FixedDataTableCellGroup extends React.Component { | ||
/** | ||
* PropTypes are disabled in this component, because having them on slows | ||
* down the FixedDataTable hugely in DEV mode. You can enable them back for | ||
* development, but please don't commit this component with enabled propTypes. | ||
*/ | ||
static propTypes_DISABLED_FOR_PERFORMANCE = { | ||
isScrolling: PropTypes.bool, | ||
/** | ||
* Height of the row. | ||
*/ | ||
height: PropTypes.number.isRequired, | ||
|
||
offsetLeft: PropTypes.number, | ||
|
||
left: PropTypes.number, | ||
/** | ||
* Z-index on which the row will be displayed. Used e.g. for keeping | ||
* header and footer in front of other rows. | ||
*/ | ||
zIndex: PropTypes.number.isRequired, | ||
}; | ||
|
||
shouldComponentUpdate(/*object*/ nextProps) /*boolean*/ { | ||
/// if offsets haven't changed for the same cell group while scrolling, then skip update | ||
return !( | ||
nextProps.isScrolling && | ||
this.props.rowIndex === nextProps.rowIndex && | ||
this.props.left === nextProps.left && | ||
this.props.offsetLeft === nextProps.offsetLeft | ||
); | ||
} | ||
|
||
static defaultProps = /*object*/ { | ||
left: 0, | ||
offsetLeft: 0, | ||
}; | ||
|
||
render() /*object*/ { | ||
var { offsetLeft, ...props } = this.props; | ||
|
||
var style = { | ||
height: props.cellGroupWrapperHeight || props.height, | ||
width: props.width, | ||
}; | ||
|
||
if (this.props.isRTL) { | ||
style.right = offsetLeft; | ||
} else { | ||
style.left = offsetLeft; | ||
} | ||
|
||
return ( | ||
<div | ||
style={style} | ||
className={cx('fixedDataTableCellGroupLayout/cellGroupWrapper')} | ||
> | ||
<FixedDataTableCellGroupImpl {...props} /> | ||
</div> | ||
); | ||
} | ||
} | ||
export default FixedDataTableCellGroup; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Now there is no need of this wrapper div .We have provided the styling and the className to the corresponding div .So we can remove this FixedDataTableCellGroup class component and rename FixedDataTableCellGroupImpl to FixedDataTableCellGroup as we are passing all the props from FixedDataTableRow to FixedDataTableCellGroup. |
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.
Change this style so that the content should not overflow from a particular cell .The text wraps to the next line when needed.