Pager should allow changing the "number of items per page" (and allow "show all")

rather then expect others to do a better job, please attach your patch for others to examine your appraoch and comment yes/no/ ideas.

bump: this is a ubiquitous interface outside of drupal, and incredibly handy. it'd be *really* nice to have this for D6. anyone available to work on it? my hands are way too full as it is already... :(

i could see problems with running out of memory, especially if people used 'show all' carelessly (e.g. on the front page of d.o)... if that's the only objection, i think we can come up with some solutions. worst case is we add a new permission for using this functionality. better yet is some fine-grained settings to contol limits for different pager queries (gee, if only we had a query builder and each query had a unique query id). ;) perhaps in the code, where we currently hard-code the value, we'd just hard-code the upper limit and default value for that particular pager query. or some combination of the above.

one of my primary motivations is for the issue queue pager. by default, 20 issues seems ok, but in some cases it'd be really handy to bump that upto 100, or even do a "show all" for a given project. thing is, this isn't a site-wide setting that makes sense, nor would it work as a per-user setting in your profile. i'd want to see different things depending on what issue queue i was looking at. always having a tab open on my profile page to keep changing the setting would drive me insane. in theory, i *could* add some custom crap to the pagers in project* to handle this as a special case, but i'd rather just put that effort into the D6 core pager...

anyone else with me? ;) any chance this is going to get committed if i come up with a nice solution? i'd hate to work like crazy on this, only to run into a veto from the D6 maintainers...

Yes, I was clicking on this link in your mail to come and share my performance concerns. What if someone asks for a few hundred nodes on the tracker pager for example? Tracker is already slow enough as far as I see. So limits should be in place. Otherwise it is indeed better to have this setting in the URL (against my point expressed on 2005-10-12), because you can hand around links that way, which would list the same number of things on the pager (maybe if the user has the permission, if this is bound to permissions).

Hello!

:)

It would be great to add a user choice widget to pager so that we can choose from some options (5, 10, 15, 100 or Show All) - and seeing that it didn't happen for 4.X, 5 or D6 - can it happen for D7? Pretty Please?

I realise it can put load on the server but ways to mitigate that risk were pointed out above.

Also - one query that delivers 1000 results saves another 20 queries looking through pages that deliver 50 results each.

Give me the option to grind my server to a halt. Restrict it to admin users, whatever... but really, it would be very very useful.

Sadly, no chance for D7 now. :( This is a new feature, and we're long past feature freezes and exceptions for D7. This will have to happen in D8 if it happens. I've got no time to work on it anytime soon, so I'm unassigning myself, too.

What if someone submitted a patch? Would there be a chance?

No chance, sorry. A feature freeze means that the only patches accepted to Drupal 7 core are bug fixes at this point. If they kept allowing new features, we'd never stabilize the release and be able to ship 7.0.

Fair enough! :)

