Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
So even though you clicked to go on one of the child pages, you still know that you are in such and such a section because you have the css class so you can highlight it.
Comment | File | Size | Author |
---|---|---|---|
#51 | nice_menus-active-path_219804-51.patch | 4.04 KB | add1sun |
#33 | nice_menus-expanded-class-219804-33.patch | 4 KB | add1sun |
#28 | nice_menus-expanded-class-219804-28.patch | 3.93 KB | add1sun |
#12 | nice_menus-expanded-class-219804-12.patch | 3.04 KB | add1sun |
#9 | nice_menus-expanded-class-219804-9.patch | 3.03 KB | add1sun |
Comments
Comment #1
douggreen CreditAttribution: douggreen commented@arcX, this looks good, Thanks!
A few suggestions (new patch attached)...
$state = ($is_active) ? ' expanded' : '';
). However, I've reworked it even more, so that you avoid setting the extra variable that is only used once, and so that the assignment to '' isn't even done if the condition isn't true.Comment #2
add1sun CreditAttribution: add1sun commentedHey arcX, cool stuff. Thanks for getting this going. There is already an issue for this over at http://drupal.org/node/171693. Since this has movement and patches in it though, I'll dupe that one and we can keep moving here.
Also note that I am traveling for work and won't get to really look at this until mid-week.
Comment #3
alexiscott CreditAttribution: alexiscott commentedOk, so the code was taken from that page, 171693 :) so credit is to user ivan.g. All I did was create the patch, forget where I found the code, and start using it on a live site! But it works :)
Comment #4
add1sun CreditAttribution: add1sun commentedSetting this to needs review.
Comment #5
add1sun CreditAttribution: add1sun commentedI got a failure on the last patch so I rerolled against the 5 branch. The class addition looks fine but that makes the menus look screwy in core themes since they have their own styling for expanded (notably they have a graphic which pushes things over a bit.) We will need to also edit the Nice menus default CSS to override that so it doesn't look broken out of the box.
Comment #6
add1sun CreditAttribution: add1sun commentedThis patch needs to be redone against HEAD. I may backport to 5 but I'm not really keen on new features in 5 at this point.
Comment #7
edward.peters@drupal.org CreditAttribution: edward.peters@drupal.org commentedI am having this same problem with the module, and I cannot see how to apply this patch against the published version of the module, and I cannot find the dev version. Please could someone help me? Thanks.
Comment #8
add1sun CreditAttribution: add1sun commentedDev releases can be found in the full release list (http://drupal.org/node/43047/release) or in CVS. As mentioned above the patch needs to be rerolled for Drupal 6 so it won't work at all on any D6 version. As far as I can remember the D5 release (1.2) should be pretty much the same as the dev version for 5.
Comment #9
add1sun CreditAttribution: add1sun commentedA patch against HEAD along with the CSS cleanup.
Comment #10
add1sun CreditAttribution: add1sun commentedNeeds a reroll now that the JS patches have landed.
Comment #11
ddanier CreditAttribution: ddanier commentedsubscribing
Comment #12
add1sun CreditAttribution: add1sun commentedPatch wasn't that bad, just fuzz. Here is a new clean patch.
Comment #13
scottrigbyHi add1sun – I may be doing something wrong, but I don't see any class applied to the parent menu item when we're on one of the child pages?
I see 'menuparent' but this was already there (just shows that it's a parent, not that we're on the page of one of it's children).
Maybe I didn't apply the patch correctly, but I just downloaded HEAD, and the patch seemed to go through without a hitch...
I'm eager to test – today ideally – so I await any advice about what I may be missing?
Cheers!
:) Scott
Comment #14
add1sun CreditAttribution: add1sun commentedHrm, I'll try and get a look at it today. It should add an "expanded" class to the parent items.
Comment #15
scottrigbyCool - I'll be happy to test again - I'll keep an eye on this thread later today.
By the way, I'm testing 2 patches, but I have to test them seperately (this one, and the other patch here: http://drupal.org/node/135771 )
Earlier when I tried to patch one after the other (this one first) the otehr patch didn't work correctly – so I had to do on a new install... Is there anything I should know about patching so I can test them both together?
Thanks! – & will be back to check or updates in a few hours...
:) Scott
Comment #16
add1sun CreditAttribution: add1sun commentedHm, it is adding the "expanded" class for me on a local 6.3 install. When I select Administer > Site building > Blocks, I get things like
<li id="menu-17" class="menu-path-admin-build menuparent expanded">
in the parent menu items (that one is for Administer > Site building's li).Comment #17
scottrigbyOh! then I must have done something wrong ;) i will retry the patch and test...
Thanks for clarifying!
Scott
Comment #18
scottrigbytest confirmed for admin menu! (on new D6.3 install patched against HEAD)... I have to run out for a few hours but will be back to test with primary links then -- thanks add1sun :) Scott
Comment #19
scottrigbyHi add1sun –
Test Failed for Primary links... I don't get the 'expanded' class on parent menu items, only something like this
<li id="menu-122" class="menu-path-node-1 menuparent ">
.So works on admin menu
doesn't work on Primary links
Sorry to report this :(
Hope it helps pin down the problem a bit though?
:) Scott
Comment #20
add1sun CreditAttribution: add1sun commentedverified that it doesn't work on primary links. the menu_get_active_trail returns no menu trail info. No idea why right now and not sure when I'll get to actually look at it.
Comment #21
edward.peters CreditAttribution: edward.peters commentedGeeting the patch for the 'expanded' class is very important for me, but as I am not a developer :) could someone kindly tell me how to get a latest Dev version which has this class in the code? Thanks if possible.
Comment #22
add1sun CreditAttribution: add1sun commentedOK, I found out *why* now. #184955: menu_get_active_breadcrumb breaks breadcrumbs in primary links
So this is a core "by design" issue that I will need to work around. Thankfully stevedondley and Moonshine were working on this problem (not nice menus, but similar active trail problems) in #drupal when I happened to be there. Basically we'll need to do a menu_set_active_menu_name() before the menu_get_active_trail().
I'll play with this later this week and see if I can get a working patch.
Comment #23
scottrigbyGreat news! good luck with this... still on board to help test next patch ;)
Comment #24
Steve Dondley CreditAttribution: Steve Dondley commentedheh, I was actually trying to solve this problem. I got it to work! Patch coming later tonight.
Comment #25
add1sun CreditAttribution: add1sun commentedoh frickin sweet!
Comment #26
Steve Dondley CreditAttribution: Steve Dondley commentedHi, sorry for the delay in getting a patch up. Here is the full theme for now. Sorry about this but I am behind in some work and so this will have to suffice until I can create a proper patch or until someone else feels the urge to touch this up and roll the patch.
It has one problem in that the 'primary-links' string is hard coded into the code. This obviously has to change. Also, how the css class is added to the code is changed from the patch above. I did not add an "expanded" class as that seemed to introduce a bug (caused a little triangle to show up next to the list item). So I just put in an "active" for all list items that are in the active path. Seemed simpler that way. It works, I tested.
Comment #27
add1sun CreditAttribution: add1sun commentedHey thanks Steve! I'll get a patch rolled later today or tomorrow that addresses the menu name coming in dynamically.
Re: the class name, my earlier patch has the CSS correction needed to work with the expanded class. I'm still not sure if that is better or not. I think that the way core uses these is where "expanded" is for a parent in use and "active" is used for the menu item of the page itself when on that page. In those definitions the active trail should be using "expanded" and not "active." The other option, of course, is to simply create a new class to distinguish it altogether. I don't know if it is better to use core classes that folks may already have themed or just add our own new one. Opinions?
Comment #28
add1sun CreditAttribution: add1sun commentedBleh, i couldn't help poking at this and basically this doesn't work if you have more than one nice menu. The first nice menu name gets set as active and then that's it. It is a static var in menu_set_active_menu_name(). If we can't figure out how to "reset" the active menu name each time it is called then we may just need to write our own helper function to get the trail.
I've attached a patch of what we have so far.
Comment #29
vitalie CreditAttribution: vitalie commentedThanks for this. I have applied this patch yesterday. I found that some further changes are still needed.
1. The line in the patch which was
$class .= $children ? 'menuparent ' : '';
works better for me if it is$class .= $children ? ' menuparent' : '';
2. The line (a bit above that from point 1)
$children = theme('nice_menu_build', $menu_item['below']);
should take in the newly added function parameter, i.e. it should be$children = theme('nice_menu_build', $menu_item['below'], $menu_name);
Comment #30
add1sun CreditAttribution: add1sun commentedThanks vitalie, I'll reroll it after I commit the menu depth patch (which will happen soon).
Comment #31
wuf31 CreditAttribution: wuf31 commentedThanks, finally got it working..
Though, for performance I suggest
$trails_id
be set static.Comment #32
edward.peters CreditAttribution: edward.peters commentedThis patch is exactly what we have been waiting for! But since installing it with the two extra patches of the patch described by Vitalie above, drop downs don't work in IE7 at all. They are fine in Ffx. We have had a skilled CSS person look at it, and he thinks there must be an issue with the code, but we are stumped. Any ideas? I opened this as a separate issue at http://drupal.org/node/292483 but mention here in case it helps. Thanks for any advice...
Comment #33
add1sun CreditAttribution: add1sun commentedReroll with the depth patch in HEAD now and adding vitalie's changes. Untested. The patch definitely isn't optimized yet, but unless we can figure out the multiple menu issue, this patch isn't going in anyway. Just to be clear *this is not a fully working patch* and I am not sure this direction will work in the long run. I am more and more inclined to write a helper function that does this for us manually, since the API is not designed to do what we are trying to do here.
I don't know what about this would make things stop working in IE7 and this hasn't happened to me.
Comment #34
scottrigbyHi add1sun!
just making sure I understand where the progress is at with this issue...
Thanks for clarifying :)
Scott
Comment #35
add1sun CreditAttribution: add1sun commentedHey Scott, here is the run-down:
The depth patch has been committed to 6.x-2.x-dev (HEAD). So #33 is the only one that will apply cleanly now.
#33 should work for *one* nice menu. If you only use one nice menu, then it should be fine. If you use more than one, only one of the menus (the first one called by NM) will get the expanded class for its items. So, you can use it for now, but it is unacceptable to be committed to the module long term as is. Any solution that goes in will need to work will all NM blocks. I'm not sure that core API will accommodate that so we may need to write our own helper function to determine the trails.
If you only need one *specific* menu block to work for the time being you can use the patch and manually set the name of the menu you want to use. You can do that a number of ways but the simplest is to probably add a line in function theme_nice_menu_build() along with the patch.
In the function, before the line:
you can add:
Where primary-links can be changed to any menu name, which you can find in the
menu_name
column of themenu_custom
table in the DB.I will not be able to work on this any more until September, unless I manage to sit down with it while at DrupalCon Szeged. I'll be very busy there though, so I doubt much progress will be made until I get back.
Comment #36
edward.peters CreditAttribution: edward.peters commentedJust to say: we have got the patched, patched version to work (expanded class) in our dev site with seven nice menus, with no problem. But we still have not fixed the issue of drop downs not working in IE7! No doubt something with our theme/css because it works ok with Garland enabled.
Comment #37
scottrigbyHey add1sun – great, thanks for clarifying...
(hope u have a great time at Drupalcon!)
edward.peters – what do u mean by 'patched, patched version'? It'd be great if I could do with more than one menu... :)
Comment #38
edward.peters CreditAttribution: edward.peters commented@scottrigby: see comment #29 above by my developer Vitalie.
Comment #39
add1sun CreditAttribution: add1sun commentedJust to be clear, Vitalie's additions are now in patch #33, so that patch is complete.
@edward.peters, it doesn't work for me with multiple menus. that is to say that Nice menus itself works fine but the expanded class is only added to one of the menus and not to the others. Can you verify that multiple menus get the class and, if so, please provide a fresh patch with your local changes.
Comment #40
vitalie CreditAttribution: vitalie commentedYes, I think we (see comments #36 and #38) do not actually ever display two nice menus at the same time.
It seems to me that the problem with multiple menus is coming from
menu_set_active_trail()
function (called by the respective _get_ function) which has a static$trail
and when once generated (in the longest else clause in that function) never changes unless one specifically gives it another trail (but where to get it from in the first place, if not just from a replicated code?!). Practically, once a trail is set, it remains the same. This is why on the second and third, etc nice menus we are getting actually the trail from the first one. Should not this core function provide a way for us to tell it we want a new trail to be generated?A possible round-about solution follows, but let me first say that I think one more change is needed to the patch in comment #33. In the function
theme_nice_menu_tree()
just above the one we are talking about here, at the end (line 300, i think), the line$output['content'] .= theme('nice_menu_build', $menu, $depth);
should be$output['content'] .= theme('nice_menu_build', $menu, $depth, $menu_name);
(so with the $menu_name passed on).The possible round about: just add this at the beginning of the theme_nice_menu_build function:
Comment #41
brainski CreditAttribution: brainski commentedI had the same issue. The patch 33 worked for me.
Furthermore I added another functionality. I wanted to have the active menu point also highlighted. I got a working solution with the following code:
Comment #42
demenece CreditAttribution: demenece commentedHi, does this patch work for Drupal 5.x.1 versions?
Comment #43
add1sun CreditAttribution: add1sun commented@dcalderon, no it doesn't and this feature will not be added to the D5 version unless someone else writes a backport patch for it.
Comment #44
demenece CreditAttribution: demenece commentedwhat about the first version posted by ivan.g? (we've it here: http://drupal.org/node/171693#comment-599475 ) wasn't it working for D5? Maybe I can figure out how to make it a backport patch later... actually, i thought what douggreen upload in comment 1: http://drupal.org/node/219804#comment-723769 was a patch for D5, but i then read the following comments and they were all related to D6.
Comment #45
petertj CreditAttribution: petertj commentedHey, add1sun, thank you for pointing me here!
Once I got the css rule overrides figured out, the latest function from brainski works well for me on 6.4 - at least on nodes that are directly referenced by the menu.
You can see it working at http://atma.syn8.com/content/test-page - created placeholder submenus for testing nested menus, even though our intended heirarchy is only one deep.
I'll keep an eye on this thread to see if I can help further with testing any new revisions.
Comment #46
petertj CreditAttribution: petertj commentedMore on the subject of highlighting of indirect nodes:
We've been using http://drupal.org/project/menutrails to set rules for how menu .active classes get applied based on content type and/or taxonomy.
This had the highly desirable result that our taxonomy-based primary and secondary menus highlighted appropriately to show the context of nodes accessed indirectly via taxonomy or views.
You can see this operational on the secondary menu at: http://atma.syn8.com/media-forum/articles/brief-history-media
Think there's a way for Nice Menus to support Menu Trails inserting the .active class like it does on the core menus? Or maybe there's a better way to get the same result?
Comment #47
seutje CreditAttribution: seutje commentedawesome, looks like I'ma be hacking again :(
turns out it was rather easy
for drupal 5 version drop this in ur template.php:
it doesn't set and "active" class to the tag but the tag instead
still enough to distinguish a parent with an active child using ul.nice-menu li.active a { ... } and ul.menu li.inactive a {...}
Comment #48
jrabeemer CreditAttribution: jrabeemer commentedThe above code should be rolled into a patch and moved into its own issue.
Meanwhile, I like the idea of having a class set for the active parents and current active menu item. I had a client ask me today for this feature and I had to tell them no go for now...
Comment #49
johnpv CreditAttribution: johnpv commentedGood feature to have. Any idea when this will be incorporated?
Comment #50
MurzI try to use this patches on my site, but no one is worked for me. I try to get the active trail with another method and create a new patch. Patch is here: http://drupal.org/node/320588
I think that my method is more effective that using
menu_get_active_trail()
andmenu_in_active_trail
. And funcitonmenu_in_active_trail
is unavaliable in Drupal 6: http://api.drupal.org/api/search/6/menu_in_active_trail.Can we review my patch?
Comment #51
add1sun CreditAttribution: add1sun commentedI checked out Murz's patch in the other issue and it works really nicely. I cleaned it up some (style) and am closing the other issue as a duplicate (#320588: Patch to add 'in-active-trail' CSS class to each menu item that is in active trail of current menu). Other than style there are two things I changed:
1. I made the class name "active-path" and not "in-active-path".
2. I have commented out adding the class to the link itself and added code to make it put the class on the LI. The reason I did this is that that is where core puts that class, but I'm not married to it. I left the commented line in so that people can play with the patch both ways and give feedback on what is easiest to work with. Ideally I want to use the one where you can make decent CSS changes in the simplest way. One problem with it being on the link is that a bg color doesn't cover the whole LI. So, please play with both ways AND actually try to theme a menu the way you need it with both.
So, basically this patch works really well with multiple menus and the main thing (other than just testing to make sure it really works for everyone) is to figure out the actual class.
Thanks Murz!
Comment #52
add1sun CreditAttribution: add1sun commentedchanging title
Comment #53
brainski CreditAttribution: brainski commentedWhy is this patch not in the dev version? I tested it and it works.
Comment #54
add1sun CreditAttribution: add1sun commentedThanks for the test report. It isn't in the dev version yet because:
1) We need at least a few people to test to make sure it is good. So your report back helps with that. :-)
2) The patch isn't done yet. Point 2 in #51 needs to be resolved. When you tested it did you manage to theme the CSS OK? Did you try both versions of the class placement?
For clarity, the section of code I am talking about is in the theme_nice_menu_build function:
To play with both class placements, add the patch and then try to theme the CSS for the active-trail class the way you would like (this will be with the class on the LI element). Then switch the class placement used by uncommenting the $menu_item line and commenting the $class line, like so:
Now try to theme the CSS again the way you want it to look (this will place the class on the A element).
Which attempt at the CSS was more successful and/or easier to accomplish?
Comment #55
mrthrelfall CreditAttribution: mrthrelfall commentedI had themed nice_menus in Drupal 5 in order to provide the functionality delivered by this patch. I had called the class 'active-trail' and applied it on the LI elements in my Drupal 5 environment. I have now taken my Drupal 5 setup and migrated it to Drupal 6. I removed my custom theming, applied this patch and it worked as advertised without my needing to change any of my CSS. So this patch works perfectly for me in Drupal 6 and allows me to remove custom theming.
I think that applying the class on LI elements is better because you are able to style the A elements as in "li.active-trail a" in addition to the LI elements as in "li.active-trail' whereas you would not be able to style the LI element for the active path if the class is only applied to the A element.
Comment #56
add1sun CreditAttribution: add1sun commentedOK, I have committed this to HEAD (6.x-2.x-dev), setting the class on the LI. Enjoy.
Comment #57
alexiscott CreditAttribution: alexiscott commentedThanks! That's super cool.
Comment #59
Ne_L@drupal.ru CreditAttribution: Ne_L@drupal.ru commentedups
Comment #60
TravisCarden CreditAttribution: TravisCarden commentedWill this feature be back-ported to 6.1? If not, any idea when 6.2 might be production ready? Thanks so much for all the hard work!
Comment #61
Mark B CreditAttribution: Mark B commentedI've backported add1sun's patch (#51) to the 6.x-1.3 release of the module at http://drupal.org/node/465738
Comment #62
aipheshi
whats the way to target this css class (active-trail) to have this functionnality in nice menus ?
i ve tried with unsuccess...
thx
Comment #63
temp CreditAttribution: temp commentedfor highlighting active parent menus of selected item.
Comment #64
aiphesok i've solved my problem : http://drupal.org/node/614864