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

[5.3] LengthAwarePaginator custom pageName and currentPage #15870

Merged
merged 1 commit into from
Oct 13, 2016
Merged

[5.3] LengthAwarePaginator custom pageName and currentPage #15870

merged 1 commit into from
Oct 13, 2016

Conversation

oaklees
Copy link

@oaklees oaklees commented Oct 12, 2016

Hi there.

New to Laravel, 3 weeks in and absolutely love it.

Ran into minor problem using multiple LengthAwarePaginators on a single page with alternate pageName properties. Traced problem to the attached proposed change.

When checking the current page for a LengthAwarePaginator instance, any non-default pageName is not sent to resolveCurrentPage and as such 1 is always returned (unless pageName of 'page' is used.

Andrew

Ran into minor problem using multiple LengthAwarePaginators on a single page with alternate 'pageName' properties. Traced problem to the attached proposed change.

When checking the current page for a LengthAwarePaginator instance, any non-default pageName is not sent to resolveCurrentPage and as such 1 is always returned (unless pageName of 'page' is used).
@themsaid
Copy link
Member

Hello @jivemonkey2000, can you please provide more details about this issue? How can I replicate it in a fresh laravel 5.3 installation?

@GrahamCampbell GrahamCampbell changed the title Update LengthAwarePaginator.php [5.3] Update LengthAwarePaginator.php Oct 12, 2016
@themsaid
Copy link
Member

ping @jivemonkey2000

@oaklees
Copy link
Author

oaklees commented Oct 13, 2016

Hi @themsaid, sorry, only just got a notification of reply.

Add the following to the base / route:

use Illuminate\Pagination\LengthAwarePaginator;
use Illuminate\Pagination\Paginator;

$collection = collect(['this','is','a','collection','of','items']);
$pageName = 'foo';
$perPage = '5';
$path = '/';

$paginator = new LengthAwarePaginator(
    $collection->forPage(Paginator::resolveCurrentPage($pageName),$perPage),
    $collection->count(),
    $perPage,
    null,
    ["path"=>$path, 'pageName' => $pageName]
);

dd($paginator);

Then, navigate to /?foo=2 you'll see:

LengthAwarePaginator {#170 ▼
  #total: 6
  #lastPage: 2
  #items: Collection {#176 ▶}
  #perPage: "5"
  #currentPage: 1
  #path: "/"
  #query: []
  #fragment: null
  #pageName: "foo"
}

The current page is returning 1, even though the pageName for storing our page number is foo. If we change the request to /?page=2 we then do get the correct currentPage value - but this is what I believe to be incorrect as we have specified a non-default value for the pageName.

I also believe this may also be applicable to Illuminate\Pagination\Paginator but I've yet to check.

Thanks.

Edit: Just to mention, on implementing my fork locally the correct current page is returned.

@oaklees oaklees changed the title [5.3] Update LengthAwarePaginator.php [5.3] LengthAwarePaginator custom pageName and currentPage Oct 13, 2016
@taylorotwell taylorotwell merged commit 6976bf1 into laravel:5.3 Oct 13, 2016
@taylorotwell
Copy link
Member

Thanks.

@oaklees
Copy link
Author

oaklees commented Oct 13, 2016

You're welcome. Keep up the great work.

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.

None yet

3 participants