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

Fix issue where rows equal to row option would cause error #948

Merged
merged 4 commits into from
Oct 15, 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
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);
});
});