This patch adds in an additional "active" class to the LI item when using theme_links. This is very useful for themers since right now there is only an "active" class on the A tag, not on the LI. This is useful for using various CSS tricks to make better looking menus.

Credit for this idea goes to Nicolas from Ourbrisbane.com :-)

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

webchick’s picture

Status: Needs review » Reviewed & tested by the community

Works for me.

robertDouglass’s picture

Holy crap.... I've never needed this, UNTIL 30 MINUTES AGO. Now there is a patch. The universe (or m3avrck) is looking after me.

RobRoy’s picture

FileSize
1.05 KB

Just cleaned up whitespace and put a period at the end of the code comment.

m3avrck’s picture

Actually the period was left out to be consistent with the rest of the comments in that function and file.

I suggest another patch to add in periods everywhere to be fully consistent -- this original patch was to be consistent with the rest of the coding style in the function.

RobRoy’s picture

I think if we're making additions we shouldn't perpetuate incorrect coding style just to be consistent with surrounding code. This way will make any future "cleanup" patches just that much easier to write.

No big deal either way, although the line of whitespace should still be removed in the original. Good patch regardless! ;)

Dries’s picture

I don't understand why the strpos searches for ('-active') -- where does that string come from?

m3avrck’s picture

 * @return A nested array of links and their properties. The keys of
 *   the array contain some extra encoded information about the results.
 *   The format of the key is {level}-{num}{-active}.
 *   level is the depth within the menu tree of this list.
 *   num is the number within this array, used only to make the key unique.
 *   -active is appended if this element is in the active trail.
 */
function menu_primary_links($start_level = 1, $pid = 0) {
drumm’s picture

Status: Reviewed & tested by the community » Needs work

What about links not generated by the menu system?

drumm’s picture

What about links not generated by the menu system?

m3avrck’s picture

Yeah I'm not sure -- I don't think there is a proper active class to base that upon, is there? Or maybe I'm too tired to think right now :-p

dmitrig01’s picture

Version: 5.x-dev » 6.x-dev
dvessel’s picture

Title: add additional class to LI in theme_links » add active class to theme_links and cleanup anchor classes.
Status: Needs work » Needs review
FileSize
1.96 KB

Would this work? Adds the active class but it also remove the redundant classes passed to the links. Completely useless.

To supply custom attributes for the links, it's easy enough to assign it through the $links array. The keys are still converted to classes for list items.

Now only if we could kill that active class for l(). Minor but annoying.

dvessel’s picture

Oh, and if anyone's wondering why not just pass the active class from the key. Well, it's not connected to any single function. Trying to construct it in all the other functions just means duplicate code.

dmitrig01’s picture

Status: Needs review » Reviewed & tested by the community

Might need a re-roll but RTBC

webchick’s picture

Status: Reviewed & tested by the community » Needs review

We should not waste core commiters' time setting untested patches to RTBC.

dvessel’s picture

Aye dmitrig01, how about testing it out. ;)

I admit I didn't check all the style sheets to see if any of them connects to the anchor styles instead of the li styles. I'll double check and up another patch if necessary.

Bevan’s picture

subscribing

eaton’s picture

Status: Needs review » Reviewed & tested by the community

Tested and verified as working with current HEAD. Nice cleanup of some of our ugliest selectors.

Gábor Hojtsy’s picture

Status: Reviewed & tested by the community » Needs review

Who checked how themes are affected? Seems like we remove classes from the link/span, and those might be built upon in themes. I completely understand adding the same classes to the list items and links makes no sense.

Otherwise I see no show-stopper here not to commit the patch.

dvessel’s picture

FileSize
1.96 KB

I scanned through all the style sheets and this patch does not affect any of them. Anything targeting the links would be using "a.class" and I found none that would be affected.

There are a handful of styles using the "active" class for the anchors but l() adds them automatically.

Scanned through each call of theme('links') also to double check. It all looks fine on this end.

New patch to remove hunk offsets.

dvessel’s picture

Forgot to mention the spans look fine also.

Stefan Nagtegaal’s picture

Status: Needs review » Reviewed & tested by the community

This is ready! Very nice simplificitaion of the code..

Works as advertised and does not harm the way themes are presenting our drupal atm. Nice!

dvessel’s picture

FileSize
1.96 KB

I flipped the isset() check on $link['attributes'] for spans by accident.

Gábor Hojtsy’s picture

Status: Reviewed & tested by the community » Fixed

Looks better now, thanks! Committed!

bdragon’s picture

Status: Fixed » Needs review
FileSize
690 bytes

This is causing warnings. It seems you are neglecting to check if href is set.

dvessel’s picture

Status: Needs review » Reviewed & tested by the community

Whoops, sorry about that. :)

Gábor Hojtsy’s picture

Status: Reviewed & tested by the community » Fixed

Thanks, committed.

Anonymous’s picture

Status: Fixed » Closed (fixed)