Use both first and last class for single-item menu item. [#393632]

Comment	File	Size	Author
#11	drupal-single-menu-item-both-first-and-last__d6.patch	1001 bytes	andypost
#9	drupal-single-menu-item-both-first-and-last_d7_v4.patch	986 bytes	andypost
#6	drupal-single-menu-item-both-first-and-last_d7_v3.patch	782 bytes	ao2
#4	drupal-single-menu-item-both-first-and-last_d7_v2.patch	634 bytes	ao2
	drupal-single-menu-item-both-first-and-last.patch	677 bytes	ao2

Comment #1

Dutch

🇧🇪 (Ghent)

commented 13 May 2009 at 12:30

Title:	Use both first and last class for single menu items.	» Use both first and last class for single-item menu item.
Version:	6.10	» 7.x-dev
Category:	feature	» bug
Status:	Needs review	» Needs work
Issue tags:		+Quick fix

Ran into the same issue today. I'd classify this as a bug. If a menu item is the first, it should have the class "first", if it's the last item it should have a class "last", and if it's the only item, it should have both classes.

Current practice is to fix this in 7.x-dev first though, and then backport it to D6.

Not sure about:

$extra_class .= ' last';

Feels a bit messy since $extra_class might be NULL and the space is only required in very specific circumstances (single-item menu) anyway. What about treating $extra_class as an array:

    $extra_class = array();		
    if ($i == 0) {
      $extra_class[] = 'first';
    }
    if ($i == $num_items - 1) {
      $extra_class[] = 'last';
    }
    $link = theme('menu_item_link', $data['link']);
    if ($data['below']) {
      $output .= theme('menu_item', $link, $data['link']['has_children'], menu_tree_output($data['below']), $data['link']['in_active_trail'], implode(' ', $extra_class));
    }
    else {
      $output .= theme('menu_item', $link, $data['link']['has_children'], '', $data['link']['in_active_trail'], implode(' ', $extra_class));
    }

Log in or register to post comments

Comment #2

ao2 commented 18 May 2009 at 08:27

Yes that will work as well, or if we want to use the same style used in theme_item_list, we could do:

Index: drupal/includes/menu.inc
===================================================================
--- drupal.orig/includes/menu.inc
+++ drupal/includes/menu.inc
@@ -753,7 +753,7 @@
       $extra_class = 'first';
     }
     if ($i == $num_items - 1) {
-      $extra_class = 'last';
+      $extra_class = empty($extra_class) ? 'last' : $extra_class . ' last';
     }
     $link = theme('menu_item_link', $data['link']);
     if ($data['below']) {

Regards,
Antonio

Log in or register to post comments

Comment #3

mr.baileys

Dutch

🇧🇪 (Ghent)

commented 18 May 2009 at 09:08

That looks cleaner than my solution. Would you mind rolling a new patch?

Log in or register to post comments

Comment #4

ao2 commented 18 May 2009 at 09:41

Status:

Needs work

» Needs review

Status	File	Size
new	drupal-single-menu-item-both-first-and-last_d7_v2.patch	634 bytes

Ok,

I rebased the code above to apply cleanly on drupal7, the patch still applies to drupal6 tho.

Regards,
Antonio

Log in or register to post comments

Comment #5

18 May 2009 at 10:00

Status:

Needs review

» Needs work

The last submitted patch failed testing.

Log in or register to post comments

Comment #6

ao2 commented 18 May 2009 at 13:09

Status:

Needs work

» Needs review

Status	File	Size
new	drupal-single-menu-item-both-first-and-last_d7_v3.patch	782 bytes

Mmh, the System Message says Failed: Failed to apply patch....

I guess it's because the patch I sent was made with quilt which put also the parent directories in paths (which, in general, is good IMHO), let's try with a 'p0' patch as it expects.

Regards,
Antonio