Since OG 7 makes heavy use of Panels I think a cTools content type would be very handy to use for rendering the menu in a pane.

By having this functionality as a cTools content type the settings for the pane is exportable. The setting for OG Menu that is link menu title to group node is now optional per pane. By having the group context configurable it makes the settings very flexible.

After a IRC chat with rv0 this functionality should be added as a submodule so that OG Menu does not add more dependencies than it has to. The supplied submodule adds a cTools content type that has settings for

  • Provided context "Organic groups membership: Group gid"
  • Link the pane title to the group node
  • Override title text (standard)
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

pontus_nilsson’s picture

Supplied file provides the submodule as a patch instead of the archive in the post above.

rv0’s picture

haven't tested yet and won't have the time to do so soon, but if people want this they should help out by testing it and posting feedback in here!

just did a quick read of the patch, some minor comments:
- isn't panels needed as a dependency?
- please try to follow the coding standards a bit more.. Like place a "dot" after a comment.
- empty lines should be empty, i spotted at least one which has spaces
- when placing comments in array definition, please place them after the relevant key=>val unless it's really long
- i've never seen ctools spelled like cTools before. be consistent.

it seems like a nice addition to OG Menu, hope to get some feedback from users.

teekay78’s picture

I tested the patch using og_menu from 11/10/26.

It works fine, as long you have only one single menu in the group.
og_menu allows multiple menus per group. As soon you add a second menu, the menu pane disappears.

Thanks!

pontus_nilsson’s picture

New patch with fixes from comments in #2.
Regarding #3. I guess it will be hard to determine which menu should be rendered. Should we have a setting render the n:th menu in the group or should it be render first found menu or render all menus?

rv0’s picture

Nice to see this is still going on.
About the multi menu feature:
I really dont get the point of the legacy OG Multi menu thing, I don't like how it currently works.
It seems to merge the links and places the title of the current node on top. Seems "weird" to me.
Haven't used it myself until I got a patch for it recently. Don't really know what to do with it.

fabsor’s picture

Why not make the plugin add multiple content types, one for each menu in each group? that way, you will be able to select the appropriate menu based on the current context.

Edit: Hmm... I guess we still would have to have a "N:th menu in group" kind of pane if we want to be able to create new menus without changing panels though.

pontus_nilsson’s picture

New patch renders through menu_block with settings for depth.

rv0’s picture

A quick look at the patch, I see $form['depth'] setting for max depth of the menu tree
afaik, drupal limits menu depth to 9: http://api.drupal.org/api/drupal/includes%21menu.inc/constant/MENU_MAX_D...
maybe best to use that constant and the range function to create the array: http://www.php.net/manual/en/function.range.php

Why the menu_block dependency? what does it do?

pontus_nilsson’s picture

I was thinking menu_block does some pretty cool stuff. For example only render first level of the menu in the first pane, render the second level starting at the active menu trail etc.

pontus_nilsson’s picture

Screwed up the last uploaded patch, new file comes here.

pontus_nilsson’s picture

New patch provides some sane defaults if they are not set in the pane.

gdl’s picture

Thanks for the patch! Two minor suggestions on the comments:

+/**
+ * Implementation of hook_ctools_plugin_dierctory() to let the system know
+ * where our content_type plugins are.
+ */

1) "dierctory" should be "directory"
2) As I understand them, the current comment style guidelines suggest that the first, and probably only, line in the comment block should be:

/**
 * Implements hook_ctools_plugin_directory().
 */
 

I can't comment on how well the code works, however, since I haven't actually tested it.

Cheers,
-G

denny84’s picture

Where is the function og_menu_get_menus(array($gid)); coming from?

denny84’s picture

AAAAAAh ....... this is 2.x ...... for 1.x its og_menu_get_group_menus(array($gid));

rv0’s picture

@ denny84
that function was renamed, see #793854: og_menu inadvertently implements hook_get_menus()

denny84’s picture

@rv0 You are right. I didnt know that.

fabsor’s picture

Why do we need to create a new module for adding this functionality? CTools is plugins are only loaded if the proper modules that define the plugin types are enabled, so I don't see why we couldn't just add the plugins directly to the main module. You don't have to add panels and CTools as dependencies, the plugin just isn't loaded if they are not enabled.

rv0’s picture

if that is indeed the case, an include file according to whatever the standard is for ctools would be enough

rv0’s picture

Status: Needs review » Needs work

RE: #17
If anyone can provide a clean patch without the new module, I will commit it.
Same goes for #1871390: ctools content type plugin for OG Menu 3.x

neclimdul’s picture

Title: Provide cTools content type for OG Menu » Provide ctools content type for OG Menu

After reading the patch I was about to echo #17 but I see everyone is on the same page. Title was bothering me though.

bkildow’s picture

Version: 7.x-2.x-dev » 7.x-3.x-dev
FileSize
6.79 KB

I'm merging #1871390: ctools content type plugin for OG Menu 3.x into this one as suggested. Patch is attached and is not using a separate module as suggested in #17. This is for 7.x-3.x version of Og Menu.

bkildow’s picture

Status: Needs work » Needs review
bkildow’s picture

I've attached a new patch fixing some white spaces.

zipymonkey’s picture

I installed patch #23 and used it to a add og_menu content to a panelized group node. The menu show up properly but the following error was displayed. I'm getting the context from the group node.

