| Project: | Context |
| Version: | 7.x-3.x-dev |
| Component: | Code |
| Category: | feature request |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | needs work |
| Issue tags: | Debut enabler |
Issue Summary
Problem/Motivation
The current menu context reaction, which allows setting the active class on a given menu item, is limited in that it doesn't affect inheritance. For example, if there is a contextual menu set to display depending on the active menu item, that menu won't display based on this context reaction.
Ideally, we would have an additional context reaction that actually adds the current menu item to a given menu trail. Doing so would trigger menu inheritance beyond simple CSS display.
Proposed resolution
The current patch provides a new context reaction that allows the site admin constructing a context to select a menu item. In the reaction, the current menu item is dynamically assigned the path of the the selected menu item.
Remaining tasks
- The current patch is a workaround that accomplishes part of the desired result. However, it would be preferable if there were a way to actually assign the selected menu item as the parent, rather than rewriting the current menu item as if it had the parent's path. This appears to be possible now due to the recent API additions in #520106: Allow setting the active menu trail for dynamically-generated menu paths.. The patch should be rewritten, likely using the new methods
menu_tree_get_path()andmenu_tree_set_path(). - Address the identified issue in #25:
This patch incorrectly sets the active trail condition when on node edit forms - if there are multiple contexts that specify 'set on node form' for a given content type, the active trail will be set on the first of these contexts alphabetically, regardless of the node type.
Can this bug be confirmed? Is there a fix?
User interface changes
Provides a new option in the list of context reactions.
API changes
Original report by rjacobs
Hi,
I've been looking through several notes about folks having problems getting context to set the "active menu" as a reaction. It seems that most of these boil down to people not using primary/secondary menus, displaying a menu through a block (instead of directly via the theme), etc. All this makes sense, as there is plenty of info about this floating around in other threads.
However, what I still can't seem to get to the bottom of is whether or not context is just setting the "active menu" (which I think would only impact the way menu items are rendered and their css classes) or if it's setting the "active menu trail" (which could be used internally by Drupal and other modules).
I apologize if "active menus" and "active menu trails" are not actually 2 different things, or if this question has already come up, but I have spent some time searching and can't seem to get to the bottom of things.
The reason I ask is because we are using the "menu block" module (who's visibility is dependent on the active menu trail), and it just does not seem to work properly with context.
The "menu trails" module has been our solution to date to set the menu trails of various content types, create site "sections", etc. It works great with the "menu block" module, but were hoping to replace it with context (as context is radically more feature rich).
Any insights are really appreciated.
Ryan
Comments
#1
Ok, upon a bit more searching, and what I can gather from #274577: Context doesn't expand menu's item of primary links block and #653698: Menu Trail Condition incorrectly displays depth it's starting to look like Context 2.x (and quite possibly 3.x) simply do have the ability to set the "active menu trail", and can only set the "active menu".
Can anyone confirm this 100% (the issues noted above focus on different problems, and only make reference to the concept of "active menu" vs "active menu trail", so I'm not sure if I am understanding correctly)?
If I am in fact correct, is there any chance that setting the "active menu trail" could eventually become an available reaction?
Cheers!
Ryan
#2
Wanted to adjust the title to make it clear that I am referring to the menu "reaction" and not the menu "condition" in this issue.
#3
I'm looking to replace my use of Menu Trails with context and the one missing piece is a reaction that sets the menu trail (as opposed to the active class).
Menu Trails (usage stats: over 8,000) allows designation of parent menu menu trails for node pages by node type and by taxonomy term. Typical use case: provide a persistent contextual sidebar menu (e.g., using Menu block). Menu trails more or less works, but lacks a lot of the flexibility that using Context would provide (and has significant outstanding bugs).
There would be several options for adding this functionality to context:
* Add into Context core as a new reaction.
* Add into Context core as a variation of the existing Menu reaction.
* Implement in a new contrib module.
* Implement in an existing contrib module, maybe Menu trails.
To get started, here's a patch that implements the first of these options, providing a new context reaction, "Menu trail."
#4
Previous version had superfluous code.
#5
See also http://drupal.org/project/context_menu_block.
#6
Wow, honestly, you have summarized out situation exactly nedjo. Exactly. In fact, the inability for menu block to integrate with context is the one thing that is keeping us from using context (and also phasing out menu trails)
You have outlined 2 direct solutions (your patch and "context menu block"). I'm going to test both in a current development space I am working on.
Personally I'd vote for this to be integrated into context (as I believe your patch addresses) as there may be other modules that use the menu trail as an input for various actions (in addition to menu block). If context is to be a nexus for information architecture and site sectioning, it would seem that this option would be valuable to bundle into the module itself.
Really, thanks do much for this!
Best,
Ryan
#7
Subscribe
#8
I've tried the patch from #4 against context 3.0-rc2 and it seems to work just fine. This means that with the "menu trail" reaction and the "breadcrumb" reaction I can achieve everything I was doing previously with the menu trails module.
I'm note sure (as I have not tested it), but I suppose this patch would also provide the functionality of http://drupal.org/project/context_menu_block (though via different means).
Thanks!
#9
This was posted to the Context: Menu Block issue queue here: #890584: Bundle this functionality in context itself?
To summarize my response there:
I don't think the issue Context: Menu Block solves would be solved by this patch. The original issue from which the module was borne (#751700: Setting active with Context Module) wasn't a confusion between active and active-trail: the use case for Context: Menu Block was that the active menu must be set, but Menu Block creates its own menu tree that's completely unaware of Context. Context: Menu Block merely alters the tree Menu Block creates to add the active class back in based on the current available contexts.
I think if you're going to combine functionality, it should probably be done in Menu Block, but if you wanted to somehow roll this into Context, telling people they need to redo their contexts (and themes) to use active trail rather than active menu is less than ideal. Rather, Context should implement
hook_menu_block_tree_alter()to set the active menu.But then we get to the question of whether Context should support every other contrib module under the sun. For that matter, should Menu Block have to account for every other contrib module? I don't think contrib modules should have to, which is why Context: Menu Block is being maintained (although I wouldn't complain if one did opt to support the other).
#10
Tracking.
#11
Re: #9, I don't think this feature is too contrib-specific to be in context. It's not really contrib-specific at all--as far as I can tell, the patch adds functionality that is responsive only to the core menu system. Also, I don't see why this patch means anyone would need to modify existing contexts or themes. It simply provides an additional feature; it doesn't change existing features.
I'll try upgrading to Context 3 to test this patch.
#12
Be sure to use the
-p0option when you apply this patch; otherwise the new plugin is created in the wrong directory.This patch (#4) meets my needs perfectly. (Wish I'd tested it yesterday before I spent hours trying to get this functionality a different way.) +1.
#13
xjm: yes, it would provide an additional feature, but it's also being contended that this patch obviates Context: Menu Block (see: #890584: Bundle this functionality in context itself?) which I do not believe it does.
#14
Here is a bit of what I have been able to distill (regarding Menu Block specifically):
There appeared to be at least 2 disconnects between Context and Menu Block:
From what I can see, Context: Menu Block fixes both of these, while the patch in #4 only fixes the 2nd one (via the menu trail). It also looks like the 1st issue is related to some of the concepts discussed in #442882: Document the menu links issue.
So it would seem that users who are only experiencing issues with Menu Block would be best serviced by Context: Menu Block
Regardless, the ability to set the menu trail as a reaction certainly has applications above and beyond just the use of Menu Block, and seems quite useful. I also like the way the patch in #4 renames the title of the reaction "Menu" to "Menu Class" as this helps provide clarification as to how this reaction differs from the additional "Menu Trail" reaction.
Cheers,
Ryan
#15
Re: #14 - Currently, the context_menu_block module does not wholly fix #2. So, this patch is still beneficial to Menu Block, even if it does not resolve #1. It also works well in combination with context_menu_block.module.
In any case, I think it is an extremely useful feature by itself. I'm already using it in five different contexts on my current site (only two of which are menu_block-related). I agree wholeheartedly with your summary of the patch.
#16
xjm, I tested a couple scenarios, and I was able to get a 2nd-level menu block to appear properly by only having a "menu class" reaction set the parent item of my 2nd-level menu branch as "active". I did this with context_menu_block installed and no modification of the menu trail.
Regardless, you said "does not wholly fix #2", so I suppose you may have a scenario that differs from anything I tested.
Anyway, I too prefer to address my issues by setting the menu trail (as opposed to using context_menu_block) as my theme also uses the menu trail for the styling of some menu buttons, and I don't have a need to fix issue #1 in my note above.
#17
Awesome, this just saved my day. Thanks!
#18
Patch at #4 works really well. It probably should be committed to module or build as a separate module.
#19
Tagging.
#20
I have primary links displaying above secondary links, which are the second level of the primary links.
If I use the menu class reaction I only get active related css on the second level and if I use the new menu trail reaction I only get active related classes on the first level.
If I use them both together I get what I want.
Is this how it is supposed to be?
#21
#5 the module fixed the menu trail problem when using menu block module with context module
#22
subscribing
#23
The current patch causes tab links to follow the active menu trail's setting instead of the node being viewed. So, for example, the edit tab will take you to the edit screen for the active menu trail instead of the edit screen for node being viewed. Since my use of this patch is confined to a context that is triggered solely by one content type, I did a hack in page.tpl.php to get the nid and and manually write the edit links for that content type.
I was thinking this might be an issue of module weighting? But I'm about 90% done with the site I'm working on and did not want to rock the boat :)
Just an FYI. If I have time to fix this, I'll post something. Or, is this intended behavior? (I hope not...)
jyg
#24
@jyg the patch following active-trail is intended. Context: Menu Block follows the active menu.
#25
This patch incorrectly sets the active trail condition when on node edit forms - if there are multiple contexts that specify 'set on node form' for a given content type, the active trail will be set on the first of these contexts alphabetically, regardless of the node type.
#26
please commit this to 6.x-3.dev
I do not want menu_block and menutrail or any other modules if i am using Context.
Patch from #4 helped me strongly. don't want to think about patching during each update.
#27
subscribing
#28
@karlos007, My guess is that the maintainers will not consider committing this patch until its status is set to "reviewed and tested". scottrigby flagged a problem in comment #25, and I think someone will need to look into that. I think I conceptually understand the problem outlined, but I have not encountered this scenario yet, and have not been effected by this problem.
I might be able to help look into the issue, but not for some time. Perhaps the patch creator (nedjo) has some comments here?
#29
subscribe
#30
Is there any technical reason why the functionality in Context Menu Block cannot be integrated directly into the Context module?
#31
@real_ate no, and speaking as the maintainer of Context: Menu Block, I'd actually rather prefer it.
But, as evidenced by how long this issue has been going on (and the issue that sparked the creation of Context: Menu Block, #751700: Setting active with Context Module), it was far easier to just make a separate integration project than it was to get code added to Context.
#32
had to add a (empty) function "condition_form" to context_reaction_menu_trail.inc to make the patch from #4 work on Context 6.x-3.0
Will this patch be integrated in the next release? Considering the functionality it offers, it would be great!
#33
subscribing.
better to commit it with this minor bug on node form then to patch each update...
#34
Tried this patch from #4 and got:
Call to undefined function menu_trail_options()
I can't find that function in any of the code I have...what am I missing?
#35
Subscribing, greetings, Martijn
#36
Patch in number 4 does not work for me on 6.x-3.0
Fatal error: require_once() [function.require]: Failed opening required './sites/biloba.aegirwip.praece.com/modules/context/plugins/context_reaction_menu_trail.inc' (include_path='.:/usr/local/lib/php') in /var/aegir/platforms/pressflow-6.20.97-dev/sites/biloba.aegirwip.praece.com/modules/ctools/includes/plugins.inc on line 747
#37
#36: See my comment in #12. You need to use the
-p0option for the file to be created in the correct location.#38
#37: where should this file have to be created. I try to manual patch and getting same fatal error as #36.
#39
subscribe
#40
Ultimately is the comment in #25 indeed what's keeping this from being again marked as "reviewed and tested" (and hopefully committed)? Or are these patching issues more than just isolated problems?
From what I can see the issue flagged in #25 may not be significant... or at least would need some more explanation (and specific steps to replicate) so it can be addressed.
It would just be great to see this adopted, as it certainly seems like a useful and in-demand feature. It would also be great to port a final version of this feature for Context in Drupal 7.
Would someone be able to provide a bit more clarity on #25 and what's missing?
Ryan
#41
subscribe
#42
sub.
#43
Same deal with us.
Trying to get context to play nice. Also looked at Menu_position which lights up the trail but no active state for the selected menu. That would be a nice option to select.
I would love to see context work without any additional modules or hacks.
#44
+1
#45
I had some problems using Context and Context menu block. In time before a solid solution is in place consider using Menu position, it did the trick for me :)
#46
The only ways I can replicate this issue is if a content type is told to set the active-path to two separate paths with no other arguments in the context. This doesn't seem to be the module's fault as it's been provided with bad input. The other way I was able to replicate this issue was to tell the content type to have to different active paths with an added dependency ( such as taxonomy terms ) to differentiate which active path to set and neglected to check off the all conditions required box. Unless I missed the specific use case I would say this patch functions as intended unless you want it to do error checking and to look for contradictions. But in the instance where there are two possible contexts for the path to be set it seems reasonable that the module would pick which ever one it sees first.
#47
That makes sense. Is there any reason not to incorporate the patch? It would be nice to have menu trails working finally.
#48
Subscribe - stumbling into the same issue in D7 with the Nice Menus module (embedded in the template via its' own theme function)
#49
+1
#50
Any movement on this? It sounds like _not_ adding the patch is a much greater problem than the small issue that it may introduce in some cases. Could the maintainers please speak to the current status of this issue and what they feel needs to be done in order to get this patch committed?
#51
Here's a new version of #4, just updated to be Git- and 7.x-3.x-compatible.
This solves an issue I was having that I don't think has been raised so far- my theme uses menu_tree_all_data() to build the menu, instead of $vars['main_menu'], so setting the active menu item in Context had absolutely no effect. Setting the active menu item using the new 'menu trail' plugin solves this. See #1098660: Megamenu: parent link not active when visiting child page for details.
#52
Whoops, forgot to add the menu trail plugin file to the patch, try this one...
#53
Hi Dane, thanks for your effort!
I tried your patch for this scenario: I have one panel that should work in two languages, which it won't due to this bug: #609542: Active trail isn't set on all menu items pointing to the current path - so, I want to set the active trail via Context using path + language as conditions, and menu trail as reaction.
What I get with this patch that I didn't have before is that I get the class "active" on the parent menu item, but nothing more.
What I expected from a set active trail are the classes "active-trail" and "active" (and probably "expanded") on the actual menu item I am on, and the class "active-trail" on the parent item. I believe these are the classes Drupal should respond with if the active menu trail is correctly set?
#54
@vegardjo - sorry, I have only a very limited understanding of the menu / theme system, so I can't really answer your question - all I really did here was to update and apply an earlier patch :)
#55
@Dane - ok, so have I :)
However, my understanding is that "setting the active trail" is more than adding classes to menu items, it's something Drupal does with the menu_set_active_trail function (http://api.drupal.org/api/drupal/includes--menu.inc/function/menu_set_ac...), and classes are added to the active menu item and parents due to that. And this is also what I hope could be accomplished via Context, but I have no idea if that is possible or out of scope for Context.
The way to control this (UI wise) would probably not be to specify the "active trail" as such, but specifying the active menu item would result in a valid active trail.
#56
of course it is not about classes only. For example you need it to have menu items expanded.
#57
There was a little error in file context.plugins.inc. Two times: $registry['reactions']['menu']. So only the menu trails functionallity survives.
#58
I updated the issue summary, noting outstanding concerns (including one I added).
#59
This patch is not the right way. As stated in #55 is will only set an active trail by setting classes. But it won't trigger - for example - a menu_block to show up which was configured as a level 2 (submenu) block and you selected a menu item at that level to be active for a specific node type. This is a very common usecase for nodes who don't have menu items but need to have an active submenu shown which is rendered through menu_block.
I think the solution is to use the new function (as from 7.9) menu_set_active_trail:
http://api.drupal.org/api/drupal/includes--menu.inc/function/menu_set_ac...
Testing this in a custom module at hook_init level works exactly as expected.
However, I wasn't able to use this function in context_reaction_menu.inc because (I think) it's fired too late? Is there a way to do this in context at an earlier stage?
#60
See also: #942782: Custom menus never receive an active trail.
#61
I'm not sure if that's really related (I'm using a core menu) but I think it should be able to fix this easily with menu_set_active_trail,
see also a recent patch in the menu_position module which commited this to fix a similar problem:
#979464: Use menu_tree_set_path() to set the active trail of menu trees
#62
@askibinski: Thanks for the issue link, that indeed looks like what's needed. I've updated the "Remaining tasks" summary accordingly.
#63
Any updates on this?