@diosmosis opened this issue on August 30th 2015

This change changes the way menu.js detects the active menu item.

The old way of detecting menu items includes: 1. Iterating through each <li> element, finding it's associated menu link 2. Setting the id of the <li> element to something like {{ module}}_{{ action }} 3. Querying the <li> element w/ the proper ID.

This creates a problem for plugins that add menu items to existing menu tabs (eg, adding a page to Goals). When the page is active, Piwik will look for the menu item for the plugin (eg, MyPlugin) and won't find it or will activate the wrong tab group.

This change fixes this bug by changing the way active items are detected to: 1. Find all links w/ the right module/action/other parameters. 2. Getting the closest <li> parents that represent the menu tab / menu item.

Fixes #8624

@diosmosis commented on September 4th 2015

@mattab @tsteur ping-ing for review

@sgiehl commented on September 5th 2015

Just had a small look. It seems not to work when directly opening a specific goal. There is no selection happening in that case. Guess that happens as the goals are in a special "submenu"

@diosmosis commented on September 5th 2015

I tested this specifically since that was the bug I was trying to fix in another plugin. Can you check again? It's possible it doesn't work anymore though...

@sgiehl commented on September 5th 2015

Sure. I'll recheck that...

@sgiehl commented on September 5th 2015

Ah ok. Works! Guess I forgot to kill the assets

@sgiehl commented on September 5th 2015

I've now tried that several more times. When having only one goal it seems to work always, but as soon as there are some more goals, and the goals submenu is rendered it often doesn't. Seems to be a timing issue. Invoking the method manually with the correct params, always works. I'll try to find out why...

@sgiehl commented on September 5th 2015

Guess the reason is, that the dropdowns are not finished rendering. When activateMenu() is called the links found in the menu don't contain the goals. Maybe we need to increase the timeout or maybe also trigger menu activation when the dropdowns are finished

@diosmosis commented on September 5th 2015

Thanks for looking into it! I'll see if I can fix it somehow.

@diosmosis commented on September 7th 2015

When having only one goal it seems to work always, but as soon as there are some more goals, and the goals submenu is rendered it often doesn't.

Can't reproduce this on firefox or chrome. What browser are you using?

@sgiehl commented on September 7th 2015

Chrome on Windows 7 But I can test on Firefox or IE later

@sgiehl commented on September 7th 2015

Retried in Chrome and can't reproduce it there atm. Maybe due to the Chrome update yesterday. But I did some tests on IE and there the menu doesn't appear on reload:

@diosmosis commented on September 8th 2015

Can reproduce on IE11

@diosmosis commented on September 8th 2015

Only way I could fix this issue was w/ two $timeouts. It should work I think, but it might be too much of a hack...

CC @mattab

@mattab commented on September 9th 2015

Feedback: - Going to http://localhost/piwik-master/index.php?module=CoreHome&action=index&idSite=1&period=day&date=yesterday#/module=Referrers&action=index&idSite=1&period=day&date=yesterday - Got: Websites & Social and Overview submenus are black. - Expected: only Overview (current menu) should be black. - When adding a new "Custom dashboard" - Got: no submenu entry was added. (however on page refresh, the submenu entry is displayed as expected.) - Expected: sub menu refreshes and displays the custom dashboard as submenu of Dashboard.

It should work I think, but it might be too much of a hack...

The menu was already rewritten in Piwik 3.0.0 branch so this "hack" is not a problem IMHO

@diosmosis commented on September 9th 2015

When adding a new "Custom dashboard"

Got: no submenu entry was added. (however on page refresh, the submenu entry is displayed as expected.) Expected: sub menu refreshes and displays the custom dashboard as submenu of Dashboard.

Unrelated bug, happens for me on demo2.

@diosmosis commented on September 9th 2015

Going to http://localhost/piwik-master/index.php?module=CoreHome&action=index&idSite=1&period=day&date=yesterday#/module=Referrers&action=index&idSite=1&period=day&date=yesterday

Should be fixed.

@mattab commented on September 9th 2015

@diosmosis there are couple UI tests failing: http://builds-artifacts.piwik.org/piwik/piwik/8624_module_active_detect/15246/ - could you investigate, and once fixed, +1 to merge

This issue was closed on September 9th 2015
Powered by GitHub Issue Mirror