Refs #8624, do not set the ID of link elements in the reporting menu and query the IDs when detecting the active menu item, instead find the link w/ the appropriate parameters get the parent li element. Also, do not activate the menu item until all angular menu group dropdowns are rendered.

3 files changed, 42 insertions, 46 deletions

diff --git a/plugins/CoreHome/javascripts/broadcast.js b/plugins/CoreHome/javascripts/broadcast.js
index 51b2a8a64d..425936ff8b 100644
--- a/plugins/CoreHome/javascripts/broadcast.js
+++ b/plugins/CoreHome/javascripts/broadcast.js
@@ -409,11 +409,20 @@ var broadcast = {
      */
     loadAjaxContent: function (urlAjax) {
         if (typeof piwikMenu !== 'undefined') {
-            piwikMenu.activateMenu(
-                broadcast.getParamValue('module', urlAjax),
-                broadcast.getParamValue('action', urlAjax),
-                broadcast.getParamValue('idGoal', urlAjax) || broadcast.getParamValue('idDashboard', urlAjax)
-            );
+            // we have to use a $timeout since menu groups are displayed using an angular directive, and on initial
+            // page load, the dropdown will not be completely rendered at this point.
+            angular.element(document).injector().invoke(function ($timeout) {
+                $timeout(function () {
+                    piwikMenu.activateMenu(
+                        broadcast.getParamValue('module', urlAjax),
+                        broadcast.getParamValue('action', urlAjax),
+                        {
+                            idGoal: broadcast.getParamValue('idGoal', urlAjax),
+                            idDashboard: broadcast.getParamValue('idDashboard', urlAjax)
+                        }
+                    );
+                }, 1);
+            });
         }
 
         if(broadcast.getParamValue('module', urlAjax) == 'API') {
diff --git a/plugins/CoreHome/javascripts/menu.js b/plugins/CoreHome/javascripts/menu.js
index 85e25f2334..2bb3645527 100644
--- a/plugins/CoreHome/javascripts/menu.js
+++ b/plugins/CoreHome/javascripts/menu.js
@@ -53,53 +53,40 @@ menu.prototype =
         this.menuNode.find("li:has(ul),li#Searchmenu").hover(this.overMainLI, this.outMainLI);
         this.menuNode.find("li:has(ul),li#Searchmenu").focusin(this.overMainLI);
 
-        // add id to all li menu to support menu identification.
-        // for all sub menu we want to have a unique id based on their module and action
-        // for main menu we want to add just the module as its id.
-        this.menuNode.find('li').each(function () {
-            var link = $(this).find('a');
-            if (!link) {
-                return;
-            }
-            var href = link.attr('href');
-            if (!href) {
-                return;
-            }
-            var url = href.substr(1);
-
-            var module = broadcast.getValueFromUrl('module', url);
-            var action = broadcast.getValueFromUrl('action', url);
-
-            var moduleId = broadcast.getValueFromUrl("idGoal", url) || broadcast.getValueFromUrl("idDashboard", url);
-            var main_menu = $(this).parent().hasClass('Menu-tabList') ? true : false;
-            if (main_menu) {
-                $(this).attr({id: module});
-            }
-            // if there's a idGoal or idDashboard, use this in the ID
-            else if (moduleId != '') {
-                $(this).attr({id: module + '_' + action + '_' + moduleId});
-            }
-            else {
-                $(this).attr({id: module + '_' + action});
-            }
-        });
-
         this.menuNode.find('a.menuItem').click(this.onItemClick);
 
         menu.prototype.adaptSubMenuHeight();
     },
 
-    activateMenu: function (module, action, id) {
+    activateMenu: function (module, action, params) {
+        params = params || {};
+        params.module = module;
+        params.action = action;
+
         this.menuNode.find('li').removeClass('sfHover').removeClass('sfActive');
-        var $li = this.getSubmenuID(module, id, action);
-        var mainLi = $("#" + module);
-        if (!mainLi.length) {
-            mainLi = $li.parents('li');
-        }
+        var $activeLink = this.menuNode.find('a').filter(function () {
+            var url = $(this).attr('href');
+            if (!url) {
+                return false;
+            }
 
-        mainLi.addClass('sfActive').addClass('sfHover');
+            for (var key in params) {
+                if (!params.hasOwnProperty(key)
+                    || !params[key]
+                ) {
+                    continue;
+                }
+
+                if (url.indexOf(key + '=' + params[key]) === -1) {
+                    return false;
+                }
+            }
+
+            return true;
+        });
 
-        $li.addClass('sfHover');
+        $activeLink.closest('li').addClass('sfHover');
+        $activeLink.closest('li.menuTab').addClass('sfActive').addClass('sfHover');
     },
 
     // getting the right li is a little tricky since goals uses idGoal, and overview is index.
diff --git a/plugins/CoreHome/templates/_menu.twig b/plugins/CoreHome/templates/_menu.twig
index 6baf228bf2..8ec928eb4e 100644
--- a/plugins/CoreHome/templates/_menu.twig
+++ b/plugins/CoreHome/templates/_menu.twig
@@ -12,7 +12,7 @@
     <li>
         <div piwik-menudropdown show-search="true" menu-title="{{ name|translate|e('html_attr') }}">
             {% for item in group.getItems %}
-                <a class="item"
+                <a class="item menuItem"
                    href='#{{ item.url|urlRewriteWithParameters|slice(1) }}'
                    {% if item.tooltip %}title="{{ item.tooltip|e('html_attr') }}"{% endif %}>
                     {{ item.name|translate }}
@@ -38,7 +38,7 @@
 
     <ul class="Menu-tabList">
         {% for level1,level2 in menu %}
-            <li id="{% if level2._url is defined %}{{ _self.getId(level2._url) }}{% endif %}">
+            <li id="{% if level2._url is defined %}{{ _self.getId(level2._url) }}{% endif %}" class="menuTab">
                 <a class="menuItem" {% if level2._url is defined %}href="#{{ _self.getFirstUrl(level2._url) }}"{% endif %}>
                     {{ level1|translate }}
                    <span class="hidden">