Support from Acquia helps fund testing for Drupal Acquia logo

Comments

wundo’s picture

If you can write a test for this I'd love to commit it ;)

natted’s picture

I've just been testing.

In the latest dev, menu block appears to be working as long as "Expand all children of this tree." is not checked under Advanced Options. If that is checked, then bootstrap applies the .dropdown-submenu class.

I'd have to think about what the best approach would be to play nicely with menu_block and keep the bootstrap style menu's as an option.

In the meantime, in your subtheme, you could always override bootstrap_menu_link

natted’s picture

Ok, so I've tested Menu Block and I guess the main issue is that Bootstrap converts all the menu's to dropdowns if they have "Show as expanded" enabled or "Expand all children of this tree".

Some people may not want dropdown menus when using menu block module... am I correct?

I didn't see any other issues but please advise and include as much information as possible if something else with menu_block is broken in relation to bootstrap.

woop_light’s picture

+1

I'm having trouble any time that I have child menu items in a Menu Block. Could you explain how to override bootstrap_menu_link? I'm jumping through hoops trying to get the Bootstrap out of my Menu Block.

Killer job on the Bootstrap theme -- must have been a hell of a job make the two play nice.

woop

woop_light’s picture

Most importantly, I'm noticing that -- at least in my configuration, might be worth double checking -- when I go to the page of a child link, I cannot go back to its parent's page using the menu. The problem is that it just keeps toggling the dropdown menu.

jorge.aguero’s picture

Due to the configuration needs for bootstrap dropdown menu, I added a few lines to menu.inc file to accommodate for toggled feature. Currently I am only using it on a per project basis, as this is not the solution for everyone.

Line 62ish

