Clean up pager theme functions

This is my summary copied from the original issue:

I replaced all the theme_pager_* functions with theme suggestions based off of theme_pager_link(). With this change a themer can add any of the following functions to template.php:

mytheme_pager_link($variables){ ... }
mytheme_pager_link__first($variables){ ... }
mytheme_pager_link__last($variables){ ... }
mytheme_pager_link__previous($variables){ ... }
mytheme_pager_link__next($variables){ ... }
.. or they can create the corresponding tpl instead.

Thanks @sun for copying this over ...

drupal8.pager-theme.0.patch queued for re-testing.

reroll

Doh! .. re-roll

I missed that, thanks ...

this patch includes the "rel" attributes

I had a small error in #9. Please ignore it and review this one

Sun: WOW you're fast...

I think this needs to be rerolled because of the attributes change
#1290694: Provide consistency for attributes and classes arrays provided by template_preprocess()

See http://drupal.org/node/1727592

No, that was my code that needed to be updaed :/

Okay, patch applies cleanly.

bartik_pager_link() works.
bartik_pager_link__first() works.
bartik_pager_link__last() works.

But bartik_pager_link__previous() and bartik_pager_link__next() seem to affect more than just the previous and next links. It also affects the numeric link that represents the same page as the next/prev link.

I don't think that's the desired affect.

This should solve the issue that jenlampton caught in #14

... and one more without the whitespace issues

related change record http://drupal.org/node/1795832 from #1777324: Remove theme_user_list() from core here

What tests do we need for this? Manual ones to ensure the CSS/Markup is equivalent? Would be good to get this one in.

Manual ones to ensure the CSS/Markup is equivalent

That is how I have been testing...

#16: 1598886.patch queued for re-testing.

This is my first attempt at re-rolling a patch. I hope it worked properly.

I did a manual testing. Here's a diff of the markup.

--- /Users/dodo/Desktop/pager.html 
+++ (clipboard) 
@@ -4,17 +4,17 @@
       <a rel="first" title="Go to first page" href="/drupal/node">« first</a>
     </li>
     <li class="pager-previous even">
-      <a rel="first" title="Go to previous page" href="/drupal/node">‹ previous</a>
+      <a rel="prev" title="Go to previous page" href="/drupal/node">‹ previous</a>
     </li>
     <li class="pager-item odd">
-      <a rel="first" title="Go to page 1" href="/drupal/node">1</a>
+      <a title="Go to page 1" href="/drupal/node">1</a>
     </li>
     <li class="pager-current even">2</li>
     <li class="pager-item odd">
-      <a rel="last" title="Go to page 3" href="/drupal/node?page=2">3</a>
+      <a title="Go to page 3" href="/drupal/node?page=2">3</a>
     </li>
     <li class="pager-next even">
-      <a rel="last" title="Go to next page" href="/drupal/node?page=2">next ›</a>
+      <a rel="next" title="Go to next page" href="/drupal/node?page=2">next ›</a>
     </li>
     <li class="pager-last odd last">
       <a rel="last" title="Go to last page" href="/drupal/node?page=2">last »</a>

It looks good to me.

 <li class="pager-current even">2</li>

Its always bugged me that the links in the pager have the benefit of an extra element in the dom for theming, while the plain text items (current page, and ellipses) do not. This means you have to get creative (and inconsistent) with the CSS. Maybe this belongs as a separate request, for CSS lint cleanup or standardizing the Component Library but it would be incredibly useful to add a wrapper span around non-links in the pager, which would pave the way for consistent css like this:

.pager-last a,
.pager-first a,
.pager-next a,
.pager-previous a,
.pager-ellipse span,
.pager-current span {
  display: inline-block;
  ...
}

I understand if this is too little too late, but its worth a try ;)

In other words, #26 is justification for providing theme_pager_link__ellipse and theme_pager_link__current as well.

One more thing...

Quoting from the proposed resolution in #1499460: [meta] New theme system (emphasis added by me):

[...] we would like to write as few new templates as possible. Match theme functons to HTML and reuse them. For example, a list of menu links, a list of pager elements etc is just a list.

Please, correct me if wrong, but this kind of throws a monkey wrench into the current work to *create more themables for pagers*.

Without knowing too much about the internals of pagers, it seems like it would be pretty easy to convert pagers to a simple theme_list() if we moved the classes off of the LI into to the item appearing in the list, and used wrapper elements like SPANS suggested in #26. It would also make the CSS even *more* clean than the example in #26, because we could get away with using *just classes*:

<li class="odd first"><a rel="first" class="pager-first">« first</a></li>
<li class="even"><a rel="previous" class="pager-previous" [...]>[...]</a></li>
<li class="odd"><span class="pager-ellipse">...</span></li>
<li class="even"><a rel="current" class="pager-current active">...</a></li>
[etcetera]

.pager-last,
.pager-first,
.pager-next,
.pager-previous,
.pager-ellipse,
.pager-current {
  display: inline-block;
  ...
}

@jwilson3: that's #1778990: Merge theme_pager_link*() theme functions into theme_link() but this patch makes that one easier - since we end up with only one pager theme function instead of four or however many it currently is to convert.

So based on the re-roll in #24 and the HTML diff in #25 is this now RTBC?

Thanks catch.

One more question, why are there two underscores in pager_link__first? is this a new feature of drupal 8 theming?

@jwilson .... actually that was added in D7. See http://drupal.org/node/1089656 for more details

#lightbulb. Ok, yeah. Definitely knew about theme suggestions, but always encountered that feature from the front end point of view '--', never really thought about how that could be useful in Drupal core. Nice usage here.

RTBC per the manual testing.

looks like its Twig related too
+1 RTBC for #24

Committed/pushed to 8.x, thanks!

Will need a change notice for the removed/replaced theme functions.

Havent done a change notice before so lemme know what Im missing... http://drupal.org/node/1819308

The content looks fine, but all the metadata/title etc. is about the entity render controller.

It appears that instead of making a new change notice, you edited an existing one? I've reverted it. See http://drupal.org/node/1819308/revisions/view/2409260/2409470

I'm guessing you were using one as an example, and just copy/pasted into the wrong tab.

DOH!!!!! Tim.plunkett nailed it.

Sorry bout that. This should be better: http://drupal.org/node/1819788

Looks great.