Notice: Undefined property: stdClass::$etid in og_menu_pane_content_type_render() (line 73 of /Users/zipymonkey/Sites/acquia_d/sites/og.localhost/modules/og_menu/plugins/content_types/og_menu_pane_content_type.inc).

bkildow’s picture

New patch attached that should address #24.

rv0’s picture

A quick glance at the latest patch:

Why the menu block dependency? Please remove.

Patch does not follow drupal coding standards for several reasons:
- comments (esp for hook implementations)
- spacing in if statements not always correct

rv0’s picture

Issue summary: View changes

Adds cons for having this feature.

heyyo’s picture

Any chance to see this patch commited ?

rv0’s picture

Issue summary: View changes
Status: Needs review » Needs work

As stated in #26, this still needs work

heyyo’s picture

Is the menu block dependency should still be removed ?
I just retest this patch which works really nice inside Panels.
Menu Block gives the possibility to display only part of a menu with settings of level + depth.

I add for my need a "menu_index" settings in the pane to be able to display any menu on my group panel page.
In my case I need one menu for navigation and another one in the footer as explained in issue #2179789: Howto display several menus on group page

I also needed to add a sort by title in function og_menu_group_menus() to reflect the same order used on the page group/node/XXX/admin/menus

I tried to correct the code according to Drupal standard.

Please my first Drupal patch attached

rv0’s picture

I don't see why the menu_block dependency is there, when I check the patch I see some duplication from menu_block, but no menu_block functions.. Correct me if I'm wrong.

heyyo’s picture

This line permits to render only part of a menu:
$rendered_menu = menu_tree_build($conf);

http://drupalcontrib.org/api/drupal/contributions!menu_block!menu_block....

heyyo’s picture

If you don't want to add this dependency to menu_block it's also possible to show those options of menu_block only if menu_block is enabled. with module_exists()

heyyo’s picture

Status: Needs work » Needs review
FileSize
10.83 KB

Here it is a version of the same patch without dependency on "Menu Block"
When configuring the Og Menu pane related settings to Menu Block will be disabled with description explaining that Menu Block is required to use them.

I also fixed og_menu_index in select field which couldn't equal to 0, limitation of #options:
https://api.drupal.org/api/drupal/developer!topics!forms_api_reference.h...

So the index of the first menu is now 1 not 0.

heyyo’s picture

FileSize
48.88 KB

Og_menu index

timwood’s picture

Patch from #33 rerolled for latest -dev code as of April 9, 2014 (up to commit 90c540c).

mglaman’s picture

Status: Needs review » Needs work

Needs work. Working on re-roll.

  1. +++ b/plugins/content_types/og_menu_pane_content_type.inc
    @@ -0,0 +1,180 @@
    +  // Optionally pass in the node context. If no context is selected,
    +  // we will try to determine the group from the args passed.
    +  'required context' => new ctools_context_optional(t('Node'), 'node'),
    +);
    

    Context isn't passed if this is optional, needs to be required.

  2. +++ b/plugins/content_types/og_menu_pane_content_type.inc
    @@ -0,0 +1,180 @@
    +  // Grabs the gid from the node's group ref field (for group content nodes).
    +  if (isset($context->data->{OG_AUDIENCE_FIELD}[LANGUAGE_NONE][0])) {
    +    $gid = $context->data->{OG_AUDIENCE_FIELD}[LANGUAGE_NONE][0]['target_id'];
    +  }
    +
    +  // This should be true for group nodes.
    +  elseif (isset($context->data->{OG_GROUP_FIELD})) {
    +    $gid = $context->data->nid;
    +  }
    +
    

    If the group is also a group content type, it displays parent menu and not the current group's menu.

mglaman’s picture

Revised patch. This taps into og_context() like the blocks to ensure a group's proper menu is display.

If viewing a group, displays group. When view group content, menu displays. When viewing a subgroup, that group's menu displays.

mglaman’s picture

Assigned: pontus_nilsson » Unassigned
mglaman’s picture

FileSize
6.25 KB

Realized that patch that was re-rolled, and previous, had a lot of extra cruft outside of patch scope. Here is a proper patch that just adds Ctools API hook and the content pane. Previous interdiff relevant, as it shows switch from Ctools context to the OG context function.

Chris Burge’s picture

During testing, I noticed the menu order is reversed when rendered.

For example, there are two menus: Group Menu 1 and Group Menu 2. The menus were created in that order and are listed in that order on the group's menu page (group/node/%/admin/menus). The patch provides a 'Menu index' field with the following options available: 'First Menu, 'Second Menu', and 'Third Menu'.

Observed Behavior
When 'First Menu' is selected, Group Menu 2 is rendered. When 'Second Menu' is selected, Group Menu 1 is rendered.

Expected Behavior
When 'First Menu' is selected, Group Menu 1 is rendered. When 'Second Menu' is selected, Group Menu 2 is rendered.

Running $menus through array_reverse() corrects this issue.

Chris Burge’s picture

Status: Needs review » Needs work

After further testing, the behavior I described occurs when the 'Automatically create menu for new Organic Group' option is selected (admin/config/group/og_menu). It does not occur when this option is not selected.

Perhaps instead of keying on 'og_menu_index', we should key on 'menu_name'. This is the approach taken by #2328915: Method to assign weights to menus. This approach would also populate the 'Menu index' field from the database instead of hardcoding 'First Menu, 'Second Menu', and 'Third Menu'.