Early Bird Registration for DrupalCon Portland 2024 is open! Register by 23:59 PST on 31 March 2024, to get $100 off your ticket.
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 :-)
Comment | File | Size | Author |
---|---|---|---|
#25 | theme.oops_.patch | 690 bytes | bdragon |
#23 | cleanup_theme_links_b.patch | 1.96 KB | dvessel |
#20 | cleanup_theme_links.patch | 1.96 KB | dvessel |
#12 | cleanup_theme.inc_.patch | 1.96 KB | dvessel |
#3 | li.patch | 1.05 KB | RobRoy |
Comments
Comment #1
webchickWorks for me.
Comment #2
robertDouglass CreditAttribution: robertDouglass commentedHoly crap.... I've never needed this, UNTIL 30 MINUTES AGO. Now there is a patch. The universe (or m3avrck) is looking after me.
Comment #3
RobRoy CreditAttribution: RobRoy commentedJust cleaned up whitespace and put a period at the end of the code comment.
Comment #4
m3avrck CreditAttribution: m3avrck commentedActually 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.
Comment #5
RobRoy CreditAttribution: RobRoy commentedI 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! ;)
Comment #6
Dries CreditAttribution: Dries commentedI don't understand why the strpos searches for ('-active') -- where does that string come from?
Comment #7
m3avrck CreditAttribution: m3avrck commentedComment #8
drummWhat about links not generated by the menu system?
Comment #9
drummWhat about links not generated by the menu system?
Comment #10
m3avrck CreditAttribution: m3avrck commentedYeah 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
Comment #11
dmitrig01 CreditAttribution: dmitrig01 commentedComment #12
dvessel CreditAttribution: dvessel commentedWould 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.
Comment #13
dvessel CreditAttribution: dvessel commentedOh, 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.
Comment #14
dmitrig01 CreditAttribution: dmitrig01 commentedMight need a re-roll but RTBC
Comment #15
webchickWe should not waste core commiters' time setting untested patches to RTBC.
Comment #16
dvessel CreditAttribution: dvessel commentedAye 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.
Comment #17
Bevan CreditAttribution: Bevan commentedsubscribing
Comment #18
eaton CreditAttribution: eaton commentedTested and verified as working with current HEAD. Nice cleanup of some of our ugliest selectors.
Comment #19
Gábor HojtsyWho 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.
Comment #20
dvessel CreditAttribution: dvessel commentedI 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.
Comment #21
dvessel CreditAttribution: dvessel commentedForgot to mention the spans look fine also.
Comment #22
Stefan Nagtegaal CreditAttribution: Stefan Nagtegaal commentedThis is ready! Very nice simplificitaion of the code..
Works as advertised and does not harm the way themes are presenting our drupal atm. Nice!
Comment #23
dvessel CreditAttribution: dvessel commentedI flipped the isset() check on $link['attributes'] for spans by accident.
Comment #24
Gábor HojtsyLooks better now, thanks! Committed!
Comment #25
bdragon CreditAttribution: bdragon commentedThis is causing warnings. It seems you are neglecting to check if href is set.
Comment #26
dvessel CreditAttribution: dvessel commentedWhoops, sorry about that. :)
Comment #27
Gábor HojtsyThanks, committed.
Comment #28
(not verified) CreditAttribution: commented