show as expanded is forced to true if menu item has children and you are on child trail

Ibrahim Tameme → closed merge request !8506

After checking it turn out that all set as expanded option in Administration menu links are set to false, so with the patch provided above you will need to update all the Administration menu links which is a rough task so this patch ignores the Administration menu and add the condition to other menus.

@Ibrahim Tameme i installed drupal 10.3.x on DDEV v1.22.6 and php 8.3.0 and apply the patch 3456536-fix-menu-expansion-7.patch which is working fine.

This may be a duplicate of ✨ Menu item checkbox "Show as expanded" not working properly Needs work .

I tested this on Drupal 11.x, using the steps in the issue summary. I did not see any difference in the UI between when the patch was not applied and when it was. Is there supposed to be a change?

Changes are made on on 11.x (our main development branch) first, and are then back ported as needed according to our policies.

This seems to fix the issue for me.

Previously {{ item.is_expanded }} was being forced to true when on the active menu item.

Debugging it reveals false when on the active menu item and it has children, even when its not set to "expanded".

" alt="Expanded - True" />

After applying the patch;

And then just to ensure your menu doesn't print out, ensure you have some logic like;

{% if item.below and item.is_expanded %}
  {{ menus.menu_links(item.below, attributes, menu_level + 1, item) }}
{% endif %}

Thanks for the patch!

I tried to update the issue summary to be a little more clear. I do think this is a duplicate of ✨ Menu item checkbox "Show as expanded" not working properly Needs work , but I think the approach in this issue is the correct way to go so I'd recommend closing the other one.

To @aaronchristian's point, I think this patch is only complete with a change to the template file. Maybe I'm wrong, but I think we shouldn't even be building the subtrees of a menu item if it's not expanded. And if we do that, we won't need a change to a template file. I'll take a stab at that in an MR now.

They read as duplicates to me, both are talking about confusion over the option and how it affects hardcoded behavior.

I was recently diving into how menus load, and learned about using MenuLinkTree service to load the menu. At first I was trying to stuff extra content into the items with a custom modifier for MenuLinkTree->transform(). But then I discovered that those are more for controlling the structure of the menu, like removing links you don't have access to, or changing the order. But loading additional content is intended to be in a preprocess.

I took notes on things I discovered here: https://www.drupalarchitect.info/articles/rendering-menus

I think the question is where we should make the decision on how menus render. If everything is available, the decision can be made in the template. If the child links are removed further back in the render pipeline, they take deeper development to modify. For example, adding a custom handler to adjust the link loading takes way more effort than deciding not to print level 3 items in a template file.

That said, I think default menu templates should be able to handle whatever links are passed in, and the UI should give us clear control over what is happening per menu display.

Actually, scratch pretty much everything I just said.

I'm seeing in MenuLinkTree::getCurrentRouteMenuTreeParameters(), we are explicitly expanding links in the active trail:

/**
   * {@inheritdoc}
   */
  public function getCurrentRouteMenuTreeParameters($menu_name) {
    $active_trail = $this->menuActiveTrail->getActiveTrailIds($menu_name);

    $parameters = new MenuTreeParameters();
    $parameters->setActiveTrail($active_trail)
      // We want links in the active trail to be expanded.
      ->addExpandedParents($active_trail)
      // We marked the links in the active trail to be expanded, but we also
      // want their descendants that have the "expanded" flag enabled to be
      // expanded.
      ->addExpandedParents($this->treeStorage->getExpanded($menu_name, $active_trail));

    return $parameters;
  }

If we're going to do something here, I think we have to do it there.

Doing it in SystemMenuBlock makes sense to me, that's where the "Expand all menu links" checkbox is. An additional checkbox for "Don't auto-expand items in the active menu trail"?

@mortona2k, I was just coming back to this after sleeping on this to say basically the same thing! Except I think I'd recommend the additional checkbox be "Expand items in the active trail" and we can default it to checked.

That makes more sense.