Skip to content

[5.3] LengthAwarePaginator custom pageName and currentPage - #15870

Merged
taylorotwell merged 1 commit into
laravel:5.3from
oaklees:5.3
Oct 13, 2016
Merged

taylorotwell merged 1 commit into
laravel:5.3from
oaklees:5.3

Conversation

@oaklees

@oaklees oaklees commented Oct 12, 2016

Copy link
Copy Markdown

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
Copy Markdown
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
Copy Markdown
Member

ping @jivemonkey2000

@oaklees

oaklees commented Oct 13, 2016 •

Copy link
Copy Markdown
Author

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
Copy Markdown
Member

Thanks.

@oaklees

oaklees commented Oct 13, 2016

Copy link
Copy Markdown
Author

You're welcome. Keep up the great work.

salmanarshad321 pushed a commit to salmanarshad321/framework that referenced this pull request Sep 23, 2026
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).
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.

3 participants