Problem/Motivation

I have a menu structure like this:

  • Europe
    • Northern Europe
      • The Netherlands
    • Southern Europe
      • Italy
  • America
    • North America
      • Canada
    • South America
      • Colombia

Block configuration:

  • Initial menu level: 2
  • Maximum number of menu levels to display: Unlimited
  • [x] Expand all menu links
  • Fixed parent item:

Proposed resolution

Make the Drupal 8 menu block 2+ behave like in Drupal 7

Remaining tasks

t.b.d.

API changes

t.b.d.

Members fund testing for the Drupal project. Drupal Association Learn more

Comments

matsbla created an issue. See original summary.

matsbla’s picture

Category: Feature request » Support request
Status: Active » Closed (works as designed)
matsbla’s picture

Category: Support request » Feature request
Issue summary: View changes
Status: Closed (works as designed) » Active

Updating issue to describe the problem better.

matsbla’s picture

Issue summary: View changes
mogtofu33’s picture

Status: Active » Needs review
FileSize
2.18 KB

Here is a patch for this functionality.
Add 2 options for "Fix parent":

  • Active trail (Set current page as menu root)
  • Active trail parent (If current page has a parent, set this parent as menu root. When you want siblings menu)

Please review and test.

Regards.

matsbla’s picture

Status: Needs review » Reviewed & tested by the community

That patch came fast!
Works perfect :)

Thanks a lot!!

johndevman’s picture

#5 works as expected. We had an additional use case where we needed the active trail parent, but not necessarily the first one. How about adding an additional option to the "Active trail parent" which allows you to select parent depth.

kevinquillen’s picture

Not sure this is working exactly as it should.

When I have 'Active Trail Parent' set, it does not make the menu block 'stick' for the children links. No menu shows at all for a third level item, but does show for a second level item.

I want the second level of the menu tree to always show if I am on a third level item. Not just the parent. "Active Trail" should be what I want (I think), but the menu just goes away.

parent1
- child 1.1
--- child X
- child 1.2
parent2
- child 2.1
- child 2.2

When I am on the parent1 node, I should see the entire tree below it, same for if I click on child 1.1, I want to stick at that level. Child X, same, show that first level tree.

kevinquillen’s picture

Status: Reviewed & tested by the community » Needs work
kevinquillen’s picture

Just poking around a little:

// Active trail parent.
        else {
          array_pop($trailIds);
          $menuLinkID = end($trailIds);
        }

This will always select the previous link. If I edit that and make it uses the first link, I get the desired result. How can we implement an option to tell it which level to force the trail to begin from?

matsbla’s picture

You are right, seem like this is not entirely working with several menu levels!

To me it also seems like it counts "Initial menu level" from current active node, and not the menu tree itself, so the menu is displayed at some levels, and others not.

Maybe it would make more sense to only fix specific menus inside "Fixed parent item" and then have a check-box under to filter menu options on that fixed parent item to active trail / active parent trail?

kevinquillen’s picture

I think you need to allow fixed parent level, to 'pin' the menu tree at a given spot for anything under it.

I was able to do it by adapting it to:

       else {
          //array_pop($trailIds);
          reset($trailIds);
          $menuLinkID = key($trailIds);
        }

I don't pop the array because at the first level, it removes the parent, which doesn't generate a tree. Now, when I visit anything at that level, the entire tree under it shows for that section just the way I want it to. No matter where I am now after the first level, I see the menu tree in my left sidebar. The depth is still capped at 3 (for me) and this appears to work though is by no means a true solution.

I think we need a new option to enable this and update the code to reflect this. Some people might like the menu constantly drilling into the tree with parent depth -1, but I just want to pin an entire section of the tree no matter where I am.

kevinquillen’s picture

FileSize
4.9 KB

Here are some changes incorporating the original patch.

I did not have a lot of time to dig into it, but I added an option "Fix to parent level" for the active trail parent option. When enabled, it should use the 'initial menu level' as the root level to set for menu block.

I tried a few different kinds of logic to get the menu link ID. While each one was correct, the tree below is never built. Hopefully someone can view and expand on this.

Sutharsan’s picture

Title: New advanced option: Fixed parent item to active trail » 2+ level menu block not limited to active parent
Category: Feature request » Bug report
Issue summary: View changes
FileSize
43.97 KB
77.15 KB

If I compare the Drupal 8 module behavior to the Drupal 7 module behavior, I consider the Drupal 8 behavior as a bug. I have made a menu structure and configured two blocks. One to show the top level, one to show the levels below the top.

  • Europe
    • Northern Europe
      • The Netherlands
    • Southern Europe
      • Italy
  • America
    • North America
      • Canada
    • South America
      • Colombia

"Below top" configuration:

  • Menu: Main menu
  • Starting level: 2nd level (secondary)
  • Maximum depth: Unlimited
  • [x] Expand all children of this tree.
  • Fixed parent item:

Drupal 7 result:

Drupal 8 result:

Sutharsan’s picture

Status: Needs work » Needs review
FileSize
2.08 KB

Inspired by the Drupal 7 menu_tree_prune_* functions, I've created a manipulator (like \Drupal\Core\Menu\DefaultMenuLinkTreeManipulators) that prunes the menu tree and saves the active tree.

Note: Not providing an interdiff as the patch takes a different approach.

vlad.dancer’s picture

FileSize
62.77 KB

I consider the Drupal 8 behavior as a bug

I agree with you. Let's imagine that tree on attached screenshot has the whole items expanded ))

