Refactored theme_megamenu_menu_tree(), multiple fixes.

And just in case, here's the function:

function theme_megamenu_menu_tree($menu_name) {

  $menutree = _megamenu_get_menu_tree($menu_name);

  if (function_exists('i18nmenu_localize_tree')) {
    i18nmenu_localize_tree($menutree);
  }

  return theme_megamenu_menu_tree_helper($menu_name, $menutree, 1);
}

And a couple of helper functions:

function theme_megamenu_menu_tree_helper($menu_name, $item, $level) {
  $ul_classes = array('', 'megamenu-menu', 'megamenu-bin', 'megamenu-items');
  $li_classes = array('', 'megamenu-parent', 'megamenu-slot', 'megamenu-item', 'megamenu-subitem');
  $h_classes = array('',  'h2', 'h3', 'h4', 'h5');
  
  $minimize_html = TRUE;
  
  $position = 0;
  $leaf_count = count($item);
  
  foreach ($item as $leaf) {
  
    // Nested call
    if ($leaf['below']) {
      $items_list = theme_megamenu_menu_tree_helper($menu_name, $leaf['below'], $level + 1);
    }
    else {
      $items_list = NULL;
    }

    $position++;
    // are we active / active-trail ?
    $active = _megamenu_active_classes($leaf);
    
    $mlid = $leaf['link']['mlid'];
    $link_options['attributes'] = array('class' => $active/* . ' menu-' . $mlid*/); // No need for mlid in class

    // http://drupal.org/node/799280
    if(is_array($leaf['link']['options']['attributes'])) {
      $link_options['attributes'] = array_merge($link_options['attributes'], $leaf['link']['options']['attributes']);
    }
    
    // http://drupal.org/node/1078694
    // do we have a query string
    if(isset($leaf['link']['options']['query'])){
      $link_options['query'] = $leaf['link']['options']['query'];
    }
    
    // This <li> content
    // TEMP - TODO: if no children - change 'hx' to 'leaf'
    $menu_title = _megamenu_get_translated_menu_title($menu_name, $mlid);
    
    // Create $data to add to $leaf_item
    $data = '<';
    // Opening header tag
    $data .= $h_classes[$level];
    $data .= ' class="' . $li_classes[$level] . '-title';
    // http://drupal.org/node/1170472
    if ($leaf['link']['router_path'] == 'nolink') {
      $data .= ' nolink">' . $menu_title;
    }
    else {
      $data .= '">' . l($menu_title, $leaf['link']['href'], $link_options);
    }
    // Closing header tag
    if (isset($items_list)) {
      $data .= '</' . $h_classes[$level] . '>';
    }
    
    $data .= $items_list;

    $leaf_item = array();
    
    if($minimize_html == FALSE) {
      $leaf_item['id'] = 'megamenu-mlid-' . $mlid;
      $leaf_item['class'] = $li_classes[$level] . '-' . _megamenu_count_attributes($position, $leaf_count);
    }
    
    $leaf_item['data'] = $data;
    $leaf_item['class'] .= ' ' . $li_classes[$level] . $active;
    // http://drupal.org/node/1172776#comment-4674980
    $leaf_item['class'] .= isset($items_list) ? ' has-children' : ' no-children';
    $leaf_items[] = $leaf_item;
  } // END leaf iteration
  
  // Build leaf list
  $list_options = array( 'class' => $ul_classes[$level] . ' ' . _megamenu_menu_attribute_by_level($menu_name, $level) );
  if ($level == 1) {
      $list_options['id'] = 'megamenu-' . $menu_name;
  }
  $items_list = theme('item_list', $leaf_items, NULL, 'ul', $list_options);
  return _megamenu_strip_list_wrapper($items_list);
}

function _megamenu_menu_attribute_by_level($menu_name, $level) {
// TODO: cache
  switch ($level) {
    case '3':
      /* TODO: temp value, should be attached to twig level in admin interface. */
      return ' ' . _megamenu_get_slot_attributes_by_name($menu_name); 
      
    case '2':
      /* TODO: temp value, should be attached to branch level in admin interface */
      return ' megamenu-slots-' . _megamenu_get_slot_orientation_by_name($menu_name); 
      
    case '1':
      $skin = _megamenu_get_skin_by_name($menu_name);
      $menu_orientation = _megamenu_get_menu_orientation_by_name($menu_name);
      return ' ' . $menu_orientation . ' megamenu-skin-' . $skin;
  }
}

I added $minimize_html = TRUE; as my taxonomy menu has 500 items, so by removing unused classes and attributes I reduce the html size significantly. I am going to make it configurable, but for now it should be changed to FALSE.

And I move here my refactored _megamenu_get_menu_tree() from #1198976: Warning: Invalid argument supplied for foreach() in i18nmenu_localize_tree() (line 114 of sites/all/m

function _megamenu_get_menu_tree($menuname) {
  $menutree = menu_tree_all_data($menuname);
  $menutree_page = menu_tree_page_data($menuname); // Contains active trail
 
  _megamenu_get_menu_tree_helper($menutree, $menutree_page, 1);
  
  return $menutree;
}

/*
 * On top level $item = $menutree, $page_item = $menutree_page
 */
function _megamenu_get_menu_tree_helper(&$item, $page_item, $level = 1) {
  foreach ($item as $tier_key => $tier_item) {
    if ($page_item[$tier_key]['link']['in_active_trail']) {
      $item[$tier_key]['link']['in_active_trail'] = TRUE;
    }
    
    if ($tier_item['link']['hidden'] == 1 || empty($tier_item['link']['link_title'])) {
      unset($item[$tier_key]);
    }
    else {
      if ($tier_item['below']) {
        if ($level < 4) {
          _megamenu_get_menu_tree_helper($item[$tier_key]['below'], $page_item[$tier_key]['below'], $level + 1);
        }
        else {
          dsm($item[$tier_key]['below'], 'Removing');
          unset($item[$tier_key]['below']);
          $item[$tier_key]['below'] = FALSE;
        }
      }
    }
  }
}

And I think that with the new theme_megamenu_menu_tree() implementation _megamenu_get_menu_tree() is no longer necessary, the check for active menu item can be easily moved to theme_megamenu_menu_tree_helper() or _megamenu_active_classes().

If maintainers of the module are interested in my changes, I can do this one also.

I've split up the theme_megamenu_menu_tree_helper() function in smaller functions - I tried to implement #1172776: Make MEGA only one menu item?, but don't have time anymore.

I've got some hacky code there (commented with //HACK) which actually does work, but it's not nice. So if the items at second level don't have children - they are displayed as level 3 items, in single slot. Image attached.

I guess it's simpler to do that in _megamenu_get_menu_tree_helper() instead - if list of items doesn't have children - insert an extra level (single 'nolink' line) and move the list to this line children. Then the hack can be removed from theme_megamenu_menu_tree_helper().

And the file attached.

Hi there, how is review going? I think that original code is a bit too messy and before doing any fixes/enhancements it should be refactored. I spent already some time cleaning the code - using nested call instead of having 3 or 4 FOREACH levels. Would be nice if this code could be used.

Yeah, this was just for debugging in case taxonomy has more than 3 levels

I'm really sorry I didn't make a patch, I have to start learning patching. As my changes (refactoring) are pretty significant, I attached my version of two files, hope this helps to make this beautiful module even better!

I added .txt as drupal doesn't allow .module or .inc extensions.

I finally learned how to create patches, well, not completely, see http://drupal.org/node/1054616#comment-5725064, help appreciated.

Here's the patch against 6.x-2.0-beta2 (I hope)

My pleasure @greggmarshall, it's my first Drupal patch :)