The menu items consists of <li><a></a></li>, but only the <a> element has the 'active' class (for the active menu item). The active <li> has the active-trail class, but that is not enough. For bootstrap to style correctly the <li> element needs to have the 'active' class.

Jarl

Files: 
CommentFileSizeAuthor
#3 bootstrap-add_active_class_to_li_elements-1896674-2.patch994 bytesandregriffin
PASSED: [[SimpleTest]]: [MySQL] 0 pass(es). View
#1 bootstrap-add_active_class_to_li_elements-1896674-1.patch1008 bytesgeneralredneck
PASSED: [[SimpleTest]]: [MySQL] 0 pass(es). View

Comments

generalredneck’s picture

Status: Active » Needs review
FileSize
1008 bytes
PASSED: [[SimpleTest]]: [MySQL] 0 pass(es). View

This should take care of the problem. It worked for me.

andregriffin’s picture

Status: Needs review » Fixed
andregriffin’s picture

Status: Fixed » Needs review
FileSize
994 bytes
PASSED: [[SimpleTest]]: [MySQL] 0 pass(es). View

Reroll against dev

generalredneck’s picture

My bad.

andregriffin’s picture

Status: Needs review » Fixed

Thanks.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

markcarver’s picture

Status: Closed (fixed) » Needs work
Issue tags: +needs backport to 7.x-2.x
+++ b/includes/menu.inc
@@ -90,7 +90,11 @@ function bootstrap_menu_link(array $variables) {
+ if (($element['#href'] == $_GET['q'] || ($element['#href'] == '<front>' && drupal_is_front_page())) && (empty($element['#localized_options']['language']) || $element['#localized_options']['language']->language == $language_url->language)) {

The $language_url variable (towards the end) is actually not defined in this function. Where is it coming from??

Stumbled upon this when refactoring for 8.x-2.x. Would change the version for this issue, but that branch doesn't have a "release" yet. Tagging for backport.

markcarver’s picture

Version: 7.x-2.x-dev » 8.x-2.x-dev
byteways’s picture

Yep, it looks like $language_url is a global variable and should be declared as such in bootstrap_menu_link(), am I correct?

clarkorz’s picture

Issue summary: View changes

Doesn't work with Context menu reaction.

regilero’s picture

Maybe I do not understand everything, but.. the problem here is to add 'active' when only 'active-trail' is present?

Then why not simply doing it?

  if (array_key_exists('class',$element['#attributes']) 
    && is_array($element['#attributes']['class'])
    && in_array('active-trail',$element['#attributes']['class'])) {
      $element['#attributes']['class'][] = 'active';
  }

Current patch tests the href with q argument, but this will not work for active-trail set via menu_tree_set_path or menu_set_active_trail, as by definition theses things are used to set active trail on things where the path is not an obvious way of finding the active trail.

  • andregriffin committed cc016cc on 8.x-3.x.x
    Issue #1896674 by generalredneck: Adds class 'active' to li on menu item
    
  • andregriffin committed 1b90324 on 8.x-3.x.x
    Issue #1896674 by generalredneck: Adds class 'active' to li on menu item
    
markcarver’s picture

Version: 8.x-2.x-dev » 7.x-2.0
Status: Needs work » Closed (fixed)

I'm just moving this back to 7.x. If there needs to be any re-evaluation for 8.x, create a new issue.

kristiaanvandeneynde’s picture

Status: Closed (fixed) » Needs work

+1 for regilero in #11

This patch that went in breaks the use of hook_preprocess_menu_link() to unset the active class on a link on the front page.

markcarver’s picture

Status: Needs work » Closed (fixed)

Please do not re-open old issues.

kristiaanvandeneynde’s picture