Please, please could you add a radio button which gives you a choice of attaching the attribute to either the <li> tag or the <a> tag - it would be fabulous

Files: 
CommentFileSizeAuthor
#38 menu_attributes_module.txt11.87 KBckt.orange
#38 menu_attributes_api.txt1.55 KBckt.orange
#26 menu_attributes-empty-fieldset-26.png77.77 KBandrewmacpherson
#25 menu_attributes-attributes_for_li-1488960-25.patch7.37 KBandrewmacpherson
PASSED: [[SimpleTest]]: [MySQL] 108 pass(es).
[ View ]
#24 menu_attributes-attributes_for_li-1488960-23.patch7.37 KBandrewmacpherson
PASSED: [[SimpleTest]]: [MySQL] 108 pass(es).
[ View ]
#2 menu_attributes-attributes_for_li-1488960-2.patch7.41 KBmichaek
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch menu_attributes-attributes_for_li-1488960-2.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
#2 Screen Shot 2012-06-19 at 12.05.08 PM.png40.81 KBmichaek

Comments

I agree that it's important to be able to set attributes on the list item. However, several of the attributes can only be applied to an anchor: target, relationship, access key. It probably makes more sense to provide a separate fieldset for list item attributes.

Title:<li> <a>Attributes for LI element
StatusFileSize
new40.81 KB
new7.41 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch menu_attributes-attributes_for_li-1488960-2.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

I'm providing a patch here that adds a 'scope' key for attribute info definitions, allowing attributes to be used for anchors, list items, or both. A fragment of the UI is represented in the attached screenshot. Sensible defaults are provided for the existing attributes.

I wasn't able to add tests supporting this feature, because I couldn't get the tests to run at all. If a maintainer is able to run the tests, perhaps they could add a few to provide coverage.

I just tested the patch and it works great, exactly what I needed. This is really great for theming an entire menu trail and only having to enter a class in the UI once so it's a bit more extensible. Thank you.

And my first patch ever...WORKED! Thanks so much @michaek !

Can anyone share how I know if this patch gets applied to the menu attributes module? In other words, what's best practice for dealing with a patched module? How do I make sure it doesn't get updated without the patch?

One thing I just noticed yesterday while theming a new design for a client with this patch is that the new features (class, id etc.. added to the menu's <li> tag) did not work with the Drupal Superfish module whereas custom classes added to a menu <a> tag did.

To test, I went back to using the main menu from the theme settings and sure enough the <li> tag's custom classes I added showed up. I know that's probably an edge case anyway. In the end, I got rid of the Superfish module and used the Menu Block Module to render my entire main menu tree and then manually added in the Superfish script and this worked great, all my custom classes were still there that I added to the <li> tag.

Status:Active» Needs review

Tested #2, worked like a charm, thanks !

Status:Needs review» Needs work

The last submitted patch, menu_attributes-attributes_for_li-1488960-2.patch, failed testing.

Version:7.x-1.0-rc1» 7.x-1.x-dev
Status:Needs work» Needs review

Wondering why the patch is failing.

The patch is tested on the master branch, which contains only three files: menu_attributes.info menu_attributes.install menu_attributes.module.

The file patch the file menu_attributes.api.php which is not present, therefore the test fail.

Also, i think that menu_attributes.test should be updated according to the new feature provided by the patch of Michaek

But, if you read the log, it * supposedly* checks out the right branch, no?

...
Main branch [7.x] checkout [complete].
...
Dependency branch [7.x-1.0-rc1] checkout [complete].
...
git apply --check -p1 /var/lib/drupaltestbot/sites/default/files/review/menu_attributes-attributes_for_li-1488960-2.patch 2>&1] failed

Oh yes you're right, but i see also:

Command [git clone 'git://git.drupal.org/project/menu_attributes.git' '/var/lib/drupaltestbot/sites/default/files/checkout/sites/default/modules/menu_attributes' --reference /var/cache/git/reference 2>&1] succeeded.

I cloned this repository, and it contains only three files.

Status:Needs review» Reviewed & tested by the community

Test seems now to be ok.

Set this to RTBC, feel free to change the status if needed

Using a custom menu module, like Superfish, options on LI are not applied.

I think because Superfish rewrite all LI classes, any ideas?

