Skip to content

Improve frozen columns performance #1744

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

Merged
merged 21 commits into from
Oct 7, 2019
Merged
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
35 changes: 0 additions & 35 deletions examples/__tests__/Grid.spec.js

This file was deleted.

10 changes: 7 additions & 3 deletions examples/scripts/example03-frozen-cols.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,9 @@ import exampleWrapper from '../components/exampleWrapper';
class Example extends React.Component {
constructor(props, context) {
super(props, context);
this.createRows();
const extraColumns = [...Array(500).keys()].map(i => ({ key: `col${i}`, name: `col${i}` }));
this.createRows(extraColumns);

const extraColumns = [...Array(50).keys()].map(i => ({ key: `col${i}`, name: `col${i}` }));
const columns = [
{
key: 'id',
Expand Down Expand Up @@ -59,7 +59,7 @@ class Example extends React.Component {
return new Date(start.getTime() + Math.random() * (end.getTime() - start.getTime())).toLocaleDateString();
};

createRows = () => {
createRows = (extraColumns) => {
const rows = [];
for (let i = 1; i < 1000; i++) {
rows.push({
Expand All @@ -71,6 +71,10 @@ class Example extends React.Component {
startDate: this.getRandomDate(new Date(2015, 3, 1), new Date()),
completeDate: this.getRandomDate(new Date(), new Date(2016, 0, 1))
});

for (const extraColumn of extraColumns) {
rows[rows.length - 1][extraColumn.key] = `${extraColumn.key}`;
}
}

this._rows = rows;
Expand Down
1 change: 1 addition & 0 deletions examples/scripts/example31-isScrolling.js
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ class Example extends React.Component {
rowsCount={this.state.rows.length}
minHeight={800}
onScroll={this.onScroll}
enableIsScrolling
/>
</div>
);
Expand Down
10 changes: 5 additions & 5 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
"scripts": {
"docs": "node docs/utils/docsGen.js",
"test": "jest",
"test-dev": "jest --watch",
"test:watch": "jest --watch",
"postinstall": "lerna bootstrap --no-ci",
"prebuild": "node tools/buildStylesheets.js",
"build": "lerna run build",
Expand All @@ -25,8 +25,8 @@
"@types/classnames": "^2.2.9",
"@types/enzyme": "^3.10.1",
"@types/jest": "^24.0.15",
"@types/react": "^16.8.23",
"@types/react-dom": "^16.8.3",
"@types/react": "^16.9.2",
"@types/react-dom": "^16.9.0",
"@types/react-is": "^16.7.1",
"@types/shallowequal": "^1.1.1",
"@typescript-eslint/eslint-plugin": "^1.11.0",
Expand All @@ -49,13 +49,13 @@
"less-loader": "^5.0.0",
"markdown": "^0.5.0",
"node-dir": "^0.1.12",
"react": "^16.8.6",
"react": "^16.9.0",
"react-contextmenu": "^2.10.0",
"react-data-grid": "file:packages/react-data-grid",
"react-data-grid-addons": "file:packages/react-data-grid-addons",
"react-dnd-test-backend": "^2.6.0",
"react-docgen": "^2.21.0",
"react-dom": "^16.8.6",
"react-dom": "^16.9.0",
"react-router-dom": "^5.0.1",
"recharts": "^1.6.2",
"style-loader": "^0.23.1",
Expand Down
51 changes: 24 additions & 27 deletions packages/react-data-grid-addons/src/__tests__/Grid.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ describe('Grid', () => {
};

const getBaseGrid = (wrapper) => {
return wrapper.instance().base.current;
return wrapper.find('Grid');
};

const buildFakeEvent = (addedData) => {
Expand All @@ -57,7 +57,7 @@ describe('Grid', () => {
};

const simulateGridKeyDownWithKeyCode = (wrapper, keyCode) => {
getBaseGrid(wrapper).props.onViewportKeydown(buildFakeEvent({ keyCode }));
getBaseGrid(wrapper).props().onViewportKeydown(buildFakeEvent({ keyCode }));
};

it('should create a new instance of Grid', () => {
Expand Down Expand Up @@ -87,18 +87,19 @@ describe('Grid', () => {
beforeEach(() => {
wrapper = setup({ toolbar: <ToolBarStub /> }).wrapper;
wrapper.find(ToolBarStub).props().onToggleFilter();
wrapper.update();
});

it('uses the appropriate default for the grid row height', () => {
expect(getBaseGrid(wrapper).props.rowHeight).toEqual(35);
expect(getBaseGrid(wrapper).props().rowHeight).toEqual(35);
});

it('uses the appropriate default for the header row height', () => {
expect(getBaseGrid(wrapper).props.headerRows[0].height).toEqual(35);
expect(getBaseGrid(wrapper).props().headerRows[0].height).toEqual(35);
});

it('uses the appropriate default for the header filter row height', () => {
expect(getBaseGrid(wrapper).props.headerRows[1].height).toEqual(45);
expect(getBaseGrid(wrapper).props().headerRows[1].height).toEqual(45);
});
});

Expand All @@ -108,18 +109,19 @@ describe('Grid', () => {
beforeEach(() => {
wrapper = setup({ toolbar: <ToolBarStub />, rowHeight: 40 }).wrapper;
wrapper.find(ToolBarStub).props().onToggleFilter();
wrapper.update();
});

it('passes the correct heigth to the grid rows', () => {
expect(getBaseGrid(wrapper).props.rowHeight).toEqual(40);
expect(getBaseGrid(wrapper).props().rowHeight).toEqual(40);
});

it('passes the grid row heigth to the header row when no height to the specific header row is provided', () => {
expect(getBaseGrid(wrapper).props.headerRows[0].height).toEqual(40);
expect(getBaseGrid(wrapper).props().headerRows[0].height).toEqual(40);
});

it('uses the default prop height for the filter row when none is provided', () => {
expect(getBaseGrid(wrapper).props.headerRows[1].height).toEqual(45);
expect(getBaseGrid(wrapper).props().headerRows[1].height).toEqual(45);
});
});

Expand All @@ -134,18 +136,19 @@ describe('Grid', () => {
headerFiltersHeight: 60
}).wrapper;
wrapper.find(ToolBarStub).props().onToggleFilter();
wrapper.update();
});

it('passes the correct heigth to the grid rows', () => {
expect(getBaseGrid(wrapper).props.rowHeight).toEqual(40);
expect(getBaseGrid(wrapper).props().rowHeight).toEqual(40);
});

it('passes the correct heigth to the header row', () => {
expect(getBaseGrid(wrapper).props.headerRows[0].height).toEqual(50);
expect(getBaseGrid(wrapper).props().headerRows[0].height).toEqual(50);
});

it('passes the correct heigth to the header filter row', () => {
expect(getBaseGrid(wrapper).props.headerRows[1].height).toEqual(60);
expect(getBaseGrid(wrapper).props().headerRows[1].height).toEqual(60);
});
});
});
Expand All @@ -169,7 +172,7 @@ describe('Grid', () => {

it('should set filter state of grid and render a filterable header row', () => {
expect(wrapper.instance().state.canFilter).toBe(true);
expect(getBaseGrid(wrapper).props.headerRows.length).toEqual(2);
expect(getBaseGrid(wrapper).props().headerRows.length).toEqual(2);
});
});
});
Expand All @@ -182,11 +185,11 @@ describe('Grid', () => {

beforeEach(() => {
({ wrapper, columns, rows } = setup({ enableRowSelect: true }));
selectRowCol = getBaseGrid(wrapper).props.columnMetrics.columns[0];
selectRowCol = getBaseGrid(wrapper).props().columnMetrics.columns[0];
});

it('should render an additional Select Row column', () => {
expect(getBaseGrid(wrapper).props.columnMetrics.columns.length).toEqual(columns.length + 1);
expect(getBaseGrid(wrapper).props().columnMetrics.columns.length).toEqual(columns.length + 1);
expect(selectRowCol.key).toEqual('select-row');
expect(TestUtils.isElementOfType(selectRowCol.formatter, CheckboxEditor)).toBe(true);
});
Expand Down Expand Up @@ -232,7 +235,7 @@ describe('Grid', () => {
describe('when selected is false', () => {
beforeEach(() => {
wrapper.instance().setState({ selectedRows: [{ id: 0, isSelected: false }, { id: 1, isSelected: false }, { id: 2, isSelected: false }, { id: 3, isSelected: false }] });
const selectRowCol = getBaseGrid(wrapper).props.columnMetrics.columns[0];
const selectRowCol = getBaseGrid(wrapper).props().columnMetrics.columns[0];
selectRowCol.onCellChange(3, 'select-row', rows[3], buildFakeEvent());
});

Expand All @@ -244,7 +247,7 @@ describe('Grid', () => {
describe('when selected is null', () => {
beforeEach(() => {
wrapper.instance().setState({ selectedRows: [{ id: 0, isSelected: null }, { id: 1, isSelected: null }, { id: 2, isSelected: null }, { id: 3, isSelected: null }] });
const selectRowCol = getBaseGrid(wrapper).props.columnMetrics.columns[0];
const selectRowCol = getBaseGrid(wrapper).props().columnMetrics.columns[0];
selectRowCol.onCellChange(2, 'select-row', rows[2], buildFakeEvent());
});

Expand All @@ -256,7 +259,7 @@ describe('Grid', () => {
describe('when selected is true', () => {
beforeEach(() => {
wrapper.instance().setState({ selectedRows: [{ id: 0, isSelected: null }, { id: 1, isSelected: true }, { id: 2, isSelected: true }, { id: 3, isSelected: true }] });
const selectRowCol = getBaseGrid(wrapper).props.columnMetrics.columns[0];
const selectRowCol = getBaseGrid(wrapper).props().columnMetrics.columns[0];
selectRowCol.onCellChange(3, 'select-row', rows[3], buildFakeEvent());
});

Expand Down Expand Up @@ -292,7 +295,7 @@ describe('Grid', () => {
}
}
}).wrapper;
selectRowCol = getBaseGrid(wrapper).props.columnMetrics.columns[0];
selectRowCol = getBaseGrid(wrapper).props().columnMetrics.columns[0];
});

it('should call rowSelection.onRowsSelected when row selected', () => {
Expand Down Expand Up @@ -348,7 +351,7 @@ describe('Grid', () => {
}
});

const selectRowCol = getBaseGrid(wrapper).props.columnMetrics.columns[0];
const selectRowCol = getBaseGrid(wrapper).props().columnMetrics.columns[0];

// header checkbox
const checkboxWrapper = document.createElement('div');
Expand Down Expand Up @@ -381,7 +384,7 @@ describe('Grid', () => {
}
});

const selectRowCol = getBaseGrid(wrapper).props.columnMetrics.columns[0];
const selectRowCol = getBaseGrid(wrapper).props().columnMetrics.columns[0];

// header checkbox
const checkboxWrapper = document.createElement('div');
Expand Down Expand Up @@ -417,12 +420,6 @@ describe('Grid', () => {
});

describe('Table width', () => {
it('should generate the width based on the container size', () => {
const { wrapper } = setup();
const tableElement = ReactDOM.findDOMNode(wrapper.instance());
expect(tableElement.style.width).toEqual('100%');
});

describe('providing table width as prop', () => {
it('should set the width of the table', () => {
const { wrapper } = setup();
Expand Down Expand Up @@ -451,7 +448,7 @@ describe('Grid', () => {
}
});

getBaseGrid(wrapper).props.cellMetaData.onCellClick({ idx: 1, rowIdx: 1 });
getBaseGrid(wrapper).props().cellMetaData.onCellClick({ idx: 1, rowIdx: 1 });
expect(rowClicks).toBe(1);
const { row, column } = rowClicked;
expect(row).toMatchObject(rows[1]);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,8 @@ export default function(stateValues, events) {
width: 400,
totalWidth: 0,
totalColumnWidth: 400,
minColumnWidth: 80
minColumnWidth: 80,
lastFrozenColumnIndex: -1
},
selectedRows: [],
canFilter: false,
Expand Down
Loading