Function theme_menu_tree() provide standard ul that we can not give custom attributes.

CommentFileSizeAuthor
#12 menu_attributes_ul_attributes_2298033_12.patch6.1 KBijortengab
#7 menu_attributes.module.txt13.4 KBijortengab
#5 menu_attributes.module.txt13.58 KBijortengab
#2 ulattributes.patch12.14 KBijortengab
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

joelpittet’s picture

joelpittet
Primary language English
Location Vancouver
CreditAttribution: joelpittet commented
Related issues: +#1862048: Class attribute for entire menu
ijortengab’s picture

ijortengab CreditAttribution: ijortengab commented
Status: Active » Needs review
FileSize
ulattributes.patch12.14 KB

How that patch working...
theme_menu_tree doesn't have any information like menu machine name, plid (parent link id), etc ... it's confusing.
Therefore, we ride information about menu with SESSION and title.

ijortengab’s picture

ijortengab CreditAttribution: ijortengab commented

How to use..
set ul attributes in first link per depth
example: we have custom menu

<ul custom-attributes> <!-- set in "indonesia" link-->
    <li>
        <a>indonesia</a> 
        <ul custom-attributes> <!-- set in "jakarta" link-->
            <li>
                <a>jakarta</a>
                <ul custom-attributes> <!-- set in "jakarta-pusat" link-->
                    <li>
                        <a>jakarta-pusat</a> 
                    </li>
                    <li>
                        <a>jakarta-selatan</a> 
                    </li>
                    <li>
                        <a>jakarta-timur</a> 
                    </li>
                    <li>
                        <a>jakarta-utara</a> 
                    </li>
                    <li>
                        <a>jakarta-barat</a> 
                    </li>
                </ul>
            </li>
        </ul>
    </li>
    <li>
        <a>malaysia</a> 
    </li>
    <li>
        <a>singapore</a> 
    </li>
</ul>
joelpittet’s picture

joelpittet
Primary language English
Location Vancouver
CreditAttribution: joelpittet commented
Status: Needs review » Needs work
  1. I know your pain on the menu not getting any information. Though really uncomfortable passing variables around in session.
  2. You can likely tack on some of info you are passing through session to the ['#below'] of theme_menu_link, but that won't get the first level ul because it doesn't have a parent link to get it's info from.
  3. Seems like the default 'menu' class get's removed in your patch which may cause existing themes to break.
  4. Not sure what's up with that patch but maybe it's the wrong format? It doesn't apply with the patch command or git apply
ijortengab’s picture

ijortengab CreditAttribution: ijortengab commented
FileSize
menu_attributes.module.txt13.58 KB

alternative patch.

ijortengab’s picture

ijortengab CreditAttribution: ijortengab commented
Status: Needs work » Active

We need to save information about menu to passing in theme_menu_tree().
I suggested that we ride menu information on the #title in first item menu.
example:
before:
#title = 'Home';
after
#title = '[menu_attributes]6[/menu_attributes]Home';

and if you do not agree about passing variable to SESSION, we can still ride through #title
example:
#title = '[menu_attributes]6-' . serialize($array_attributes). '[/menu_attributes]Home';
how about my idea?

ijortengab’s picture

ijortengab CreditAttribution: ijortengab commented
FileSize
menu_attributes.module.txt13.4 KB

Alternative without passing to session

ijortengab’s picture

ijortengab CreditAttribution: ijortengab commented

i forget about default 'menu' class...
before return in function menu_attributes_menu_tree(), this code will be added:

  if (isset($attributes['class']) && !in_array('menu', $attributes['class'])) {
    $attributes['class'][] = 'menu';
  }
joelpittet’s picture

joelpittet
Primary language English
Location Vancouver
CreditAttribution: joelpittet commented

Can you do a regular patch file against the 7.x-1.x so we can review the changes against the 7.x dev code?

Detailed instructions are here: https://www.drupal.org/node/707484
My git patch workflow that may be easier to digest if you already know git to some extent is here: http://pittet.ca/drupal/sprint/patch

We aren't doing this token thing in the module yet: [menu_attributes]6[/menu_attributes] and it looks a bit suspect, TBH. I'd really rather not include this if we have to do session hacking because that will likely effect varnish or other caching mechanisms and #title feels like too much of a hack as mentioned in #6... It's a bit of an imperfect world the way theme_menu works but there are plenty of solutions in the issue queue for Drupal 8 improvements here. We've made it this far without this feature and honestly if it's that tricky to get this feature in, it may not be worth the hassle it could incur.

Thanks for the effort thus far @ijortengab and tenacity on this feature request.

ijortengab’s picture

ijortengab CreditAttribution: ijortengab commented
Status: Active » Closed (fixed)

:thumbs_up:
yes.. this issue just to share my idea how to hack menu_attributes to have features with ul attributes. a lazy code but.. it's work. :-)

ijortengab’s picture

ijortengab CreditAttribution: ijortengab commented
Status: Closed (fixed) » Closed (won't fix)

miss..

ijortengab’s picture

ijortengab CreditAttribution: ijortengab commented
FileSize
menu_attributes_ul_attributes_2298033_12.patch6.1 KB

fyi:
this link explain why my patch error..
http://stackoverflow.com/questions/13675782/git-shell-in-windows-patchs-...
i upload new patch and it should be fine... just for reference.

joelpittet’s picture

joelpittet
Primary language English
Location Vancouver
CreditAttribution: joelpittet commented

Oh wow that is good to know about the patch error, I don't use windows but when I must knowing these little gotchas are invaluable. thanks @ijortengab!

Dreditor is making that patch look much better to read:) http://dreditor.org/