@H-BES - see my comment above (#5) in regard to the Drupal Superfish module (if that's what you are using). I don't think it's an issue with Menu attributes, it's Superfish most likely causing the problems.

I've patched the module, all ok. But it doesn't seem to do anything. I can add a class and ID to the menu item but they don't show up in the source code. Anyone any thoughts on this?

ID: list-item_1
Classes: active

<div id="main-menu" class="navigation">
<h2 class="element-invisible">Hoofdmenu</h2>
<ul id="main-menu-links" class="links clearfix">
<li class="menu-843 first"><a href="/~deb64479/nl/inschrijven" title="Subscribe" id="menu_2">Inschrijven</a></li>
<li class="menu-782"><a href="/~deb64479/nl/tourinformatie" title="Information" id="menu_3" class="active-trail">Tourinfo</a></li>
<li class="menu-772"><a href="/~deb64479/nl/nieuws" id="menu_4">News</a></li>
<li class="menu-818"><a href="/~deb64479/nl/fotos" title="Photo's" id="menu_5">Foto's</a></li>
<li class="menu-885 last"><a href="/~deb64479/nl/geschiedenis" id="menu_6">Geschiedenis</a></li>
</ul>
</div>

Just tested #2 patch on 1.0-rc2 and it works great. Hope to see this incorporated into the module!

Oh, I will also mention that from my experience, applying the CSS to the li for hover events, is the only way you can keep from breaking older browsers. This is another reason to make sure this ability is included.

Older browsers don't recognize stuff like:

a.menu-01:hover

The only thing the older browsers DO recognize is:

li.menu-01 a:hover

So, if the class is applied to the anchor, you can't do on hover stuff... I haven't messed with the active class it, but I would think it would make it difficult too since that class is applied to the anchor itself.

Thx for the #2 patch! I'd be pulling my hair out otherwise : )

Just applied the patch and can confirm that it's not working with the Superfish module.

subtheming AdaptiveThemes and applied the #2 patch against 7.x-1.0-rc2 and 7.x-1.x-dev and can confirm it's NOT WORKING. switched to Bartik and still NOT WORKING. pls advise. Thanks!

Cleanly applied patch in #2 to 7.x-1.0-rc2. Moved a class from the a tag to li, worked as expected.

test@prod01:~/domains/test.com/current/sites/all/modules/contrib/menu_attributes$ wget http://drupal.org/files/menu_attributes-attributes_for_li-1488960-2.patch
--2013-04-09 13:33:35--  http://drupal.org/files/menu_attributes-attributes_for_li-1488960-2.patch
Resolving drupal.org... 140.211.10.16
Connecting to drupal.org|140.211.10.16|:80... connected.
HTTP request sent, awaiting response... 200 OK
Length: 7588 (7.4K) [text/plain]
Saving to: `menu_attributes-attributes_for_li-1488960-2.patch'
100%[===================================================================================================>] 7,588       --.-K/s   in 0.05s  
2013-04-09 13:33:35 (160 KB/s) - `menu_attributes-attributes_for_li-1488960-2.patch' saved [7588/7588]
test@prod01:~/domains/test.com/current/sites/all/modules/contrib/menu_attributes$ git apply -v menu_attributes-attributes_for_li-1488960-2.patch
Checking patch menu_attributes.api.php...
Checking patch menu_attributes.module...
Applied patch menu_attributes.api.php cleanly.
Applied patch menu_attributes.module cleanly.
test@prod01:~/domains/test.com/current/sites/all/modules/contrib/menu_attributes$

Status:Reviewed & tested by the community» Needs work

The last submitted patch, menu_attributes-attributes_for_li-1488960-2.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new7.37 KB
PASSED: [[SimpleTest]]: [MySQL] 108 pass(es).
[ View ]

Following Commit ed7e9aa, the patch in comment #2 no longer aplies cleanly against against 7.x-1.x-dev

Re-rolling patch...

StatusFileSize
new7.37 KB
PASSED: [[SimpleTest]]: [MySQL] 108 pass(es).
[ View ]

Whoops, incorrect patch numbering.

Status:Needs review» Needs work
StatusFileSize
new77.77 KB

When ID, Classes and Styles are all disabled in admin/structure/menu/settings, there's an empty collapsible fieldset. Screenshot attached.

Would be good to update the options in admin/structure/menu/settings, so that ID, Class and Style can be enabled for either/both the LI and A elements.

e.g. under Class, have two check boxes: Enable class attribute for Menu Item, and Enable class for Menu Link

Item ID, Class and Style don't get applied to to primary links, but do get applied to menu blocks.

#25 worked great for me. Thank you so much andrewmacpherson! Also, I'd like to note that I had to clear all caches to get it to work, in case anyone has issues.

Status:Needs work» Needs review

Hello I applied #25 but it seems to not working.

New fields appeared on the admin side but no callses added for the li tags in menu.

I tried other solutions (b adding code in template.php and of course the patch in this page but nothing appeared.
I'm also using JQuery Menu, do you think there can be a problem with this module?

Thanks in advance

The patch is working for me. Just don't forget to clear your caches because its a new preprocess function that's added to the module trough the patch.

Can we clear the appropriate cache during a hook_update_N() function for users who are upgrading?

Works fine.

Thanks!

Status:Needs review» Reviewed & tested by the community

Patch #25 works fine, please port it into a new release.

Thanks.

+1 for patch #25 to new release

Once caches were cleared, it seems to work well!

StatusFileSize
new1.55 KB
new11.87 KB

I patched as #25.
But still can't assign a class/id to the li tag of the menu.

Here's attache the module and api file patched.
Thank you

Another confirmation that #25 works. Thank you, Andrew!

ckt.orange, are you sure you cleared *all* caches after patching #25? (Go to /admin/config/development/performance and click "Clear all caches".) Or maybe you mistook the new "Menu item attributes" collapsible fieldset that the patch creates for the standard "Menu link attributes" collapsible fieldset?

And yet another confirmation that patch from #25 works great, even using PostgreSQL, many thanks @andrewmacpherson. If you're having problems with it, make sure you're using the DEV version, and remember to clear caches.

Guess what... #25 works for me too! Let's have it commited! Thanks!

Issue summary:View changes

I had to change the special html characters in order for the tags in the message to read properly

thanks @andrewmacpherson for #25!

Issue summary:View changes

Can I find the patch included in any release?
Thanks

RTBC++ #25

Patch #25 worked for me as well (after clearing all caches)

Patch #25 Another confirm, working great.