This patch tackles two issues

The pager code is not properly separating presentation from content, so writing a pager theme would possibly invole a lot of code duplication.

The default pager theme will not pass the WAI level 3 guidelines

10.5 Until user agents (including assistive technologies) render adjacent links distinctly, include non-link, printable characters (surrounded by spaces) between adjacent links.

Admittedly, this is a weak requirement, but one which is quite helpful. In this case I'm constructing an unordered list of links, which is semantically correct, and I think better follows the spirit of w3c reccomendations than the original.

I think it would be more logical to use the same approach for the whole pager, but in the end decided not to change the code too much.

The patch introduces a new function

php function theme_pager_list($pager_list) {
   if( !empty($pager_list )) {
      $output = "<ul class='pager'>\n";
      foreach( $pager_list as $pager_item ) {
          $output .= "<li>".$pager_item."</li>";
      }
      $output .="</ul>\n";
   }
   return $output;
}

which is responsible for the rendering of the pager list. The only downside is the introduction of a second loop over the same data, but I think the gains flexibility outhweghts the minor loss of performance.

CommentFileSizeAuthor
#7 drupal.css.patch395 bytesAnonymous (not verified)
pager.inc_0.patch1.61 KBdikini
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

dikini’s picture

a complementary patch to drupal.css, to preserve the styling.

Steven’s picture

This patch does not conform to the coding standards. Please check the usage of spaces and quotes.

Steven’s picture

Also, I wonder about the trailing space in the list items. These seem unnecessary. Why not set 0.25em of margin to the left/right of the <li>s?

Steven’s picture

And finally, you should probably remove the padding/margin from the ul.pager item (also, why use .pager in the first rule, and ul.pager in the second?). List indentation and margins differ between various browsers, and pagers should be a single line only.

Dries’s picture

Patch needs more work as per Steven's suggestions. Please resubmit.

Anonymous’s picture

Sorry for the delay. I've been really busy lately.
I attach the corrected patch to pager.inc

Anonymous’s picture

FileSize
395 bytes

and the patch to drupal.css

dikini’s picture

changing status to patch

Dries’s picture

Category: bug » feature
Steven’s picture

Still does not conform to style. Check:
- Spaces around brackets and comma's
- Double quotes vs single quotes

The space after each pager item is still there and unnecessary.

Steven’s picture

More comments:
- The 'current page' indicator does not appear to have any class or tag around it, which makes it impossible to style with CSS.
- There are implode() calls with reversed arguments. This works, but it is deprecated and should be avoided. The correct order is implode($delimiter, $array).

killes@www.drop.org’s picture

Steven didn't fully like it, please fix and resubmit.

dikini’s picture

Status: Active » Closed (fixed)

irrelevant now