function fpbase_menu_link(array $variables) {
  $element = $variables['element'];
  $sub_menu = '';
+  /* added for overview page */
+	$sub_menu_parent = '';
	
  if ($element['#below']) {
    // Ad our own wrapper
    unset($element['#below']['#theme_wrappers']);
+		/* added for overview page */
+		if(in_array($element['#theme'], array('menu_link__main_menu'))) {
+			$sub_menu_parent = l($element['#title'], $element['#href']);
+			$sub_menu_parent = '<li class="submenu-title">'.$sub_menu_parent .'</li>';
+		}
-    $sub_menu = '<ul class="dropdown-menu">' . drupal_render($element['#below']) . '</ul>';
+   $sub_menu = '<ul class="dropdown-menu">' . $sub_menu_parent . drupal_render($element['#below']) . '</ul>';
    $element['#localized_options']['attributes']['class'][] = 'dropdown-toggle';
    $element['#localized_options']['attributes']['data-toggle'] = 'dropdown';

This duplicates the parent that is considered a toggle and places it into the submenu area as the top item with it's own class. The next step is labeling it according to your project needs.

For example adding: Overview or [title] Overview

-$sub_menu_parent = l($element['#title'], $element['#href']);
+$sub_menu_parent = l($element['#title'] . ' Overview', $element['#href']);
rowbeast’s picture

Re: #3 natted's question about people not wanting dropdown's all the time.
Yes, you're correct. Further I believe this should be independent of the Menu Block module.
It would be great to have that type of functionality toggleable. Perhaps as a theme setting?
"enable bootstrap drop down menus".

I'd also like to second woop_light's question in #4 about how best to override bootstrap_menu_link()?
Could an example be provided on how to approach this so we can do the more traditional indented menu structures?

Great theme, thanks so much for putting the work against it!
~Row

rowbeast’s picture

I was able to work out a menu override for my theme based on the original functions in menu.inc. I thought i would post here for those looking to pull the dropdown system out of menu's. I doesn't address the original threads topic so if there is a better place to put it just let me know (not tying to hijack, just help other googlers like myself).

This would go in your subtheme's template.php and you would replace the jetpack_ with your themes name.
If there is a cleaner way to do this, I'd love to see some refactoring.

function jetpack_menu_tree(&$variables) {
  return '<div class="nav-collapse"><ul class="menu nav">' . $variables['tree'] . '</ul></div>'; // added the nav-collapse wrapper so you can hide the nav at small size
}


function jetpack_menu_link(array $variables) {
  $element = $variables['element'];
  $sub_menu = '';
  
    if ($element['#below']) {
    // Ad our own wrapper
    unset($element['#below']['#theme_wrappers']);
    $sub_menu = '<ul>' . drupal_render($element['#below']) . '</ul>'; // removed flyout class in ul
    unset($element['#localized_options']['attributes']['class']); // removed flydown class
    unset($element['#localized_options']['attributes']['data-toggle']); // removed data toggler

    // Check if this element is nested within another
    if ((!empty($element['#original_link']['depth'])) && ($element['#original_link']['depth'] > 1)) {
      
      unset($element['#attributes']['class']); // removed flyout class
    }
    else {
      unset($element['#attributes']['class']); // unset flyout class
      $element['#localized_options']['html'] = TRUE;
      $element['#title'] .= ''; // removed carat spans flyout
    }

    // Set dropdown trigger element to # to prevent inadvertent page loading with submenu click
    $element['#localized_options']['attributes']['data-target'] = '#'; // You could unset this too as its no longer necessary. 
  }
  
  $output = l($element['#title'], $element['#href'], $element['#localized_options']);
  return '<li' . drupal_attributes($element['#attributes']) . '>' . $output . $sub_menu . "</li>\n";
}

bigscotia10’s picture

Nice @rowbeast. Anyone think it would make sense to make menu block menus, select box menu's when on bootsrap mobile? If menu's get 4+ levels deep, any sort of mobile menu gets messy. By making them select boxes, users can scroll and select the menu item quickly.

laroccadahouse’s picture

i would definitely love to see some menu block compatibility.

laroccadahouse’s picture

#8 works well to show an entire menu block with all children expanded. the functionality i would like to see is the standard menu block with collapsible menu items that show the children of an active page.

I would love to see this switch to a drop down or select list style with all menu items specified in the individual block when it is displayed on a phone.

markhalliwell’s picture

Pretty sure this is a dup of #1893532: [bootstrap][policy][7.x-3.x] Navigation/Menus.

Not sure if there's really a way to target just menu blocks (from a menu tree standpoint). I'm tempted to say that this actually might need to be in a module of some sort. That way we could toggle "dropdowns" on a per menu-block basis, I'd hate to have either "all or none".

Jehu’s picture

My quick and dirty solution to use block_menu in sidebar together with the Twitter Bootstrap navbar as main menu (here shown as patch, could be encapsulated as a module or you can overwrite the behavior in your template.php to go a proper way:

Index: sites/all/themes/bootstrap/theme/menu/menu-link.func.php
<+>UTF-8
===================================================================
--- sites/all/themes/bootstrap/theme/menu/menu-link.func.php	(revision 779b4764f3224ad5d2e8fcbe427e9a9a942b64e1)
+++ sites/all/themes/bootstrap/theme/menu/menu-link.func.php	(revision )
@@ -14,7 +14,11 @@
   if ($element['#below']) {
     // Prevent dropdown functions from being added to management menu so it
     // does not affect the navbar module.
-    if (($element['#original_link']['menu_name'] == 'management') && (module_exists('navbar'))) {
+    if (
+      ($element['#original_link']['menu_name'] == 'management' && module_exists('navbar'))
+      || ((!empty($element['#original_link']['depth']))
+          && (isset($element['#bid']) && $element['#bid']['module'] == 'menu_block'))
+    ) {
       $sub_menu = drupal_render($element['#below']);
     }
     elseif ((!empty($element['#original_link']['depth'])) && ($element['#original_link']['depth'] == 1)) {
acbramley’s picture

Issue summary: View changes

The main issue with using menu_block with this theme is that if (for example) you have a left hand menu that is set to output from level 2 to unlimited depth, you never get child links rendered because of the explicit condition in bootstrap_menu_link():

elseif ((!empty($element['#original_link']['depth'])) && ($element['#original_link']['depth'] == 1)) {

It means that no children are output apart from 2nd level.

mstrelan’s picture

I'm experiencing the same issue as acbramley in #14.
My workaround is to just call the default theme implementation for menu links in menu blocks.

<?php
/**
 * Overrides theme_menu_link().
 */
function MYTHEME_menu_link__menu_block($variables) {
  return theme_menu_link($variables);
}
?>
entrepreneur27’s picture

I am trying to get this functionality and see the code in #15. But I am unclear where to put it. I tried just putting it into my template.php file (in my sub theme) but that was not enough and I suspect I need to call it from somewhere or something. Any tips?

Kiwa’s picture

I tried the suggestion in #15 today and it worked for me. I put the code into the template.php of my subtheme as well.

@entrepreneur: Don't forget to replace 'MYTHEME' with your actual theme name and flush caches afterwards, then it should work.

There is a further refinement some might find useful, if they only want to target a specific menu_block:

<?php
/**
 * Overrides theme_menu_link().
 */
function MYTHEME_menu_link__menu_block__ID($variables) {
  return theme_menu_link($variables);
}
?>

ID is the ID of the menu block, one can easily read from the URL when editing a particular menu block.

gavmassingham’s picture

Ah! Excellent. Thanks mstrelan and kiwa - this has been annoying me for a while.

Eric At NRD’s picture

Thank you so much mstrelan and kiwa! I spent an embarassingly long time trying to figure this out.

entrepreneur27’s picture

OK. I am (stupidly) trying to make this work with 3.1 beta 1. I think this means the code in#15 needs to go somewhere other than the template.php file. I am clearly totally out of my depth.

I post this in the hope of saving someone else some time. Or of anyone knowing what to do to make 3.1 work with menu block and taking pity on me. :)

Schnitzer’s picture

Hi, i run in the same issue and this thread inspired me to another solution.

I just wrapped the
if ($element['#below']) { //make some dropdowns }
with a 2nd condition where i only pass my menus wich get dropdowns.

    // only pass the menus i want to get dropdowns
  if ( $element['#theme'][0] == 'menu_link__menu_block__1' || $element['#theme'][0] == 'menu_link__menu_block__2' ) {
    
    if ($element['#below']) {
      // Prevent dropdown functions from being added to management menu so it
      // does not affect the navbar module.
      if ( ($element['#original_link']['menu_name'] == 'management') && (module_exists('navbar')) ) {
        $sub_menu = drupal_render($element['#below']);
      }
      elseif ((!empty($element['#original_link']['depth'])) && ($element['#original_link']['depth'] >= 1)) {
        // Add our own wrapper.
        unset($element['#below']['#theme_wrappers']);
        $sub_menu = '<ul class="dropdown-menu">' . drupal_render($element['#below']) . '</ul>';
        // Generate as standard dropdown.
        $element['#title'] .= ' <span class="caret"></span>';
        $element['#attributes']['class'][] = 'dropdown';
        $element['#localized_options']['html'] = TRUE;
  
        // Set dropdown trigger element to # to prevent inadvertant page loading
        // when a submenu link is clicked.
        $element['#localized_options']['attributes']['data-target'] = '#';
        $element['#localized_options']['attributes']['class'][] = 'dropdown-toggle';
        $element['#localized_options']['attributes']['data-toggle'] = 'dropdown';
      }
    }

}

I also set the ['depth'] == 1 to ['depth'] >= 1 to get all subitems.
In an else after this wrapper you can render your normal treestyle.

judy07’s picture

I'm trying to use Menu Block module with Boostrap 7.x-3.0 and CODE discussion but it does not work

/**
 * @file
 * menu-link.func.php
 */

/**
 * Overrides theme_menu_link().
 */


 
function bootstrap_menu_link(array $variables) {
  $element = $variables['element'];
  $sub_menu = '';
   $sub_menu = '';
 /* added for overview page */
    $sub_menu_parent = '';

  if ($element['#below']) {
    // Prevent dropdown functions from being added to management menu so it
    // does not affect the navbar module.
   // if (($element['#original_link']['menu_name'] == 'management') && (module_exists('navbar'))) {
   
   if (
     ($element['#original_link']['menu_name'] == 'management' && module_exists('navbar'))
     || ((!empty($element['#original_link']['depth']))
         && (isset($element['#bid']) && $element['#bid']['module'] == 'menu_block'))
   ) 
       {
       $sub_menu = drupal_render($element['#below']);
     }
    elseif ((!empty($element['#original_link']['depth'])) && ($element['#original_link']['depth'] >= 1)) {
      // Add our own wrapper.
      unset($element['#below']['#theme_wrappers']);
       /* added for overview page */
       if(in_array($element['#theme'], array('menu_link__main_menu'))) {
          //$sub_menu_parent = l($element['#title'], $element['#href']);
		  $sub_menu_parent = l($element['#title'] . ' Overview', $element['#href']);
          $sub_menu_parent = '<li class="submenu-title">'.$sub_menu_parent .'</li>';
      }
     // $sub_menu = '<ul class="dropdown-menu">' . drupal_render($element['#below']) . '</ul>';
      // Generate as standard dropdown.
      $element['#title'] .= ' <span class="caret"></span>';
      $element['#attributes']['class'][] = 'dropdown';
      $element['#localized_options']['html'] = TRUE;

      // Set dropdown trigger element to # to prevent inadvertant page loading
	   $sub_menu = '<ul class="dropdown-menu">' . $sub_menu_parent . drupal_render($element['#below']) . '</ul>';
      // when a submenu link is clicked.
      $element['#localized_options']['attributes']['data-target'] = '#';
      $element['#localized_options']['attributes']['class'][] = 'dropdown-toggle';
      $element['#localized_options']['attributes']['data-toggle'] = 'dropdown';
    }
  }
  // On primary navigation menu, class 'active' is not set on active menu item.
  // @see https://drupal.org/node/1896674
  if (($element['#href'] == $_GET['q'] || ($element['#href'] == '<front>' && drupal_is_front_page())) && (empty($element['#localized_options']['language']))) {
    $element['#attributes']['class'][] = 'active';
  }
  $output = l($element['#title'], $element['#href'], $element['#localized_options']);
  return '<li' . drupal_attributes($element['#attributes']) . '>' . $output . $sub_menu . "</li>\n";
}
acbramley’s picture

Please check comment #14 for an explaination about why it may not be working for you.

Please refrain from posting large blocks of code, especially when they are from the bootstrap theme itself. You can easily reference a file and line number.

In terms of a fix in my case, I managed to get it working by overriding the menu_llnk themeing function in my own theme and removing the depth check from this elseif after copying it from menu-link.fun.php in the bootstrap theme.

/**
 * Overrides theme_menu_link().
 */
function mytheme_menu_link(array $variables) {
   //....
   // ....
   //
    elseif ((!empty($element['#original_link']['depth']))) {
...
..
..
..
modulist’s picture

I used the fix by @mstrelan in #15 and it worked perfectly. I'm still looking forward to trying the ID fix as well in comment #17.

markhalliwell’s picture

Version: 7.x-2.x-dev » 7.x-3.x-dev

This is still something I would like to tackle in 7.x-3.x, but definitely by 7.x-4.x.

16am’s picture

I had the same issue with the book navigation using the great Book Outline Block module.

I could fix it by #21 trick, expecially after reading the last 2 line...

I also set the ['depth'] == 1 to ['depth'] >= 1 to get all subitems.
In an else after this wrapper you can render your normal treestyle.

doakym’s picture

#15 works for me

ckng’s picture

Another issue related to menu_block is the FAPI js states not working on admin pages.

Dubs’s picture

#15 works for me too. Can this please be added by default to the bootstrap base theme?

Andy Inman’s picture

Just to offer my code here:


/**
* Book navigation override. Andy, netgenius.co.uk
* This overrides behaviour of the bootstrap theme and reverts to "standard" Drupal behaviour
* for book navigation menu block. Optionally, use dhtml_menu module as well to get animated expand/collapse.
*/
function mytheme_menu_link(array $variables) {
// If this is a book menu sub-item, override it.
if (strncmp($variables['element']['#original_link']['menu_name'], 'book-toc-', 9) == 0) {
return theme_menu_link($variables);
}
// Otherwise hand over to standard bootstrap_menu_link() function.
return bootstrap_menu_link($variables);
}

The above needs to go into your themes template.php or other suitable location. As it says, you can use DHTML menu to get animated expand/collapse. In live use here.

lucass’s picture

I would like to share a simple patch which allow enable submenus with unlimited depth and disable dropdown menus for Menu Block Module.

The setup can be done in settings of bootstrap under the Components tab in "Menu Block Module" menu.

I tested the patch in 7.x-3.0 and 7.x-3.x-dev versions and the result was positive.

bmango’s picture

The 7.x-3.0 patch in #31 worked well for me. @lucass Thank you very much!

gilsbert’s picture

Status: Active » Reviewed & tested by the community

Both patches are working.
Thanks.

The last submitted patch, 31: bootstrap-support-menu-block-module-1850194-31.patch, failed testing.

wundo’s picture

@lucass, your patch added "tabs as indenting" and trailing white spaces, also there was a typo on the form element description, here is a updated patch which fixes those.

Please notice that I haven't tested the patch, I only fixed those issues I could spot by reading it.

Also, please only submit patches against HEAD, not stable releases.

Cheers,
Fabiano

wundo’s picture

Status: Reviewed & tested by the community » Needs review
markhalliwell’s picture

Title: Support menu block module » Add support for menu block module
Component: User interface » Code
Status: Needs review » Needs work
Related issues: +#1893532: [bootstrap][policy][7.x-3.x] Navigation/Menus

I've taken a look at this patch and I'm not very convinced that this is the right approach. It appears that a lot of what this code is doing is "unsetting" previous functionality in an effort to fix a specific use case (and to perhaps address the issues outlined in the related issue I'm posting now). We shouldn't get in the habit of hacking our own code if we can avoid it (which is what the other related issue is about).

In all reality, the code attached is not actually specifically targeting the menu block module at all (which is what this issue should be about). Instead, it is rather generic and is, in fact, obscuring global menu management under the guise of "supporting" a specific module.

Furthermore, I don't think this issue should introduce new settings, but rather do what the issue title suggests: "[add] support [for] menu block module" which any code logic applied here should be inherent and automatic based simply on if the module is installed. So, I'm updating the title and component to reflect this and make it a little more explicit.

I would also say that this issue should probably be postponed on the related issue (and perhaps even moved to 7.x-4.x) as we have other issues that need focus at the moment. However, in the interest of progress, I'll just leave this issue on 7.x-3.x and set to CNW.

acbramley’s picture

Gotta agree with @markcarver here. At least the work done previously highlights what kind of thing fixes it for menu_block and hopefully that'll facilitate an easier "real" fix :)

lucass’s picture

I guess that the patch hit the issue, the options of enable deeper levels and to disable dropdown menus are features that support the Menu Block Module and allow this to work correctly.

About the UI settings, why don't introduce? This makes more customizable and allow the control of behavior according to which the users need, so who want the original behavior of Bootstrap simply dont enable nothing, moreover the standard is the options disabled.

If this is not the right approach, so what is? I would be glad to know in order to help.

gilsbert’s picture

Hello everyone.

I'm not a maintainer of bootstrap but I would like just to contribute with a few thoughts.

Personally I like (and I prefer) changes introduced by UI options instead of changes introduced on theme/module's behavior. This approach should avoid a lot of discussion about what is necessary, what is the original purpose, what is mandatory, and so on. It also avoid conflict with people who need the original behavior unchanged. That said I would suggest to keep the changes by UI options instead to give bootstrap a new menu behavior.

I agree with @lucass about the two new options: both are directly necessary for a complete integration between bootstrap theme and menu block module. Yes it is a generic approach but it should bring more credit to the patch (not less).

@wundo: thank you for your effort. I agree that a patch for the stable version is not mandatory but some people may be thankful for not being mandatory to use a dev version (less stable right?) only because they need one or both new options. For example we are using bootstrap in all our production sites and it is good to have this patch avaliable.

This issue has almost 3 years old. I would like to ask to keep it for the next release and say thanks for @markcarver for allowing us to help.

Regards,
Gilsberty

markhalliwell’s picture

Hm. I will attempt to reiterate what I said, but hopefully in a way that will make more sense:

  1. The logic introduced here is global. It applies to all menu links, not just the links that are used in a menu block. This means that if this patch were to go in (which it won't as is), if a user chooses a setting to be applied under the assumption it is "just for menu blocks", it would also be applied for the main navigation for instance. This isn't ideal. Menu blocks, historically, need to be or should be configured from the menu block configuration, not globally through a theme. If anything, I would echo my sentiments in #12 and state that this kind of functionality should live in some sort of form alter inside a module like bootstrap_core (since the admin theme is not always bootstrap based and could not be effectively altered from a project, theme, like this).
  2. I'm not specifically opposed to adding new settings in the UI, except that these settings are really global menu management settings, not specific to menu block at all. They also really should be in the "General" tab (where other Drupal integration stuff is managed) and leave the "Components" to be in parity with Bootstrap "Components".
  3. I'm not discounting the effort here, but rather saying that the patch provided here has more resonance with the related issue I have mentioned than this one. If we're going to support menu block, then the patch should reflect it. I do not see anything about menu block in this code. An example of what I mean by "menu block specific code" is something like #15 or #17, where it is specially targeting menu block links through a theme hook suggestion. That being said, I'm not entirely sure that those examples will work or if they are even the right solution per se (menu block must invoke the theme hook with those theme hook suggestions to begin with).
gilsbert’s picture

Hi @markcarver.
Good idea. I'm sure that adding those two new options in general tab will bring us an excellent result.
Should @lucass go for it? Should a new topic be created at general tab? What about "Menu behavior"?
Regards,
Gilsberty

noman_297’s picture

I have an issue with book explorer navigation with bootstrap theme,Parent link is not clickable also the expandable links not working with book explorer navigation.Also +,- icon is not working.Plz help me out.

lucass’s picture

Hello @markcarver,

There a possibility to incorporate the features, deeper level menu and disable dropdown menu, in bootstrap theme ?
If yes, as you think it should be done?
I will be at the disposal to help making this features.

Best.

drupbasic’s picture

#15 worked for me (copy/paste to the top of my subtheme). All childs are shown in the block. But, my hierarchy is flattened out. Any solutions?

markhalliwell’s picture

Priority: Normal » Minor
Status: Needs work » Postponed

The above patches are not the right approach. Something like #15 is, in fact more in line with what I had envisioned. That being said, this issue really isn't a priority right now.

gilsbert’s picture

Why postpone when you have people interested to help?
Thank you anyway.

dadderley’s picture

Thank you mstrelan for the solution in #15.

tcowin’s picture

The issue isn't so minor for those of us needing to display more than the second menu level -- the patch by @lucass, @wundo in #35 works well for me, although it would be good to be able to treat main nav and other nav areas independently.

metakel’s picture

My way to deal with the issue is to remove the class "dropdown-menu" from the secondary UL at the menu tree. I do this by adding the below function to the theme's template.php (replace the "XYZ" with the name of the menu that you wish to disable dropdown):

/**
 * Overrides theme_menu_tree().
 * Modify bootstrap class to disable Bootstrap's dropdown at the XYZ menu
 */

function bootstrap_menu_tree__menu_XYZ(&$variables) {
  return '<ul class="menu">' . str_replace('dropdown-menu', 'bootstrap-dropdown-menu', $variables['tree']) . '</ul>';
}

Then clear the cache and reload.

markhalliwell’s picture

Status: Postponed » Closed (won't fix)

I don't think this is going to happen for 7.x-3.x.

stavroch’s picture

Any news?

I have the same issue. The main-menu display only the 2nd level menu.

The third level menu doesn't displayed.

ofauravi’s picture

I have adapted patch #31 to work on later versions.