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.
Comment | File | Size | Author |
---|---|---|---|
#7 | drupal.css.patch | 395 bytes | Anonymous (not verified) |
pager.inc_0.patch | 1.61 KB | dikini | |
Comments
Comment #1
dikini CreditAttribution: dikini commenteda complementary patch to drupal.css, to preserve the styling.
Comment #2
Steven CreditAttribution: Steven commentedThis patch does not conform to the coding standards. Please check the usage of spaces and quotes.
Comment #3
Steven CreditAttribution: Steven commentedAlso, 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?
Comment #4
Steven CreditAttribution: Steven commentedAnd 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.
Comment #5
Dries CreditAttribution: Dries commentedPatch needs more work as per Steven's suggestions. Please resubmit.
Comment #6
(not verified) CreditAttribution: commentedSorry for the delay. I've been really busy lately.
I attach the corrected patch to pager.inc
Comment #7
(not verified) CreditAttribution: commentedand the patch to drupal.css
Comment #8
dikini CreditAttribution: dikini commentedchanging status to patch
Comment #9
Dries CreditAttribution: Dries commentedComment #10
Steven CreditAttribution: Steven commentedStill 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.
Comment #11
Steven CreditAttribution: Steven commentedMore 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).
Comment #12
killes@www.drop.org CreditAttribution: killes@www.drop.org commentedSteven didn't fully like it, please fix and resubmit.
Comment #13
dikini CreditAttribution: dikini commentedirrelevant now