Merge pull request #8669 from piwik/8624_module_active_detect

Fixes #8624, rewrite the way menu.js detects the active menu item (don't set elements, look for links w/ the right URL)

plugins/CoreHome/javascripts/broadcast.js

@@ -409,11 +409,23 @@ 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. using 2 $timeouts (to push
+            // the menu activation logic to the end of the event queue twice), seems to work.
+            angular.element(document).injector().invoke(function ($timeout) {
+                $timeout(function () {
+                    $timeout(function () {
+                        piwikMenu.activateMenu(
+                            broadcast.getParamValue('module', urlAjax),
+                            broadcast.getParamValue('action', urlAjax),
+                            {
+                                idGoal: broadcast.getParamValue('idGoal', urlAjax),
+                                idDashboard: broadcast.getParamValue('idDashboard', urlAjax)
+                            }
+                        );
+                    });
+                });
+            });
         }
 
         if(broadcast.getParamValue('module', urlAjax) == 'API') {

plugins/CoreHome/javascripts/menu.js

@@ -53,53 +53,41 @@ 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;
+                }
+
+                var actual = broadcast.getValueFromHash(key, url);
+                if (actual != params[key]) {
+                    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.

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">

tests/UI/specs/Menus_spec.js

@@ -23,13 +23,13 @@ describe("Menus", function () {
 
     it('should change the menu when a upper menu item is clicked in the main menu', function (done) {
         expect.screenshot('mainmenu_upper_clicked').to.be.captureSelector('.Menu--dashboard,.nav_sep', function (page) {
-            page.click('#VisitsSummary>a');
+            page.click('.Menu-tabList > li:eq(1) > a');
         }, done);
     });
 
     it('should change the menu when a lower menu item is clicked in the main menu', function (done) {
         expect.screenshot('mainmenu_lower_clicked').to.be.captureSelector('.Menu--dashboard,.nav_sep', function (page) {
-            page.click('#Live_indexVisitorLog>a');
+            page.click('.Menu-tabList > li:eq(1) > ul > li:eq(1) > a');
         }, done);
     });