-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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] Fix server-side pagination #639
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fix #568 and other small issues
Please one pull request at the time, it doesn't work https://deploy-preview-639--material-ui-x.netlify.app/components/data-grid/pagination/#server-side-pagination
Co-authored-by: Olivier Tassinari <olivier.tassinari@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pagination is not working: https://deploy-preview-639--material-ui-x.netlify.app/components/data-grid/pagination/#server-side-pagination. The CI should be red, a test case should be falling in the current state.
I tried the docs locally, and it works. I also tried with the filtering, columns, and all the new options disabled, and it works. Any idea? |
Is it feature flag related?
This is a client side error, and unrelated. Probably your ad blocker. |
I mean, it's the error on the server, I didn't mean it occured on the backend |
I don't follow, we probably have a different vocabulary, what's the backend, what's the server? I was assuming this terminology: server = node.js, backend = we don't have one. |
For me, server can be the CI server, vs local where it's my own machine. |
I managed to reproduce the error locally. It's due to the production code, so probably an issue with the minifier... 🤔 |
Just a general question, a lot of the changes from this PR are also in this one #642. I guess this PR should be the first one merged right? |
Yes |
The documentation pages are broken after the changes, however, it doesn't seem directly related, so happy to move forward. We need to fix the pages before we can cut a new release. |
Fix #568