Problem/Motivation

There is a problem with the Views full pager, the active page number is not showing on the last page when the quantity value is 1.

Steps to reproduce

  • Create a view with full pager
    pager
  • In the pager option the "Number of pager links visible" should be 1
    pager options
  • Go to the view page and go to the last page, the active page is not displaying in the pager
    last page
  • Change the pager option the "Number of pager links visible" to other values then the active page is displaying in the pager
    last page 2

Proposed resolution

This is because the following code on the template_preprocess_pager is not executing on the last page

  if ($i != $pager_max) {
    // Add an ellipsis if there are further previous pages.
    if ($i > 1) {
      $variables['ellipses']['previous'] = TRUE;
    }
    // Now generate the actual pager piece.
    for (; $i <= $pager_last && $i <= $pager_max; $i++) {
      $options = [
        'query' => $pager_manager->getUpdatedParameters($parameters, $element, $i - 1),
      ];
      $items['pages'][$i]['href'] = Url::fromRoute($route_name, $route_parameters, $options)->toString();
      $items['pages'][$i]['attributes'] = new Attribute();
      if ($i == $pager_current) {
        $variables['current'] = $i;
      }
    }
    // Add an ellipsis if there are further next pages.
    if ($i < $pager_max + 1) {
      $variables['ellipses']['next'] = TRUE;
    }
  }

