-
-
Notifications
You must be signed in to change notification settings - Fork 31.7k
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 warning when page is out of range #15616
Comments
This is a question where prop validation should reside. In this case I think app code is more appropriate. The library can't know if the current value will make sense later. Checking for 0,0 is probably a good enough heuristic but it's yet again another heuristic the user has to know. I think it's better that the app dev decides what fallback value should be used (min, max etc) until the data is loaded rather than the library. I'm inclined to believe that the warning is fine here. It's exactly the reason it is only a warning: Your code might not work but we can't be sure so we warn instead of throwing. @fzaninotto Could you include a codesandbox that illustrates the use case? |
Here is the CodeSandbox: https://codesandbox.io/s/zkqpo139rx |
That doesn't really help me understand the issue. The decision to warn against out-of-range pages was a conscious one. What I'm missing is are use cases where this is accepted behavior and why the warning is appropriate in those cases. |
I've modified the code. Is it more explicit? |
I agree with @eps1lon, I think the the warning is correct here. It's the least worse solution to the problem. @fzaninotto Do you have a better alternative in mind? |
Not warning at all if the page is out of bounds when the count is zero seems a reasonable trade off to me. |
@fzaninotto In your example, have you considered? start from 0 const PaginationWithLoading = () => {
const [count, setCount] = useState(-1);
// simulate loading
useEffect(() => {
setTimeout(() => setCount(15), 1000);
});
return (
<TablePagination
page={count === -1 ? 0 : 1}
count={count === -1 ? 0 : count}
rowsPerPage={10}
onChangePage={() => null}
/>
);
}; or start from page 1 const PaginationWithLoading = () => {
const [count, setCount] = useState(-1);
// simulate loading
useEffect(() => {
setTimeout(() => setCount(15), 1000);
});
return (
<TablePagination
page={1}
count={count === -1 ? 1 * 10 + 1 : count}
rowsPerPage={10}
onChangePage={() => null}
/>
);
}; |
Not sure I understand. You suggest to pass a fake but valid count to TablePagination with the data is loading? |
@fzaninotto I'm suggesting to fully assume the table state, either we don't know anything and we display 0 or we optimistically assume the page contains items. |
Interesting. I made the choice to hide the TablePagination during loading, but that creates a flickering effect when the loading ends and the pagination appears. Maybe showing an optimistic pagination would be less perturbing. I'll test that on a few users. It boils down to designing the loading state. That's something the Material Design specification doesn't address if I'm not mistaken, and it's always a hard task. |
To reiterate on another point that was addressed by this change: Passing an out-of-range page could cause bugs depending on how |
@fzaninotto Have you found an acceptable solution to the problem? |
Yes, I've hidden the pagination while loading. It adds a flicker, but there is one anyway (since the total number of results changes). Thanks for your help. |
Why not just check against an out-of-range page or use |
Displaying an out of range page or a wrong page number during loading is worse than displaying nothing IMO, because it's showing information that we know are wrong. |
is it Fixed in v4.9.3 ???? still there is error at the latest version.. fix properly please |
If you do this: And this: Then it fixes the issue. |
Set your page property on your like this: |
Have you all considered if someone has a largeish data set and wants to be able to start in the middle, how to handle it? For example, if I have 1000 records and i do the page length of 50 per page, and i want to start at page 10 (without having to pre-load 500 records), how could i accomplish this? I was trying to do so with using queryParams (pageNumber), and then loading the page via the api, however if I use pageNumber in with the TablePagination, I get: |
@lostmarinero Do you have a small reproduction we could look at? The TablePagniation component is only concerned with the display, you could "lies" to him around what data is available. |
So still no stable solution found? |
Maybe related: mbrn/material-table#955 (for me, the data changes when I'm on page 2, and the table doesn't update the page when data is changed, so the page is now out of bounds). |
Can you show how you hid the pagination on load? |
Get the same error and it was pretty simple, modify code like that, its work for me but i get count from redux.
|
|
onPageSizeChange={(params) => params.page = 1 } |
|
Thanks for sharing your approach. Could you provide more detail about how you're passing around the state of 'data'? I currently have a bunch of handler functions to update the current page. How are your handler methods working with the useEffect hook? I took a stab leveraging your solution but so far haven't been able to reset the page. I am doing something like
|
Hi @fzaninotto , thanks for sharing your approach, I'd like to try out the various strategies in this thread. May you give a little more detail on how you hide the pagination while loading the data? What part of the component is that custom method embedded? Thank you for your time. |
what 's solution finally ? I have try many ways , it doesn't work . |
this wont work when you are on the last page of a set that has fewer items than what youre paginating by |
I have an issue with this feature. I implemented custom pagination because I need the results to be paginated server-side, but I still use the pagination option because it's convenient. To make it work, however, I manage the next and previous page buttons myself (the table only receives 50 items, but I know that the server has a total of 60, so even with rowsPerPage=50 the next page button is clickable). When I go to the next page, everything does work well, but I get a console warning because the table thinks there is only one page, and so the current page is out of range. I would like a way to artificially tell the table how many pages I think there are, or to silence this warning, since I know that the range is going to be out of sync with the current page, and it is expected behaviour. |
const handlePageSize = (elementAmount: number): void => {
const maxPage = (Math.floor(elementAmount / rowsPerPage) - 1);
if (page >= maxPage) {
setPage(maxPage);
}
} |
Just came across this and resolved like so: <TablePagination
onPageChange={handlePageChange}
page={rowsPerPage >= count ? 0 : page}
/> |
I also came across this issue and found this issue thread where multiple solutions were suggested which I tried one-by-one but nothing really worked 😄. But below is the easy solution which I used and worked pretty well
|
This was superhelpful. |
This help me, thank you kind stranger. |
Better solution: <TablePagination
onPageChange={handlePageChange}
page={count && page}
/> |
This change adds a warning in a common use case.
Let's say a user is displaying a list with two pages. The user goes to page 2. Then, he chooses to refresh the page. The console shows a warning:
This is because, when the user refreshes the page, the list is empty (count=0) until the data is loaded. This "loading" state fails the page prop validation, which creates a warning.
This used to work well in v1, I'm not sure this regression is a desired effect.
Originally posted by @fzaninotto in #14534 (comment)
The text was updated successfully, but these errors were encountered: