From d2c29c1f9fcb13a766c6da8884443ee143f486d9 Mon Sep 17 00:00:00 2001 From: Gabriel Liwerant Date: Fri, 20 Sep 2019 19:20:01 -0400 Subject: [PATCH 1/3] Fix issue where rows equal to row option would cause error --- src/MUIDataTable.js | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/src/MUIDataTable.js b/src/MUIDataTable.js index d0fe3d5fb..fbe4f8e14 100644 --- a/src/MUIDataTable.js +++ b/src/MUIDataTable.js @@ -878,20 +878,19 @@ 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 - */ + // After changing rows per page, determine if our current page is too high + // for the number of rows, and adjust accordingly (pages are 0-indexed). const rowCount = this.options.count || this.state.displayData.length; - const nextTotalPages = Math.floor(rowCount / rows); + const totalPages = rowCount === rows ? 0 : Math.floor(rowCount / rows); this.setState( () => ({ rowsPerPage: rows, - page: this.state.page > nextTotalPages ? nextTotalPages : this.state.page, + page: this.state.page > totalPages ? totalPages : this.state.page, }), () => { this.setTableAction('changeRowsPerPage'); + if (this.options.onChangeRowsPerPage) { this.options.onChangeRowsPerPage(this.state.rowsPerPage); } From 2d197c338f3894456e8265a383d2719d7edb4ed9 Mon Sep 17 00:00:00 2001 From: Gabriel Liwerant Date: Thu, 10 Oct 2019 16:05:25 -0400 Subject: [PATCH 2/3] Fix issue 994 by extending current page checks --- src/MUIDataTable.js | 7 ++----- src/components/TableBody.js | 11 ++++++----- src/components/TablePagination.js | 3 ++- src/utils.js | 8 +++++++- 4 files changed, 17 insertions(+), 12 deletions(-) diff --git a/src/MUIDataTable.js b/src/MUIDataTable.js index fbe4f8e14..0f2e03665 100644 --- a/src/MUIDataTable.js +++ b/src/MUIDataTable.js @@ -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: {}, @@ -878,15 +878,12 @@ class MUIDataTable extends React.Component { }; changeRowsPerPage = rows => { - // After changing rows per page, determine if our current page is too high - // for the number of rows, and adjust accordingly (pages are 0-indexed). const rowCount = this.options.count || this.state.displayData.length; - const totalPages = rowCount === rows ? 0 : Math.floor(rowCount / rows); this.setState( () => ({ rowsPerPage: rows, - page: this.state.page > totalPages ? totalPages : this.state.page, + page: getPageValue(rowCount, rows, this.state.page), }), () => { this.setTableAction('changeRowsPerPage'); diff --git a/src/components/TableBody.js b/src/components/TableBody.js index efa91e9dc..6d658be93 100644 --- a/src/components/TableBody.js +++ b/src/components/TableBody.js @@ -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: {}, @@ -53,12 +54,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++) { diff --git a/src/components/TablePagination.js b/src/components/TablePagination.js index cc20ea175..e091164d4 100644 --- a/src/components/TablePagination.js +++ b/src/components/TablePagination.js @@ -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: { @@ -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={{ diff --git a/src/utils.js b/src/utils.js index e376eb1a0..22dab1ea7 100644 --- a/src/utils.js +++ b/src/utils.js @@ -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' }); @@ -90,4 +96,4 @@ function createCSVDownload(columns, data, options) { } } -export { buildMap, getCollatorComparator, sortCompare, createCSVDownload }; +export { buildMap, getPageValue, getCollatorComparator, sortCompare, createCSVDownload }; From 1986670e95acafed7ebab23fa72cf4a84c7f013a Mon Sep 17 00:00:00 2001 From: Gabriel Liwerant Date: Thu, 10 Oct 2019 16:19:26 -0400 Subject: [PATCH 3/3] Add test for out of bounds pages in pagination --- test/MUIDataTablePagination.test.js | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/test/MUIDataTablePagination.test.js b/test/MUIDataTablePagination.test.js index a5a6e9bfd..71719c167 100644 --- a/test/MUIDataTablePagination.test.js +++ b/test/MUIDataTablePagination.test.js @@ -46,4 +46,13 @@ describe('', 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(); + 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); + }); });