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

[DataGrid] Pagination is 1-based but should be 0-based #852

Closed
2 tasks done
oliviertassinari opened this issue Jan 13, 2021 · 4 comments · Fixed by #1021
Closed
2 tasks done

[DataGrid] Pagination is 1-based but should be 0-based #852

oliviertassinari opened this issue Jan 13, 2021 · 4 comments · Fixed by #1021
Assignees
Labels
breaking change component: data grid This is the name of the generic UI component, not the React module!
Milestone

Comments

@oliviertassinari
Copy link
Member

oliviertassinari commented Jan 13, 2021

  • 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 😯

The pagination is 1-based.

Expected Behavior 🤔

The pagination is 0-based.

Steps to Reproduce 🕹

Steps:

  1. https://material-ui.com/components/data-grid/pagination/#controlled-pagination

Context 🔦

  1. The Pagination component, which is meant for SEO starts at page 1. This is simpler for mapping with the URL: (?page=2).
  2. The TablePagination component which is meant for data grid use cases starts at page 0: https://material-ui.com/components/pagination/#table-pagination. This is simpler to match with the default index of JavaScript arrays.

I believe it would make more sense for the data grid to match TablePagination rather than Pagination. It also seems to be a common convention: ag-Grid is 0-based, and react-table is 0-based too. However, I haven't look further than these two comparison points.

It might be related to #685.

Your Environment 🌎

`npx @material-ui/envinfo`
  System:
    OS: macOS 11.1
  Binaries:
    Node: 12.16.1 - ~/.nvm/versions/node/v12.16.1/bin/node
    Yarn: 1.22.4 - /usr/local/bin/yarn
    npm: 6.14.9 - ~/.nvm/versions/node/v12.16.1/bin/npm
  Browsers:
    Chrome: 87.0.4280.141
    Edge: 87.0.664.75
    Firefox: 84.0.2
    Safari: 14.0.2
  npmPackages:
    @emotion/react: ^11.0.0 => 11.1.4
    @emotion/styled: ^11.0.0 => 11.0.0
    @material-ui/codemod:  5.0.0-alpha.22
    @material-ui/core:  5.0.0-alpha.22
    @material-ui/docs:  5.0.0-alpha.22
    @material-ui/envinfo:  1.1.4
    @material-ui/icons:  5.0.0-alpha.22
    @material-ui/lab:  5.0.0-alpha.22
    @material-ui/styled-engine:  5.0.0-alpha.22
    @material-ui/styled-engine-sc:  5.0.0-alpha.22
    @material-ui/styles:  5.0.0-alpha.22
    @material-ui/system:  5.0.0-alpha.22
    @material-ui/types:  5.1.4
    @material-ui/unstyled:  5.0.0-alpha.22
    @material-ui/utils:  5.0.0-alpha.22
    @types/react: ^17.0.0 => 17.0.0
    react: ^16.14.0 => 16.14.0
    react-dom: ^16.14.0 => 16.14.0
    styled-components:  5.2.1
    typescript: ^4.1.2 => 4.1.3
@oliviertassinari oliviertassinari added breaking change component: data grid This is the name of the generic UI component, not the React module! labels Jan 13, 2021
@oliviertassinari oliviertassinari added this to the beta milestone Feb 5, 2021
@dtassone dtassone self-assigned this Feb 10, 2021
@McGern
Copy link

McGern commented Jun 25, 2021

It's probably just me, but I ended up here wishing that the tablepagination was indexed from 1 as well to make it consistent with the paging component. I am using url paging for my table data and it's a real pain keeping track of the +/- 1 thing because the server, which delivers to both paged and tablepaged components, is 1 indexed.

@oliviertassinari
Copy link
Member Author

@McGern Yeah, it's a tradeoff. Keeping the page in the URL, when there is a single data grid, this is definitely a good UX pattern. We have bet on 0-page as covering more use cases.

@kovryzhenko
Copy link

they really should give us some parameter, like 'basedPagination={0|1|10|100500}' so we can easily use pagination that we want...

@oliviertassinari
Copy link
Member Author

oliviertassinari commented Mar 2, 2023

@ridl27 I would recommend not allowing this to be customized, on the basis that it will complexify the code for most users while probably benefitting a few. The workaround on userland seems simple enough.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change component: data grid This is the name of the generic UI component, not the React module!
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants