This project is read-only.


Admin menu collapses with LocalNav MenuItem from 2nd level - incorrect behaviour


Referring to Orchard.Web/Themes/TheAdmin/Views/Menu.cshtml

Line 23:
var firstOfTheSecond = secondLevelMenuItems.FirstOrDefault();
This retrieves the first second level nav item regardless of whether it is a LocalNav item or not.

It should be something like this:
var firstOfTheSecond = secondLevelMenuItems.Where(menuItem => !menuItem.LocalNav).FirstOrDefault();
Closed Aug 7, 2013 at 9:33 AM by sfmskywalker


CSADNT wrote Jul 1, 2013 at 7:51 AM

I agree, Already reported here

sebastienros wrote Jul 8, 2013 at 3:35 AM

This patch is breaking menus with tabs

CSADNT wrote Jul 8, 2013 at 7:46 AM

<ul class="menuItems">
            @foreach (var secondLevelMenuItem in secondLevelMenuItems.Where(menuItem => !menuItem.LocalNav)) {
                string secondLevelTextHint = secondLevelMenuItem.Text.TextHint;
                var firstOfTheThird = ((IEnumerable<dynamic>)secondLevelMenuItem.Items).FirstOrDefault();

sebastienros wrote Jul 8, 2013 at 5:56 PM

@CSADNT: you gave the current version of the code, what is the hidden message ;)

sfmskywalker wrote Jul 31, 2013 at 7:10 AM

@woodvorg Could you please explain the issue a little further, explaining what you are seeing, what you expected to see and what steps you took to get the undesired result? A sample implementation of INavigationProvider would help. Thanks.

woodvorg wrote Jul 31, 2013 at 8:14 AM

Sure mate! By way of example...

You know the Vandelay Industries content menu? If there is only a single item under Content, it collapses back to show just 'Content' with the URL '' and no children.

The behaviour I think we should expect to see is:
  • Have the child still visible under Content, with a different URL
  • Have the 'Content' item link to the child
It seems wrong behaviour that if there is a single child with a DIFFERENT url to the root that it would just not show the child.

I played around for a while but I couldn't come to an acceptable solution, so I just reverted to original code.

BertrandLeRoy wrote Jul 31, 2013 at 8:37 AM

Also, the New menu is no longer collapsible.

sfmskywalker wrote Jul 31, 2013 at 8:41 AM

I see. Thanks for that.

I agree with your first bullet, but I don't think I agree with the second one, because in the case of Vandelays menu, the 'Content' menu item should still link to the Contents list page without a filter by content type. If this menu has a single child item, that will be a content type.

Unless I misunderstood your point. If so, do let me know.


BertrandLeRoy wrote Jul 31, 2013 at 8:26 PM

Agreed: Content should not link to the first child.

sfmskywalker wrote Aug 7, 2013 at 9:27 AM

The issue I see with the Contents Admin Menu provider is as follows: When you enable "Vandelay.ContentAdminMenu", there will be 2 INavigationProvider instances providing a menuitem called "Content". Now, when the NavigationManager builds the entire menu, it merges all of the menu items provided by its sources. During this merge, it joins any menu item with the same name and level. Thus, the 2 Content menu items will be merged into a single one. It will take the value of the "LinkToFirstChild" property of the first item, which is the "Content" menu item provided by the Contents admin menu provider rather than Vandelay's. By default, "LinkToFirstChild" is set to true. This is why the first content type name does not show up underneath "Content".

I agree with the suggestion that "LinkToFirstChild" should only be adhered to if the "effective" URL of the first level menu item equals the URL of the "effective" (non-local nav items) first second level menu item.

This works well for Vandelay's Content Admin Menu, where the first non-local nav item (Page) has a different url than the first level item (Content), as well as for other menu items such as Media, which has a child item called Media. Since the child Media item has effectively the same URL as the first level menu item and LinkToFirstChild is true, it will not render this first child.

sfmskywalker wrote Aug 7, 2013 at 9:32 AM

Fixed in changeset 4d5e51ccf5b2b3caaa1832cdd86f026f37378d7a