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

make pagination limit configurable globally #105

Open
gondo opened this issue May 25, 2014 · 7 comments
Open

make pagination limit configurable globally #105

gondo opened this issue May 25, 2014 · 7 comments

Comments

@gondo
Copy link
Contributor

gondo commented May 25, 2014

currently pagination limit default value is harcoded in function declaration https://github.com/KnpLabs/knp-components/blob/master/src/Knp/Component/Pager/Paginator.php#L83
however passing $limit = null cause exception:
https://github.com/KnpLabs/knp-components/blob/master/src/Knp/Component/Pager/Paginator.php#L87
this is a problem when im trying to set options like this:
$paginator->paginate($query, 1, null, array(...));
basically this construction force me to specify proper $limit every time i call paginate()

it would be much better if $limit was in $defaultOptions as this way i could set $limit on one place (in Symfony2 it would be config.yml) globally for every paginator.

@salemgolemugoo
Copy link

It would be nice

@ostrolucky
Copy link
Contributor

I propose to change this interface in 2.0 to

interface PaginatorInterface
{
    function paginate($target): PaginationInterface;
    function withPage(int $page): PaginatorInterface
    function withLimit(int $limit): PaginatorInterface
    function withOptions(array $options): PaginatorInterface
}

Where in default implementation:

  • with* methods will be immutable setters. This will help dealing with cases when there is multiple paginators on single page
  • default $page will be inferred from request, as is currently done for sorting. There is even already pageParameterName, but is currently unused.
  • default $limit will be inferred from defaultOption

ping @polc

@garak
Copy link
Collaborator

garak commented Jun 21, 2019

You can open a PR on branch 2.0 if change is BC-breaking (like changing interfaces).
If instead you simply add an option, you can open a PR on master branch

@ostrolucky
Copy link
Contributor

I thought there will be more time until release of 2.0, now it's too late to do BC breaking change

@daria-sieroshtan
Copy link
Contributor

If this issue is still relevant I can open PR to add $limit to $defaultOptions (without BC break).

@ostrolucky
Copy link
Contributor

It's still relevant. At the same time, I'm not sure what you plan to do. If you add it to $options, it will ignore limit specified in $page.

@daria-sieroshtan
Copy link
Contributor

Default limit value will be set in $defaultOptions, not hardcoded in function itself.

  • Explicitly passing $limit as an argument will work as previously.
  • Passing null and getting default limit will be possible while setting other options, like mentioned
    $paginator->paginate($query, 1, null, array(...));
  • Later with a wrapper functionality in the KnpPaginatorBundle default limit will be configurable globally, as other default options.

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

No branches or pull requests

5 participants