Skip to content
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

Open
wants to merge 4 commits into
base: column-virtualization
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion site/examples/examplesPageStyle.less
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,7 @@
.exampleTitle {
margin-bottom: 0;
overflow: hidden;
white-space: nowrap;
white-space: normal;
Copy link
Author

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.

text-overflow: ellipsis;
}

Expand Down
94 changes: 28 additions & 66 deletions src/FixedDataTableCellGroup.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Copy link
Author

Choose a reason for hiding this comment

The 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
Expand Down Expand Up @@ -59,6 +59,8 @@ class FixedDataTableCellGroupImpl extends React.Component {

isHeaderOrFooter: PropTypes.bool,

offsetLeft: PropTypes.number,

Copy link
Author

Choose a reason for hiding this comment

The 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,

/**
Expand Down Expand Up @@ -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
);
}

Copy link
Author

Choose a reason for hiding this comment

The 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,
};

Copy link
Author

Choose a reason for hiding this comment

The 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;
Expand Down Expand Up @@ -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,
Expand All @@ -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;
}

Copy link
Author

Choose a reason for hiding this comment

The 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)
Expand All @@ -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
Expand All @@ -207,7 +232,6 @@ class FixedDataTableCellGroupImpl extends React.Component {
className={className}
height={this.props.rowHeight}
key={key}
columnIndex={columnIndex}
Copy link
Author

Choose a reason for hiding this comment

The 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}
Expand All @@ -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;
Copy link
Author

Choose a reason for hiding this comment

The 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.

Loading