Posted by joachim on February 5, 2009 at 4:56pm
Jump to:
| Project: | Term Display |
| Version: | 6.x-1.x-dev |
| Component: | Code |
| Category: | feature request |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | closed (fixed) |
Issue Summary
Using s standard theming function to create a list is better design.
Patch attached but diff has gone a bit mental with the braces so it's not as clear to the eye as it could be.
Basically I am replacing the whole of theme_term_display_list() with the following.
/**
* Theme terms as a list.
*/
function theme_term_display_list($vocabulary, $terms) {
foreach($terms as $term) {
$term_links[] = l($term->name, taxonomy_term_path($term));
}
return theme('item_list', $term_links, check_plain($vocabulary->name));
}| Attachment | Size |
|---|---|
| term_display-theme_item_list.patch | 846 bytes |
Comments
#1
Agreed that we shd use the standard functions where possible. In this case I didn't because I wanted to have this list themable by adding special classes. We could use the $options argument to feed a class to l() though:
<?php$term_links[] = l($term->name, taxonomy_term_path($term), array('attributes' => array('class' => 'vocabulary-list')));
?>
#2
Or set the class on the UL -- gives you more styling options. If you need to style the LIs, use CSS selectors.
return theme('item_list', $term_links, check_plain($vocabulary->name), 'ul', array('class' => 'vocabulary-list'));#3
Sounds good, pls go ahead and commit that change.
#4
Nedjo - will you be able to port/commit these changes to the D6 branch or do you want some assistance?
I guess in most instances the port will be relatively trivial?
#5
Good point.
Yes, the patch needs to be applied first to HEAD (from which the D6 releases are done) and then to the DRUPAL-5 branch.
I don't think this part of the code changed with the upgrade to 6, so the same patch shd apply.
#6
Committed to both HEAD and DRUPAL-5 branches.
#7
Automatically closed -- issue fixed for 2 weeks with no activity.