Starting level deeper than 1 ignores active trail, when "Expand all items" is enabled

Created on 22 July 2021, over 3 years ago
Updated 24 January 2023, almost 2 years ago

Problem/Motivation

#3197769: Menu Block missing expand all items option introduced an "Expand all items" block setting that doesn't work in combination with a starting level deeper than 1. With a start level of 2 or deeper, the menu block should only show the branch of the menu tree that corresponds with the current active trail. If "Expand all items" is also selected, the block shows every branch of the tree at that level, regardless of active trail.

Steps to reproduce

Create a group content menu with at least two levels of links in two branches. Place a group content menu block, configured to start at level 2 with "Expand all items" checked. Visit a level 2 link. You will see all level 2 links in every branch.

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

🐛 Bug report
Status

Needs work

Version

1.0

Component

Code

Created by

🇺🇸United States les lim

Live updates comments and jobs are added and updated live.
  • Needs tests

    The change is currently missing an automated test that fails when run with the original code, and succeeds when the bug has been fixed.

Sign in to follow issues

Merge Requests

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

  • 🇫🇮Finland thatguy

    Re-rolled patch from #2 to 3.x-dev version

  • 🇫🇮Finland thatguy

    Re-rolled #4 to work with latest 3.x-dev version

  • 🇺🇸United States daletrexel Minnesota, USA

    Patch in #5 works for latest 2.x-dev as well.

    Have successfully applied and used patch in #5 on a Drupal 10 site running Group and Group Content Menu 2.x branches. We'd previously been using the original patch on the 1.x branch, in production, with success. Patch is still needed for 2.x to do what we need to do. (We upgraded to 2.x along with our Drupal 10 upgrade.)

    Reviewed the patch and the file it patches, src/Plugin/Block/GroupMenuBlock.php:

    - The only difference in this file between 2.x and 3.x branches is line 270, far past the lines the patch touches
    - The patch additions do not include group_relationship/group_content, so it can work with either branch

    Not sure if it's safe to set this RBTC without the requested tests.

  • heddn Nicaragua

    @DaleTrexel, it would be nice to have a test case. I've tried to manually reproduce this situation myself several times and wasn't able to. Perhaps the key is "expand all items"? Anyway, it shouldn't be hard to extend an existing test to confirm this fixes the issue.

  • First commit to issue fork.
  • 🇺🇸United States electrokate

    Re roll patch for Drupal 10.3.1

  • 🇺🇸United States daletrexel Minnesota, USA

    Thanks @electrokate for the updated patch! I've tested in on Group Content Menu 2.0.3 (Group 2.2.2, Drupal 10.2.7) and can confirm that the patch applies. I can also confirm that the update to 2.0.3 does NOT resolve this issue, and that this patch is still required to fix it. (Testing so far only on a local copy of the site I manage, but not yet in production.) +1 RBTC

    A question for those of you following this issue and/or using this patch (and @heddn): is anyone using Group Content Menu with this patch only and not also in conjunction with 📌 Group Menu not visible on Node pages Needs review and/or 📌 More contexts needed Needs work ? I am finally getting a chance to work on adding tests for this issue, but so far I am unable to do so without also patching the module -- which is incompatible with writing tests! Is there any way to set up multi-level menus with Group Content Menu unpatched that I'm overlooking? It feels like this module is extremely limited without #3170013 to provide context for menu blocks placed on node pages.

  • 🇺🇸United States daletrexel Minnesota, USA

    Updating the version to 3.0.x so that the issue fork is off the correct branch.

    We're actually using 2.0.x, but this should apply to both, and we may be able to solve more than one issue with this, so keeping relevance by focusing on the highest version.

    Also simplifying/shortening the title.

  • Merge request !28Fixes for visibility level > 1 → (Open) created by daletrexel
  • Pipeline finished with Success
    5 months ago
    Total: 191s
    #243158
  • Merge request !29Resolve #3224816 "Expand all 2.x" → (Open) created by daletrexel
  • 🇺🇸United States daletrexel Minnesota, USA

    DaleTrexel changed the visibility of the branch 3.0.x to hidden.

  • 🇺🇸United States daletrexel Minnesota, USA

    DaleTrexel changed the visibility of the branch 3224816-starting-level-deeper to hidden.

  • Pipeline finished with Success
    5 months ago
    Total: 186s
    #243721
  • Status changed to Needs review 5 months ago
  • 🇺🇸United States daletrexel Minnesota, USA

    Updating issue description to include new issue brought in by Integrate some of Menu Block module's options Fixed , also surfacing when levels of 2+ are chosen.

    Have updated code, and added tests. #3366930 was also quite useful in showing me how this issue might be tested without having to wait for 📌 Group Menu not visible on Node pages Needs review to be resolved! Thanks for that!

    If possible, I'd like to make sure that Wilbur Ince, @wylbur from Electric Citizen gets credit on this issue. He was instrumental in getting me up and running with phpunit and writing tests. Thanks!

  • 🇺🇸United States daletrexel Minnesota, USA

    The 3224816-3170013-3x-combo and 3224816-3170013-2x-combo branches contain merged code from this issue and my respective merge requests for 📌 Group Menu not visible on Node pages Needs review . These versions may be useful to some, having already worked out minor merge conflicts, but I haven't issued merge requests yet, in case doing so complicates matters for the reviewers.

    These branches have both passed my local phpunit, phpcs and phpstan testing.

  • 🇺🇸United States daletrexel Minnesota, USA

    Adding related issue 📌 Group Menu not visible on Node pages Needs review

  • 🇺🇸United States daletrexel Minnesota, USA

    daletrexel changed the visibility of the branch 3224816-3170013-2x-combo to hidden.

  • 🇺🇸United States daletrexel Minnesota, USA

    daletrexel changed the visibility of the branch 3224816-3170013-3x-combo to hidden.

  • 🇺🇸United States daletrexel Minnesota, USA

    Removed (hid) the combo branches now that 📌 Group Menu not visible on Node pages Needs review has been closed as will-not-fix. Fortunately, we can still provide tests for this issue that don't rely on running a patched Group module (the reason I started working on #3170013 in the first place).

    Still would love to have some eyes on this other than my own!

  • 🇺🇸United States daletrexel Minnesota, USA

    Uploading a patch for the 2.x branch, currently up to date with 2.0.3.

    Something is definitely wrong with the 2.x branch MR !29 at the moment. I can't tell if this is a recent development or not, but I'm surprised because it originally seemed fine and passed tests. But now it seems to be confused with the 3.x branch. Also, I can't create any NEW branches on this issue fork with the "Create new branch" link on this page (it leads to a 404 error), so that suggests it's a Gitlab issue.

    This patch should also work for the 3.x branch of Group Content Menu, because it doesn't touch any code that references the machine names that differ between the 2.x and 3.x branches.

  • 🇫🇮Finland Alexander Tallqvist

    alexander tallqvist changed the visibility of the branch 3224816-3170013-3x-combo to active.

  • 🇫🇮Finland Alexander Tallqvist

    alexander tallqvist changed the visibility of the branch 3224816-3170013-3x-combo to hidden.

  • Status changed to Needs work 28 days ago
  • heddn Nicaragua

    NW because of merge conflicts.

Production build 0.71.5 2024