So if we do submit a patch (already halfway there, for our own use) to Drupal8 (once that's open) is there any chance it could get backported to 7.x or something? Or should we be looking at just maintaining this for our own use?

Dries and I have yet to hash out what features exactly from D8 could get back-ported, but since this implies UI changes I'm not sure we could do it. As a book author and documentation team member, I'm /very/ sensitive to invalidating screenshots and documentation.

For now, though, I'd like to focus on just getting D7 out the door. :)

Well, so that there's no duplication of effort for 8.x, here's my 7.x patch that allows for the 'show all' option on pagers.

It strikes me that it might be possible to work this into a module, so it could be a contrib thing for D7.

I have reworked the patch, so it works as I think it should. Clicking 'show all' will suppress all but the '« first' and 'last »' links now. It's set via the url ?all=1 so it will work in links.

As indicated by Gábor, 'show all' can cause a lot of load on a system if a giant list of stuff is loaded (and then themed, possibly with sub-queries) so I've added a permission. There is not currently a warning on that permission, though.

If a user without this permission clicks an ?all=1 link, it will happily degrade to showing the first page of a normal pager.

Attached is the patch against git 8.x.

subscribe

Status Ignored? What does that mean?

#14: pager-39954-13.patch queued for re-testing.

Fixed the exceptions in the test bot by properly initialising variables.

Patch applies cleanly and works as expected. Neither coder nor my cursory review find any style issues. I can't speak to this as a desired feature in core but if it is it seems to fit the bill well enough. ;)

This seem like a reasonable feature. Thanks for working on the patch! I have just one VERY minor note on code style:

+++ b/includes/pager.incundefined
@@ -73,7 +73,15 @@ class PagerDefault extends SelectQueryExtender {
+    // If the user did not click 'show all', calculate the query range. Otherwise

@@ -194,6 +202,17 @@ function pager_find_page($element = 0) {
+ *  A boolean that indicates whether the user has access and the link was clicked.

I think these lines are 1-2 characters too long, so let's wrap the last word of each onto the next line.

Also, note that the Drupal 8.x patch will need to be rerolled, because the core directory structure for Drupal 8 has now changed. (For more information, see #22336: Move all core Drupal files under a /core folder to improve usability and upgrades). When the patch has been rerolled, please set the issue back to "Needs Review."

Tagging as novice for the task of rerolling the Drupal 8.x patch (preferably with the line rewrapping mentioned above).

If you need help rerolling this patch, you can come to core office hours or ask in #drupal-gitsupport on IRC.

(my first patch - under supervision)

Updated the patch to apply to latest Drupal 8.x and adjusted comments as suggested by @xjm.

Thanks @kattekrab, it looks great.

+++ b/core/includes/pager.incundefined
@@ -194,6 +202,18 @@ function pager_find_page($element = 0) {
+ * Determines whether the current user has access to the 'show all' link in
+ * the pager and whether the show all link was clicked.

Just noticed this summary is two lines. I've fixed that in the attached patch.

Now, let's have one more person test out the patch and add some screenshots of how it works. Thanks!

Oops. Darn dreditor. :)

Installed D8 with patch applied, used devel to generate 1000 nodes. It certainly takes a moment to display them, but it displays 1000 nodes with the option to click back to the start or end of a pager based display, instead of show all.

In other words. It works!

Three screenshots attached showing

Permissions
"Access "show all" fuctionality in pagers "
[which has a typo - hooray - my 2nd patch will follow this.]

Content list
50 devel nodes showing
" 1 2 3 4 5 6 7 8 9 … next › last » show all "

Content list
1000 devel nodes showing
" « first last » "

My 2nd Patch (tm)

Applied xjm comments changes from #24 and fixed typo in the permission.

New screenshot with typofix shown attached.

Removed stealth whitespace.
Added description to the permission.

And just for OCD sake - a screenshot showing the permission description is attached.

I've also tested a user without permission to access the show all, and huzzah - the 'show all' does not show. Likewise editing the URL manually to show all (eg node&all=1) does not show all.

I think that it would be a very useful feature to be able to nominate how many or for all records to be displayed on a page. It would make it much easier when searching - for example I often have to search through pages of users which would be made much easier if I could just do a find once on one page rather than on ten. Great idea : )

So I dreamed about this patch last night. Seriously. Anyway, I realized the behavior after you have clicked "show all" is not intuitive--"first" and "last" don't make any sense. What I'd prefer is a link to switch me back to the previous view. Maybe just one link to toggle it back, "show X" (50 for now), and it by default uses the last page you were on when paging. What do you think?

@xjm - when I grow up, I wanna dream about patches too! :)

But yes, I tend to agree. I'm just not sure of a solution.

So... based on the cursory research I just did - here's a super simple approach.

Add "show less" to the page that shows all - and link back to the first page of results.
Mockup attached.

Perhaps some usability / ux folks could weigh in with suggestions / point to a better approach?

Research

So here's a quick cursory glance at some pagination UI design patterns.

http://ui-patterns.com/patterns/Pagination
http://www.cxpartners.co.uk/cxblog/pagination_a_lazy_interface_decision_...
http://www.smashingmagazine.com/2007/11/16/pagination-gallery-examples-a...
http://ux.stackexchange.com/questions/789/pagination-best-practice
http://www.onextrapixel.com/2010/11/03/15-ui-design-patterns-web-designe...

Here's a Drupal example
http://mundanity.com/pagination-example

(Sorry, please don't hit me, but...)

Hm. One thing I observe from all of those screenshots is that none of the pagers referenced there have this "Show all" feature. :\ So while the feature itself sounds useful, I'm not sure it should be coupled with the pager.

This one shows the "show all" link :)

http://drupal.org/files/d8-pager-39954-showall.png

Summary of IRC discussion:

+++ b/core/includes/pager.inc
@@ -355,6 +381,12 @@ function theme_pager($variables) {
+  // Only display the 'show all' link to users with permission.
+  if (user_access('show all items in pager')) {
+    // Check if the user has clicked 'show all'.
+    $pager_all = pager_show_all();
+    $li_all = theme('pager_all', array('text' => (isset($tags[5]) ? $tags[5] : t('show all')), 'element' => $element, 'parameters' => $parameters));
+  }

Yeah, I don't see how this is going to fly.

What this essentially does is make a global "flag" that anyone in the admin role is exempt from and so is any user I want to have this feature anywhere else on my site, that says in ALL instances of a pager, regardless of how many elements are in that pager and what those elements are, there will be a "Show all" link. Both in the case of a small list of small things, like 120 aggregator items AND in a list of large things like a list of 50,000 nodes with 30 fields a piece each of which requires 170 database queries to build).

This "Show all" element needs to be contextual, and used on a per-pager basis, IMO.

26 days to next Drupal core point release.

I agree that it's probably not a good idea to have a 'Show All' link on all pagers on all sites. However, there is not currently a way to allow contrib modules to affect the pager beyond theming it, so some sort of change must be made to core to allow this functionality be added.

A different way to address this issue is to allow PagerDefault to be pluggable (See the second half of #778050: Add support for database hints and make PagerDefault properly pluggeable) so that contrib modules can implement their own pager queries and thus their own pager functionality.

I've also written a patch (attached) that allows the setting of a global pager limit. Currently the default defined in pager.inc is 10. This patch changes the default to 50 (which is what admin/people and admin/content were using anyway) and allows site administrators to change this default to any number between 10 and 1000. It expressly warns about performance if a number > 200 is chosen.

This patch changes the pagers in node.admin.inc and user.admin.inc to use th new user-configurable default. Other pagers in core are not affected, unless they didn't specify a number of items to begin with.

Bugger, forgot to set the patch to needs review. Reattaching.

+++ b/core/modules/system/system.admin.incundefined
@@ -3170,4 +3208,4 @@ function system_action_delete_orphans_post($orphaned) {
+}
\ No newline at end of file

Please don't kill Mr. Newline!

(Need a newline at the end of the file.)

In general, this approach seems better, and it's a definite improvement over the current situation without the drawbacks above.

From reading the patch, it doesn't look like end users can configure it. I think I'd probably expect the option to show up next to the pager, rather than buried in the system config forms somewhere. Maybe that could live in contrib, though, given this patch.

Screenshots would be helpful here as well.

Oops, didn't mean to do that. :)

Did someone say screenshots?

Attached screenshots.

Maybe we should tell the user that numbers between 10 and 1000 are accepted...

I guess if we wanted to keep this a global thing, another option could be setting the threshold for number of records beyond which the "Show all" link is hidden as an option. variable_get('pager_show_all_record_threshold', 150) or variable_get('pager_show_all_page_threshold', 3) or something. This really only comes up in situations where the number of records displayed is just more than will fits.

Besides the 'show all' option, how is this different from the 'Number of posts per page' setting already available under config/site-information?

I don't see it, what I'm missing here?

#43: That setting doesn't have any effect on the pager, to my knowledge. Separate functionality. That's only used for the front page and for term listings (teaser pages). The pager on the other hand is used for all manner of lists, like are found on admin/content, admin/people, etc.

However, we probably should make it obvious to users what the difference is, and maybe put the two together on the same form?

Edited to be less nonsensical.

Ah, thanks. You are right, listings in admin go to 25 or 50. Hmm, so the existing one is for site front-end content listings maybe?

I think the only way to make this obvious is to treat these as *the same* thing first :) The mental image I got from this thread was more like this:

Wouldn't that be a nicer gesture than moving into an admin setting? Either way this seems like merely a nice to have and I'm not sure it's up to core to provide it out of the box.

The mismatch I had with that existing setting tells me we need a clearer picture of what's already defined for pager/result list lengths before extending the feature.

need a clearer picture of what's already defined for pager/result list lengths

There's isn't really anything. All pagers in core (minus the front page and term list) and contrib use whatever who wrote said feature thought would be appropriate. If nothing is specified, the pager defaults to 10 items per page.

An exception is the admin taxonomy term list, which lists 100 items per page. It references a variable that would in theory be able to adjust that size, except there's no UI to set said value. See #1332534: Long hierarchy taxonomy memory exhaustion; pager problem.

Wouldn't that be a nicer gesture than moving into an admin setting?

Either way, a way of changing the list size is what I want. I'm not particular bothered about how we accomplish that ;-)

I'm not sure it's up to core to provide it out of the box

Again, that fine. But if core doesn't provide a way of changing the pager page size, it should provide a way for contrib to do this. Then any number of pager modules could provide any number of pagers, making all users happy :-)

This seems to be languishing again... any progress?

Here is the patch applied to the latest version without killing the newline.

Can we have a screen of the latest screen?

Here is a screenshot and an updated patch.

Thanks @NROTC_Webmaster!

• We still haven't really addressed the confusion from #43, I don't think.
• I think I'd actually prefer to lose the configuration form and just retain cafuego's API for contrib to use, if anything.
• Something like yoroy's mockup would be great, and even appropriate to core (I think). Perhaps this API could also provide a basis for that functionality, though (perhaps as a separate followup?)

Tagging for an issue summary to summarize the possible directions for this issue. We should probably pick one before proceeding (that's why I think this issue got stuck before).

Code review follows. Note that below I pointed out things to fix if we keep the form that exposes this config to the administrator (with the style notes about those lines), but as stated I personally think it would be better to nix all the lines related to the form. :)

1. +++ b/core/includes/pager.incundefined
   @@ -10,6 +10,11 @@ use Drupal\Core\Database\Query\SelectInterface;
   + * The amount of items per page to show in pagers.
   ...
   +++ b/core/modules/system/system.moduleundefined
   @@ -1024,6 +1024,15 @@ function system_menu() {
   +    'description' => t('Configure the amount of items per page shown in generated lists.'),
   

   Grammar nitpick: These should be "The number of items" rather than "The amount."

2. +++ b/core/includes/pager.incundefined
   @@ -68,10 +73,14 @@ class PagerDefault extends SelectExtender {
   +    else if (empty($this->limit)) {
   

   Should be elseif rather than else if. Reference: http://drupal.org/coding-standards#controlstruct

3. +++ b/core/includes/pager.incundefined
   @@ -131,13 +140,14 @@ class PagerDefault extends SelectExtender {
   +   * The default if not specified is PAGER_ITEMS items per page.
   ...
   +   *   An integer specifying the number of elements per page. If passed a 0
   +   *   the configured default number of items is used. If passed a NULL or
   +   *   FALSE, the pager is disabled.
   

   Alright, so this host of different possible magic values is a little confusing. Not sure how to address it but that's a lot of special cases. It's also not clear in the method why all this is the case, so at the least I'd want some @see explaining why. Is there any way we can make the behaviors more explicit, though?

   Edit: I see now it's from the protected variable on the class, so let's clarify that at least. Still not sure about the magic "0 vs. NULL vs. FALSE," though.

   Aside (super nitpick) the word "passed" doesn't make sense in the @param. Given the current logic, I'd make the @param:
   An integer specifying the number of elements per page. If 0, the default from the 'pager_items' configuration variable is used. If NULL or FALSE, the pager is disabled.

4. +++ b/core/includes/pager.incundefined
   @@ -131,13 +140,14 @@ class PagerDefault extends SelectExtender {
   -  public function limit($limit = 10) {
   +  public function limit($limit = 0) {
   

   So we are going from a default of 10 to one of 50, correct?

5. +++ b/core/modules/system/system.admin.incundefined
   @@ -1612,6 +1612,44 @@ function system_run_cron_submit($form, &$form_state) {
   + * Form builder; Pager form.
   + *
   + * @ingroup forms
   + * @see system_settings_form()
   + */
   

6. The summary should read:
   Form constructor for the pager settings form.
   It should also have @see to its submission and validation handlers. Reference: http://drupal.org/node/1354#forms

7. +++ b/core/modules/system/system.admin.incundefined
   @@ -1612,6 +1612,44 @@ function system_run_cron_submit($form, &$form_state) {
   +  $form['pager_items'] = array(
   +    '#type' => 'textfield',
   

   We should maybe use a number element for this once #1174640: Add new HTML5 FAPI element: number is in!

8. +++ b/core/modules/system/system.admin.incundefined
   @@ -1612,6 +1612,44 @@ function system_run_cron_submit($form, &$form_state) {
   +    '#description' => t('Enter the number of items the pager should show on a single page. If you set this too high, large listings might slow down your site.'),
   

   Do we need the second sentence here when we have the warning? Or, maybe we don't need the first sentence.

9. +++ b/core/modules/system/system.admin.incundefined
   @@ -1612,6 +1612,44 @@ function system_run_cron_submit($form, &$form_state) {
   + * Validate handler for the pager settings form.
   

   This should read:
   Form validation handler for system_pager_settings().. Reference: http://drupal.org/node/1354#forms

10. +++ b/core/modules/system/system.admin.incundefined
    @@ -1612,6 +1612,44 @@ function system_run_cron_submit($form, &$form_state) {
    +    form_set_error('pager_items', t('You must enter a positive whole number between 10 and 1000.'));
    ...
    +  else if (intval($form_state['values']['pager_items']) < 10) {
    ...
    +  else if (intval($form_state['values']['pager_items']) >= 1000) {
    ...
    +  else if (intval($form_state['values']['pager_items']) >= 200) {
    

    These seem kind of magic-numbery; maybe we should define some more constants?

xjm,
Along the same lines I'm updating the patch according to your recommendations.

For #7 since we are using the number field do we want to add the min/max values here?
[edit] - the number field didn't work on my site. I don't think the patch was actually merged yet.

For #8 since we are giving a description to the input field I think it is essential to have the first sentence, but we can probably live without the second one since it is given on the validation.

In regards to #10 is there a specific reason we would not want the admin to "allow" the number to be less than 10? While I can't think of any specific examples where it may be useful having the option is always my preference. Maybe just a warning that it might hurt the user interface?

I don't know how to add the issue summary at the top but here are the things I see.

Issue Summary

Problem/Motivation

There is no way for a site to set how many items can be displayed using pager. Currently it is 10.

Proposed resolution

It would be nice to allow the site admin or end user to set how many items they want to display per page using pager. Also increasing the default limit to 50.

Remaining tasks

Decide on the proper implementation.
Two methods have been proposed. Allow the site admin to set the number or the end user to set it when viewing a page.

User interface changes

Admins or users will be able to set a number of items to display per page using pager.

Questions:

1. What numbers should be used for min/max values
2. Where do we want the validation performed?

Actually use this version.

Lets hope that this is the last one.

Pagers are pretty much antique by now :) I'd much rather abandon the pager concept wholesale and aim for infinite scroll: #1488866: Redesign pagers for administrative lists.

Isn't infinite scroll 100% dependent on javascript?

@NROTC_Webmaster, you can put the issue summary in the issue summary (click the "Edit" tab at the top of the issue). :) Also, it would be good to have more details in the proposed resolution about the different approaches that have been suggested and the implications of each.

Updated issue summary.

Updated issue summary.

Updated issue summary.

The issue summary has been updated.

Also, infinite scroll does not help when you are trying to use the browser search function to find something in a long list of stuff.

But - I agree that the numbered list of pages also isn't generally useful in trying to guess where in a long list something might be. Pages divided alphabetically are sometimes better for that kind of thing... as in mailman.

Would select all work with infinite scroll? What about graceful degradation for non-JS browsers?

It seems that technology moves faster than this issue, and because we won't settle for anything but the perfect solution, we'll never see an implementation. For administrative pages (where this is really needed!), I'd be perfectly happy with having the limit in the URL.

Updated issue summary.

Add alphabetical sort to the options

Proposed resolution

It would be nice to allow the site admin [...] to set how many items they want to display per page using pager.

Until we see a solution in core, the Pager Limits module does just that for D7, without requiring any core patches.

issue summary exists

While the latest patch adds a menu for changing the number of posts for the pager, it does not actually cause a change in the number of posts displayed. (for me, anyway)

I went to Configuration >> Pager and changed the number of posts in the appropriate field to 4, hit the save button, cleared the cache, and checked the /node page and it still pages at Drupal's default of 10 posts.

Updated issue summary.

Updated issue summary.

Updated issue summary.

We missed feature freeze on this - but with views now in Core - perhaps the pager issue went somewhere else?

oh hello Pager issue.

Can we revisit this again now?

Updated issue summary.

This is solved in ITMT in D11 (and before?)
See attached screendump.