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));  
}
CommentFileSizeAuthor
term_display-theme_item_list.patch846 bytesjoachim
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

nedjo’s picture

nedjo
he/him/his
Primary language English
CreditAttribution: nedjo commented
Status: Active » Needs work

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:


$term_links[] = l($term->name, taxonomy_term_path($term), array('attributes' => array('class' => 'vocabulary-list')));
joachim’s picture

joachim CreditAttribution: joachim commented

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'));
nedjo’s picture

nedjo
he/him/his
Primary language English
CreditAttribution: nedjo commented
Status: Needs work » Reviewed & tested by the community

Sounds good, pls go ahead and commit that change.

NikLP’s picture

NikLP CreditAttribution: NikLP commented

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?

nedjo’s picture

nedjo
he/him/his
Primary language English
CreditAttribution: nedjo commented
Version: 5.x-1.0 » 6.x-1.x-dev

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.

joachim’s picture

joachim CreditAttribution: joachim commented
Status: Reviewed & tested by the community » Fixed

Committed to both HEAD and DRUPAL-5 branches.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.