The logic behind the check if ($i != $pager_max) { is not documented.
It does not really add up because in the nested for loop $i is allowed to be equal with $pager_max
for (; $i <= $pager_last && $i <= $pager_max; $i++) {

So remove if ($i != $pager_max) { condition from here.

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

CommentFileSizeAuthor
#19 interdiff.txt1.33 KBlauriii
#12 3320721-12.patch3 KBranjith_kumar_k_u
#12 interdiff_10-12.txt1.05 KBranjith_kumar_k_u
#12 3320721-12-test-only.patch1.19 KBranjith_kumar_k_u
#10 3320721-10.patch1.82 KBmartins.bertins
#7 After Patch.png127.77 KBprasanth_kp
#7 Before Patch.png127.65 KBprasanth_kp
#6 afte.png165.47 KBgaurav-mathur
#6 before.png138.43 KBgaurav-mathur
#5 After-Patch-3320721-3.png154.25 KBManibharathi E R
#5 Before-Patch-3320721-3.png150.32 KBManibharathi E R
#3 3320721-2.patch895 bytesranjith_kumar_k_u
last-page2.png64.67 KBranjith_kumar_k_u
last-page.png40.11 KBranjith_kumar_k_u
pager-options.png275.78 KBranjith_kumar_k_u
pager-view.png69.23 KBranjith_kumar_k_u
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

ranjith_kumar_k_u created an issue. See original summary.

cilefen’s picture

cilefen CreditAttribution: cilefen as a volunteer commented
Component: other » views.module
ranjith_kumar_k_u’s picture

ranjith_kumar_k_u CreditAttribution: ranjith_kumar_k_u as a volunteer and at Srijan | A Material+ Company for Drupal India Association commented
FileSize
3320721-2.patch895 bytes

A quick fix

ranjith_kumar_k_u’s picture

ranjith_kumar_k_u CreditAttribution: ranjith_kumar_k_u as a volunteer and at Srijan | A Material+ Company for Drupal India Association commented
Status: Active » Needs review
Manibharathi E R’s picture

Manibharathi E R CreditAttribution: Manibharathi E R at Srijan | A Material+ Company for Srijan | A Material+ Company commented
FileSize
Before-Patch-3320721-3.png150.32 KB
After-Patch-3320721-3.png154.25 KB

Patch #3 Applied and Tested successfully on Drupal 9.5.x.

Before Patch:
Before-Patch

After Patch:
After-patch

gaurav-mathur’s picture

gaurav-mathur CreditAttribution: gaurav-mathur at Dotsquares Ltd. commented
FileSize
before.png138.43 KB
afte.png165.47 KB

Patch #3 do great work and Tested successfully on Drupal 9.5.x.

Thanks and Regards .

prasanth_kp’s picture

prasanth_kp CreditAttribution: prasanth_kp as a volunteer and at Zyxware Technologies commented
FileSize
Before Patch.png127.65 KB
After Patch.png127.77 KB

#3 Patch Applied on 9.5.x-dev and it fixes the issue.

heni_deepak’s picture

heni_deepak CreditAttribution: heni_deepak at Dotsquares Ltd. commented
Status: Needs review » Reviewed & tested by the community
Lendude’s picture

Lendude
Primary language Dutch
Location Amsterdam
CreditAttribution: Lendude at iO commented
Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs tests 
  1. +++ b/core/includes/theme.inc
@@ -1883,6 +1883,18 @@ function template_preprocess_pager(&$variables) {
+  // To show last active page when "Number of pager links visible"
+  // is equals to one.

    This is not a proper English sentence. Maybe:
    "Make sure the current page is shown if only one page number is shown."
    or something? 

  2. +++ b/core/includes/theme.inc
@@ -1883,6 +1883,18 @@ function template_preprocess_pager(&$variables) {
+  if ($pager_max == $pager_current && $quantity == 1) {

    can we use '===' ? 

  3. +++ b/core/includes/theme.inc
@@ -1883,6 +1883,18 @@ function template_preprocess_pager(&$variables) {
+    $options = [
+      'query' => $pager_manager->getUpdatedParameters($parameters, $element, $i - 1),
+    ];
+    $items['pages'][$i]['href'] = Url::fromRoute($route_name, $route_parameters, $options)->toString();
+    $items['pages'][$i]['attributes'] = new Attribute();
+    $variables['current'] = $i;

    Is all this really needed? Why is the link removed in the first place? Seems like we should do some more digging into why this breaks at all.

Also, this needs a test. And since the current fix has nothing to do with Views, this seems like a more general problem? But no idea which component this should go into, 'base system' sounds a bit weak too....

martins.bertins’s picture

martins.bertins CreditAttribution: martins.bertins at Wunder commented
FileSize
3320721-10.patch1.82 KB

https://git.drupalcode.org/project/drupal/-/blob/9.5.x/core/includes/the...

if ($i != $pager_max) {
  // Add an ellipsis if there are further previous pages.
  if ($i > 1) {
    $variables['ellipses']['previous'] = TRUE;
  }
  // Now generate the actual pager piece.
  for (; $i <= $pager_last && $i <= $pager_max; $i++) {
    $options = [
      'query' => $pager_manager->getUpdatedParameters($parameters, $element, $i - 1),
    ];
    $items['pages'][$i]['href'] = Url::fromRoute($route_name, $route_parameters, $options)->toString();
    $items['pages'][$i]['attributes'] = new Attribute();
    if ($i == $pager_current) {
      $variables['current'] = $i;
    }
  }
  // Add an ellipsis if there are further next pages.
  if ($i < $pager_max + 1) {
    $variables['ellipses']['next'] = TRUE;
  }
}

This part is never executed for last page if pager quantity is set to 1. Unfortunately the logic behind the check if ($i != $pager_max) { is not documented.
It does not really add up because in the nested for loop $i is allowed to be equal with $pager_max
for (; $i <= $pager_last && $i <= $pager_max; $i++) {

Created a new patch that disables the check in question. Needs tests though.

martins.bertins’s picture

martins.bertins CreditAttribution: martins.bertins at Wunder commented
Component: views.module » theme system

Changing component

ranjith_kumar_k_u’s picture

ranjith_kumar_k_u CreditAttribution: ranjith_kumar_k_u as a volunteer and at Srijan | A Material+ Company for Drupal India Association commented
Version: 9.5.x-dev » 10.1.x-dev
Status: Needs work » Needs review
FileSize
3320721-12-test-only.patch1.19 KB
interdiff_10-12.txt1.05 KB
3320721-12.patch3 KB

Added tests, please review

The last submitted patch, 12: 3320721-12-test-only.patch, failed testing. View results

smustgrave’s picture

smustgrave CreditAttribution: smustgrave at Mobomo commented
Status: Needs review » Needs work
Issue tags: -Needs tests +Needs issue summary update

Issue summary needs to be updated for missing parts.

ranjith_kumar_k_u’s picture

ranjith_kumar_k_u CreditAttribution: ranjith_kumar_k_u as a volunteer and at Srijan | A Material+ Company for Drupal India Association commented
Issue summary: View changes
Status: Needs work » Needs review
smustgrave’s picture

smustgrave CreditAttribution: smustgrave at Mobomo commented
Status: Needs review » Reviewed & tested by the community
Issue tags: -Needs issue summary update +Needs Review Queue Initiative

Confirmed on 11.x that the last page of a view has the active page link.

  • lauriii committed 3da8c309 on 11.x 
    Issue #3320721 by ranjith_kumar_k_u, martins.bertins, Manibharathi E R,...

  • lauriii committed 12690116 on 10.1.x 
    Issue #3320721 by ranjith_kumar_k_u, martins.bertins, Manibharathi E R,...
lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
CreditAttribution: lauriii at Acquia commented
Status: Reviewed & tested by the community » Fixed
FileSize
interdiff.txt1.33 KB

Made minor adjustments to the test on commit; adjusted the wrapping of the comment and simplified the selector a bit.

Committed 3da8c30 and pushed to 11.x. Also cherry-picked to 10.1.x. Thanks!

Not going to backport to 10.0.x or 9.5.x because there's a small behavior change.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.