Framework: Pagination issues with onEachSide

Created on 24 May 2020  路  5Comments  路  Source: laravel/framework


  • Laravel Version: 6.18.13, 7.12.0
  • PHP Version: 7.3.17
  • Database Driver & Version: MySQL 5.7.29

Description:

The default paginator appears to be optimized for onEachSide value of 3, resulting in discrepancies when onEachSide is set to a different value.

Issues with onEachSide set to 1

In order to make a small pagination views that fits in the mobile device view-port, I used the onEachSide() method with value 1 and observed the following issues:

  1. The number 2 is repeated on both sides separator when current page is 3
    image

  2. The number (total pages - 1) is repeated when current page is (total pages - 2)
    image

There are some more superficial issues like

  • The pagination view still does not fit within the small device view.
    Screen Shot 2020-05-24 at 00 21 37
  • unnecessary separator (...) between 2 and 3 when current page is 4 and similar when current page is (total - 3).
  • unnecessary separator (...) between 2 and 4 which can be replaced with 3 when current page is 5.

Issues with onEachSide set to 5

When onEachSide is set to 5, a different set of issues arise.

  1. The number of links on each side is not honored. For example, in the first image, the number of links on right side is only 2, and in the second image the links on left side are only 3.
    image
    image

Steps To Reproduce:

Create a pagination view for any query that returns more than 20 pages.
{{ $query->paginate()->onEachSide(1)->links() }}
{{ $query->paginate()->onEachSide(5)->links() }}

Suggestions/Bug Fixes

  1. Bug Fix: The class Illuminate\Pagination\UrlWindow seems to be optimized for default onEachSide value of 3. There are a lot of hardcoded assumptions that break when it is set to a value much different. For example, both the methods getSliderTooCloseToBeginning() and getSliderTooCloseToEnding() has a hard coded value of 2 in page range which should be equal t to $onEachSide and $onEachSide -1 respectively. Also, the value of $window in getUrlSlider() should ideally be $onEachSide + 4 instead of current value of $onEachSide * 2.

  2. Feature request: The paginator always shows 2 links on each end which results in minimum of 9 links being shown when onEachSide is set to 1. These many links can't fit in a xtra-small bootstrap screen. The suggestion is to either allow another setting onEachEnd on the paginator or assume the user wants a small paginator and show only one link on each end when onEachSide is set to 1.

Most helpful comment

This will be fixed in the next patch release.

All 5 comments

onEachSide shouldn't be used with anything lower than 2. I'll send in a PR to the docs to add a note about this.

Also see https://github.com/laravel/framework/issues/28789

@driesvints That will still leave issues with onEachSide for values different than 2 or 3, for ex 5 as I have shown above. Only 2 links on the right when close to beginning and 3 links on left when close to ending are shown as these values are hard-coded.

Would you be interested in a very small and simple PR that introduces no breaking changes and passes all tests while fixing the paginator for all values from 1 onwards?

@adwiv you can always attempt a PR but it's up to Taylor to decide if he wants it merged in.

This will be fixed in the next patch release.

Thanks a lot @taylorotwell for the patch.

There is a small change that I missed mentioning earlier.

When setting $window = $onEachSide + 4 we also need to change the condition when small slider is returned. The small slider should be returned whenever total pages is less than or equal to 2 + $onEachSide + 1 + $onEachSide + 2 i.e. last <= ($onEachSide * 2) + 7

Hence the condition in the UrlWindow::get() method should be changed from ($onEachSide * 2) + 6) to ($onEachSide * 2) + 8) in Line 47

With the current patch we are getting unnecessary separators in edge cases (when total pages = 12 or 13 for onEachSide set to default (3).

image
image

Was this page helpful?
0 / 5 - 0 ratings

Related issues

felixsanz picture felixsanz  路  3Comments

shopblocks picture shopblocks  路  3Comments

jackmu95 picture jackmu95  路  3Comments

gabriellimo picture gabriellimo  路  3Comments

Anahkiasen picture Anahkiasen  路  3Comments