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

Add partial pagination support #297

Merged

Conversation

alanpoulain
Copy link
Member

Q A
Bug fix? yes
New feature? yes
BC breaks? no
Deprecations? no
Tickets fixes #276
License MIT
Doc PR N/A

Add a partial pagination support by adding a custom previous / next pagination component when needed.

It can be considered as a bug fix and as a new feature.

It uses some kind of hack for the hydra data provider to transmit pagination information to the pagination component, by using the total prop.

I don't think there is a better way to do so without modifying react-admin.

@fzaninotto / @djhi maybe it would be interesting if you could give your point of view?

@alanpoulain
Copy link
Member Author

Merging for now but you can comment on this PR 🙂

@alanpoulain alanpoulain merged commit 845bd7f into api-platform:master May 12, 2020
@fzaninotto
Copy link
Contributor

I'm not sure I understand the problem. Can you please elaborate?

@alanpoulain
Copy link
Member Author

alanpoulain commented May 13, 2020

Sure. In API Platform we have a partial pagination support: https://api-platform.com/docs/core/pagination/#partial-pagination. It means the API doesn't tell how many pages there are, but only if there is a previous / next page.
When activated for a resource, the previous behavior was buggy because the react-admin pagination component was used and it needs to know the number of pages to work correctly.

To fix it, I created a pagination wrapper. Its behavior changes according to the information on the pagination: if the total is given, its the classical pagination, if not it's the previous / next pagination. However, in order to know if the next button needs to be displayed or not, the hydra data provider needs to tell the component if there is a next page or not. In the response format of getList ({ data: {Record[]}, total: {int}, validUntil?: {Date} }), there is no way to add more information (at least I don't see one) than the data and the total.

So I needed to use some tricks (use total), to send the information about the next page. Maybe a way would be to add an optional parameter (extraInformation for instance?) in the response format and to pass it as a prop to the pagination component?

@fzaninotto
Copy link
Contributor

Thanks, it's much clearer.

We would need an additional field in the response (I'd call it meta) to carry that kind of information. But it requires updating quite a lot of things (TypeScript types, dataProvider signatures, dataProvider hooks, controllers, views, pagination) and I find your hacky solution not too hacky ;)

So let's say that we won't address that use case in react-admin for now.

@alanpoulain
Copy link
Member Author

alanpoulain commented May 13, 2020

OK. And if provide a PR in react-admin, would it be alright? Because I think it would be useful for the Mercure support too (#144). For Mercure, a link to the hub is sent in a response header. But the data provider would have no way to send this link to the components needed to subscribe to the updates.

@fzaninotto
Copy link
Contributor

Sure: a good way to see if it's not too big a change is to give it a try. Make sure to PR against the next branch though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Error after enabling Partial Pagination
2 participants