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

[TablePagination] Out of range warning when "count={-1}" #19865

Closed
2 tasks done
ilmiont opened this issue Feb 26, 2020 · 2 comments · Fixed by #19874
Closed
2 tasks done

[TablePagination] Out of range warning when "count={-1}" #19865

ilmiont opened this issue Feb 26, 2020 · 2 comments · Fixed by #19874
Labels
bug 🐛 Something doesn't work component: table This is the name of the generic UI component, not the React module! good first issue Great for first contributions. Enable to learn the contribution process.

Comments

@ilmiont
Copy link

ilmiont commented Feb 26, 2020

  • The issue is present in the latest release.
  • I have searched the issues of this repository and believe that this is not a duplicate.

Current Behavior 😯

<TablePagination count={-1} page={0} rowsPerPage={10} />

The count={-1} should, per the documentation, indicate that the total count is unknown as we are (probably) using server-side pagination.

I therefore expect to be able to use the next/previous page toggles (excepting previous when on page 1), without the component attempting to enforce an acceptable page range (as it has no way of knowing what that is).

When the count={-1} prop is present, and the page prop changes to anything other than 0, the following warning appears in the console:

Warning: Failed prop type: Material-UI: the page prop of a TablePagination is out of range (0 to 0, but page is 1).

And despite purporting to be a warning, which I could somewhat tolerate, this is actually emitted as an error in the console.

Expected Behavior 🤔

When count={-1}, changing the page of the TablePagination should not emit this warning/error.

Steps to Reproduce 🕹

https://codesandbox.io/s/smoosh-cdn-ov0gg

Steps:

  1. Visit the Code Sandbox.
  2. Simply click the pagination next/previous page buttons and observe the console errors occuring.

Your Environment 🌎

Material UI v4.9.4

@oliviertassinari oliviertassinari added bug 🐛 Something doesn't work component: table This is the name of the generic UI component, not the React module! labels Feb 26, 2020
@oliviertassinari oliviertassinari changed the title "Page prop of a TablePagination is out of range" when "count" prop is "-1" [TablePagination] Out of range warning when "count={-1}" Feb 26, 2020
@oliviertassinari
Copy link
Member

@ilmiont Thanks for the report. What do you think of this patch?

diff --git a/packages/material-ui/src/TablePagination/TablePagination.js b/packages/material-ui/src/TablePagination/TablePagination.js
index 62ce0ed12..5f75bc459 100644
--- a/packages/material-ui/src/TablePagination/TablePagination.js
+++ b/packages/material-ui/src/TablePagination/TablePagination.js
@@ -246,6 +246,11 @@ TablePagination.propTypes = {
    */
   page: chainPropTypes(PropTypes.number.isRequired, props => {
     const { count, page, rowsPerPage } = props;
+
+    if (count === -1) {
+      return null;
+    }
+
     const newLastPage = Math.max(0, Math.ceil(count / rowsPerPage) - 1);
     if (page < 0 || page > newLastPage) {
       return new Error(

Do you want to submit a pull request :)?
I think that we could also add a test interacting with the pagination icons when count={-1}.

@oliviertassinari oliviertassinari added the good first issue Great for first contributions. Enable to learn the contribution process. label Feb 26, 2020
@tutizaraz
Copy link
Contributor

@ilmiont I did Pull Request where we potentially fixed your problem. You can jump up too! :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 Something doesn't work component: table This is the name of the generic UI component, not the React module! good first issue Great for first contributions. Enable to learn the contribution process.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants