Mini pager shows "Page 1" if there is non item at all available

This is simple change.

Well, do we need a pager at all if there are no results?

In the main template_preprocess_pager() in pager.inc we have

  // Nothing to do if there is only one page.
  if ($pager_total[$element] <= 1) {
    return;
  }

Couldn't we do the same for theme_views_mini_pager()?

+++ b/core/modules/views/views.theme.incundefined
@@ -1154,10 +1154,12 @@ function theme_views_mini_pager($variables) {
+  if (count($variables['view']->result) > 0) {
+    $items[] = array(
+      '#wrapper_attributes' => array('class' => array('pager-current')),
+      '#markup' => t('Page @current', array('@current' => $pager_current)),
+    );
+  }

In any case, one may theoretically want to 'reuse' the theme outside of views, and here we are tightening up the theme with a view. The global variable $pager_total_items[$element] would already be providing that info without having to access the view object.

Sorry @dawehner, I realize my comment in #4 is more confusing than helping.

+++ b/core/modules/views/views.theme.incundefined
@@ -1109,6 +1109,12 @@ function theme_views_mini_pager($variables) {
+
+  // Nothing to do if there is only one page.
+  if (isset($pager_total_items[$element]) && $pager_total_items[$element] <= 1) {
+    return;
+  }

In fact, we do not need to access $pager_item_total, nor to check if it set. updatePageInfo() in SqlBase will set all the pager globals after running the count query, including $pager_total. $pager_total counts the number of pages, $pager_total_items counts the records. So, if we want to have no pager in case of zero records, or in case of only one page of results (which is the main 'pager' behavior) then just

if ($pager_total[$element] <= 1) {
    return;
  }

should be sufficient.

This

+  if (isset($pager_total_items[$element]) && $pager_total_items[$element] <= 1) {

would render the pager in case there is only one page of results, but only if there is more than one record returned, which would be weird... :)

EDIT: updatePageInfo() is executed also if count query is not used

Ah ok I see that, sorry for the noise. Still, just tested on simplytest.me

- created 11 nodes
- using view Frontpage
- set mini pager, 10 items per page

- view shows 10 nodes + pager 'Page 1 next>>'

- delete 3 nodes
- view shows 8 nodes + pager 'Page 1' with no links

- delete all nodes but one
- view shows 1 node but no pager

- delete last node
- view shows no pager

So, the point is: do we want to show 'Page 1' if we have only one page of results, regardless of how many items on that page?

if YES, then I think we can do

+++ b/core/modules/views/views.theme.incundefined
@@ -1109,6 +1109,12 @@ function theme_views_mini_pager($variables) {

-  if (isset($pager_total_items[$element]) && $pager_total_items[$element] <= 1) {

+  if (isset($pager_total_items[$element]) && $pager_total_items[$element] == 0) {

if NO (my preference as it will align to main pager) can we do anything like

+  if (isset($pager_total_items[$element]) && $pager_total_items[$element] <= $pager_limits[$element]) {

?

Tested on simplytest.me, following steps in #8. Patch works as described in #9. Has tests. RTBC for me.

@dawehner no need to apologize, in fact I found this discussion valuable.

The reason I have been picky on this one is that I proposed to build a Pager 'component' to replace the current pager functionality, see #2044435: Convert pager.inc to a service, and use classes and methods in place of the current globals.

Now this has been postponed to D9 and there will be time to discuss :), but the 'hack' for the Mini pager is for me a sign that we need to consider, in whatever the future design, that we may have to page on a set of data for which we do not know the total number of elements. This is a shortcoming of the current pager. I will add a comment to that issue too.

For the records: about the hack itself, I understand that in order to avoid double querying the db (once for the count, once for the records), the count query is not executed and the limit of the records-returning query is increased by one to check if there is a 'next' page. This is just used to flag, via postExecute, the need to render the 'next >' link by entering a fictitious high value in total_items, and the recordset is then reduced by one to go back to required behaviour. Fine - but in this case of 0 or 1 page in the view, the total_items will be anyway lower than the page limit, so via upatePageInfo() $pager_total will be set to 0 (if no records), or 1 (if there are records). So testing $pager_total[$element] <= 1 should provide the same result...

+++ b/core/modules/views/views.theme.incundefined
@@ -1097,7 +1097,7 @@ function theme_views_form_views_form($variables) {
-  global $pager_page_array, $pager_total;
+  global $pager_page_array, $pager_total, $pager_total_items;

Looks like this change is no longer needed.

Very true. Also removed the extra blank line that was being added, and added the correct return type.

#12: vdc-2049723-12.patch queued for re-testing.

+++ b/core/modules/views/views.theme.incundefined
@@ -1162,7 +1161,7 @@ function theme_views_mini_pager($variables) {
   if (empty($li_next) && empty($li_previous)) {
-    return;
+    return '';
   }

Will this have to be changed back to return; in the conversion to Twig? See #1912604: Convert theme_views_mini_pager to twig.

Feedback in #11 addressed, passes tests -> back to RTBC

Committed 391755e and pushed to 8.x. Thanks!

This was committed already, see #17