Skip to content

Commit

Permalink
Fix issue where rows equal to row option would cause error (#948)
Browse files Browse the repository at this point in the history
* Fix issue where rows equal to row option would cause error

* Fix issue 994 by extending current page checks

* Add test for out of bounds pages in pagination
  • Loading branch information
gabrielliwerant authored Oct 15, 2019
1 parent b8bee52 commit 94d8cd0
Show file tree
Hide file tree
Showing 5 changed files with 27 additions and 14 deletions.
10 changes: 3 additions & 7 deletions src/MUIDataTable.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import TableResize from './components/TableResize';
import TableToolbar from './components/TableToolbar';
import TableToolbarSelect from './components/TableToolbarSelect';
import textLabels from './textLabels';
import { buildMap, getCollatorComparator, sortCompare } from './utils';
import { buildMap, getCollatorComparator, sortCompare, getPageValue } from './utils';

const defaultTableStyles = theme => ({
root: {},
Expand Down Expand Up @@ -886,20 +886,16 @@ class MUIDataTable extends React.Component {
};

changeRowsPerPage = rows => {
/**
* After changing rows per page recalculate totalPages and checks its if current page not higher.
* Otherwise sets current page the value of nextTotalPages
*/
const rowCount = this.options.count || this.state.displayData.length;
const nextTotalPages = Math.floor(rowCount / rows);

this.setState(
() => ({
rowsPerPage: rows,
page: this.state.page > nextTotalPages ? nextTotalPages : this.state.page,
page: getPageValue(rowCount, rows, this.state.page),
}),
() => {
this.setTableAction('changeRowsPerPage');

if (this.options.onChangeRowsPerPage) {
this.options.onChangeRowsPerPage(this.state.rowsPerPage);
}
Expand Down
11 changes: 6 additions & 5 deletions src/components/TableBody.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import TableBodyRow from './TableBodyRow';
import TableSelectCell from './TableSelectCell';
import { withStyles } from '@material-ui/core/styles';
import cloneDeep from 'lodash.clonedeep';
import { getPageValue } from '../utils';

const defaultBodyStyles = {
root: {},
Expand Down Expand Up @@ -55,12 +56,12 @@ class TableBody extends React.Component {
if (this.props.options.serverSide) return data.length ? data : null;

let rows = [];
const totalPages = Math.floor(count / rowsPerPage);
const fromIndex = page === 0 ? 0 : page * rowsPerPage;
const toIndex = Math.min(count, (page + 1) * rowsPerPage);
const highestPageInRange = getPageValue(count, rowsPerPage, page);
const fromIndex = highestPageInRange === 0 ? 0 : highestPageInRange * rowsPerPage;
const toIndex = Math.min(count, (highestPageInRange + 1) * rowsPerPage);

if (page > totalPages && totalPages !== 0) {
console.warn('Current page is out of range.');
if (page > highestPageInRange) {
console.warn('Current page is out of range, using the highest page that is in range instead.');
}

for (let rowIndex = fromIndex; rowIndex < count && rowIndex < toIndex; rowIndex++) {
Expand Down
3 changes: 2 additions & 1 deletion src/components/TablePagination.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import MuiTableRow from '@material-ui/core/TableRow';
import MuiTableFooter from '@material-ui/core/TableFooter';
import MuiTablePagination from '@material-ui/core/TablePagination';
import { withStyles } from '@material-ui/core/styles';
import { getPageValue } from '../utils';

const defaultPaginationStyles = {
root: {
Expand Down Expand Up @@ -63,7 +64,7 @@ class TablePagination extends React.Component {
}}
count={count}
rowsPerPage={rowsPerPage}
page={page}
page={getPageValue(count, rowsPerPage, page)}
labelRowsPerPage={textLabels.rowsPerPage}
labelDisplayedRows={({ from, to, count }) => `${from}-${to} ${textLabels.displayRows} ${count}`}
backIconButtonProps={{
Expand Down
8 changes: 7 additions & 1 deletion src/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,12 @@ function buildMap(rows) {
}, {});
}

function getPageValue(count, rowsPerPage, page) {
const totalPages = count === rowsPerPage ? 0 : Math.floor(count / rowsPerPage);

return page > totalPages ? totalPages : page;
}

function getCollatorComparator() {
if (!!Intl) {
const collator = new Intl.Collator(undefined, { numeric: true, sensitivity: 'base' });
Expand Down Expand Up @@ -90,4 +96,4 @@ function createCSVDownload(columns, data, options) {
}
}

export { buildMap, getCollatorComparator, sortCompare, createCSVDownload };
export { buildMap, getPageValue, getCollatorComparator, sortCompare, createCSVDownload };
9 changes: 9 additions & 0 deletions test/MUIDataTablePagination.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -46,4 +46,13 @@ describe('<TablePagination />', function() {
instance.handlePageChange(null, 1);
assert.strictEqual(changePage.callCount, 1);
});

it('should correctly change page to be in bounds if out of bounds page was set', () => {
// Set a page that is too high for the count and rowsPerPage
const mountWrapper = mount(<TablePagination options={options} count={5} page={1} rowsPerPage={10} />);
const actualResult = mountWrapper.find(MuiTablePagination).props().page;

// material-ui v3 does some internal calculations to protect against out of bounds pages, but material v4 does not
assert.strictEqual(actualResult, 0);
});
});

0 comments on commit 94d8cd0

Please sign in to comment.