Big menu structure

I've fixed this using a bit easier approach, patch will come later:

// If expandedParents is empty, the whole menu tree is built.
if ($expand && !$expand_only_active_trails) {
    $parameters->expandedParents = array();
}
vlad.dancer’s picture

Also I think we have two different feature requests and one bug. I assume that this issue should be splitted into separate ones.

vlad.dancer’s picture

Here is an alternative patch. It will add a new option in the block configuration "Expand only active tree", when you select "Expand all menu links" option.

johndevman’s picture

#12 gave me an working solution for all of our use cases.

matsbla’s picture

Both #15 and #18 are working. However there is a small difference about which menu items that are displayed at the inital menu level; In 18 you only see the active trail (no other options at initial menu level) whereas in #15 you will also see other menu items at initial menu level with same parents as the active trail (like displayed at screenshot in issue). I guess both results can be desirable in different scenarios and maybe it should be configurable, however to me #15 seem most intuitive (however maybe it should be a default instead of a config option?)

Patch in #13 & #5 are not working, and I do not think we should solve this issue by adding any dynamic options under "Fixed parent item".

nelslynn’s picture

#18 works great for me! I might add, it also works with this patch. The patch combination makes the module just like the Drupal 7 version. It would be nice if both patches were committed :)

Thanks!

aaronbauman’s picture

Status: Needs review » Closed (duplicate)
aaronbauman’s picture

rmarques’s picture

#13: the patch you posted seems to work although it doesn't seem related with this issue but with this one https://www.drupal.org/node/2756675

It would be nice to publish the patch there.

Gertjan.k’s picture

I didn't get the expected results with all the patches. I've rerolled #2631468-95: Menu subtrees in menu blocks show all subitems regardless of the active menu item to fix this when using MenuBlock.

Syndz’s picture

Status: Closed (duplicate) » Needs review
FileSize
1.95 KB

Reopened the issue as the menu block currently complete overwrites the SystemMenuBlock::build method and therefore does not profit from the core patch.
MenuBlock::build should probably extend SystemMenuBlock::build rather than overwrite it in the future.

The patch in #25 breaks the "Fixed parent item" functionality.
Here's a new patch that should solve that problem.

kevinquillen’s picture

I am running into an issue which may or may not be related to the changes made in this thread:

#2859703: Incorrect parent tree shows when the same URL is used in menu links many times

I cannot figure out how to get menu block to behave correctly when multiple links to the same content are present in the same menu.

petergus’s picture

I wanted to show the root level item but I could not so I had to resort to this patch to show the highest parent in title.

Suggesting a way to also show the highest parent. i.e. see parent 1 and everything below when on parent 1 or anything below.

Parent 1
- Child 1.1
-- Grandchild 1.1.1
-- Grandchild 1.1.2
- Child 1.2
-- Grandchild 1.2.1
-- Grandchild 1.2.2
Parent 2
- Child 2.2
-- Grandchild 2.2.1
-- Grandchild 2.2.2

yannickoo’s picture

The patch from #26 works nice, thank you Syndz :)

I have renamed the variable $fixedParentMenuLinkId to $fixed_parent_menu_link_id and moved the max depth into a variable.

RumyanaRuseva’s picture

Status: Needs review » Reviewed & tested by the community

Patch #29 works perfectly, the code also seems correct.

Thank you yannickoo, Syndz and everyone.

johndevman’s picture

if ($level === 1 || $level === '1') {

Why do we test it for being a string as well?

Syndz’s picture

That's a good question johndevman, but that part of the code isn't a part of this patch, perhaps a new issue should be created for that question?

johndevman’s picture

I only raised the concern as the patch does implicitly assume it's an int. PHP magically casts it to an int in any case but I would say it's better practise to make sure it is actually is an int by explicitly casting it.

matsbla’s picture

#26 works for me.
+1 for fixing this bug soon!

gsquirrel’s picture

I have also tried #29 patch and it does seem to work, restricting the 2nd level links to those under current menu trail/parent..

Miri Meltzer’s picture

patch #29 worked fine.
@Syndz & @yannickoo, Thanks.

joelstein’s picture

#29 works great for me, thank you!

rferguson’s picture

Can #29 get rolled into the stable release? Just tried, and it works.

RaspberryBlack’s picture

#29 works for me too, thank you!

nickvanboven’s picture

#29 works for me, thank you!

devil2005’s picture

Is it possible to publish a dev version with all patches ?

Thanks

caiobianchi’s picture

+1 for a patched dev version.

jimkeller’s picture

See my comment here for a patch that includes three of the major outstanding issues, including this one: https://www.drupal.org/node/2757143#comment-12137846

Is there any update on when these patches can get added to the release version?

typhonius’s picture

Patch for the dev version attached.

woutervu’s picture

Combined #5 (mogtofu33) and #18 (vlad.dancer) and added functionality:

  • Custom active trail level (relative to the active trail).
  • Ability to hide the children of the active trail parent.

--
These were things that were missing for our use case.

devil2005’s picture

I tested these 3 patches applied on stable version on the module but nothing happened

woutervu’s picture

My patch combines #5 and #18. Do they work seperately?
Do you see nothing in the advanced settings of your menu block? If so, you may need to recreate the block and check again.

devil2005’s picture

Patch apply require manual workflow, automatic is broken...
So i have applied #5 then #8 then #45... And now it seem to work... Don't know why.

Thanks for the patch :)