When adding a new menu link, restrict the available parents to the current menu

Created on 31 January 2020, over 4 years ago
Updated 19 February 2024, 4 months ago

Problem/Motivation

When adding a menu link through the Menu UI, the form present a field 'Parent link' in order to create a hierarchical menu structure. However, these options are not limited to the current menu but show all menus available in the database.

For sites using multiple menus, this list quickly becomes unwieldy.
For sites using a lot of menus, this list can create a performance bottleneck because of the large number of menu items being loaded.

Proposed resolution

Limit menu link 'Parent link' to the current menu, only when creating new menu items - menu items are always created after choosing a menu anyway.

The edit form remains unchanged so you can move a menu link from one menu to another.

Remaining tasks

Update the test coverage.

User interface changes

The menu link 'Parent link' field is limited to the current menu on the menu link add form.

API changes

None.

Data model changes

None.

๐Ÿ› Bug report
Status

Fixed

Version

10.2 โœจ

Component
Menu UIย  โ†’

Last updated 4 days ago

No maintainer
Created by

๐Ÿ‡ณ๐Ÿ‡ฑNetherlands idebr

Live updates comments and jobs are added and updated live.
  • Performance

    It affects performance. It is often combined with the Needs profiling tag.

  • Usability

    Makes Drupal easier to use. Preferred over UX, D7UX, etc.

Sign in to follow issues

Comments & Activities

Not all content is available!

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

  • ๐Ÿ‡บ๐Ÿ‡ธUnited States xjm

    While the performance issue of the menu parent select field is definitely an issue on large sites, I don't think this actually needs product manager review. The current approach is not workable because it is a significant feature regression.

    Marking as a duplicate of โœจ Scalable menu selector Closed: duplicate which has the scope we need of improving the widget (rather than just removing part of its functionality). Thanks!

  • ๐Ÿ‡บ๐Ÿ‡ธUnited States xjm

    Actually ๐Ÿ“Œ Improve usability, accessibility, and scalability of long select lists Needs work is the best issue here -- I recommend focusing on that, since we need this same fix for other select fields too.

  • ๐Ÿ‡บ๐Ÿ‡ธUnited States xjm

    Oops, half-typed re-title.

  • First commit to issue fork.
  • ๐Ÿ‡ซ๐Ÿ‡ทFrance gabesullice ๐Ÿ‡ซ๐Ÿ‡ท โ‘ โ‘ข UTC+2

    The current approach is not workable because it is a significant feature regression.

    @xjm, I think there's a legitimate bug here, but the issue went a little off track by mixing in performance and scalability concerns.

    What's the bug?

    In short, if I'm on the /admin/structure/menu/manage/main/add route (note the main route param), I shouldn't be able to create a new menu link in the footer menu, but I can.

    To expand on that: imagine if you're editing the and you follow the button. Now also imagine that you probably have a menu link with the same name in both the and menu. On the create form, you accidentally choose the link in the menu because you (understandably) assumed that you're adding a link to the . Worse, after you click save, you'll be redirected back to the overview and you'll be presented with a green status message saying "The menu link has been saved," however you will not see your newly created menu link since it was actually added to the menu.b

  • @gabesullice opened merge request.
  • ๐Ÿ‡จ๐Ÿ‡ฆCanada joseph.olstad

    @gabesullice, yes you've nailed it, it's also a performance issue, if one of your menus has 3000 links like ours, most of them disabled and used by menu_breadcrumbs, then you're going to have performance issues.

  • Status changed to Needs work 10 months ago
  • ๐Ÿ‡ฌ๐Ÿ‡งUnited Kingdom catch

    Agreed with #39, there's a genuine usability issue with the menu create form that's separate from the scalability one.

    When you in admin/structure/menu and want to add a new content menu link, you're presented with possible parents of every menu item in every menu on the site, this can be hundreds. In my case, the menu I wanted to add to had five items. Fat-fingering the scroll, or missing a -- and you can be lost somewhere in admin/config with no obvious way to get back to the menu you're actually trying to change.

    If we do ๐Ÿ“Œ Improve usability, accessibility, and scalability of long select lists Needs work it would make the massive select list more manageable, but it'd still be unnecessarily long due to all the irrelevant menus in there.

    If we only make this change on create, then if you really do want to change your mind, you can just back out and go to a different menu and add the link here, so there's no functionality regression only the usability improvement.

    Patch looks fine I think but needs test updates - we should probably expand the coverage so it handles both the create and edit cases.

  • ๐Ÿ‡ฉ๐Ÿ‡ชGermany diqidoq Berlin | Hamburg | New York | London | Paris

    Agreed with @catch. Not sure if it even is a duplicate in any sense, 1.) because there are already interesting patch attempts here in it and 2.) the other one is on a bigger scope than menus only and can be seen as related or parent but not as replacement IMHO.

  • ๐Ÿ‡ซ๐Ÿ‡ฎFinland lauriii Finland

    This seems totally reasonable. I'm marking this as a bug since this is certainly a usability issue that could prevent someone from finding the correct parent.

    Checked the latest MR and the solution looks good, and I don't see how this could be a regression. ๐Ÿ’ฏ I think we just need a test case to confirm it works and that it continues to do so.

  • First commit to issue fork.
  • 28:36
    0:34
    Running
  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia omkar.podey

    omkar.podey โ†’ made their first commit to this issueโ€™s fork.

  • last update 10 months ago
    30,047 pass, 3 fail
  • @omkarpodey opened merge request.
  • last update 10 months ago
    30,058 pass
  • last update 10 months ago
    30,058 pass
  • last update 10 months ago
    30,063 pass
  • Status changed to Needs review 10 months ago
  • Status changed to RTBC 10 months ago
  • ๐Ÿ‡บ๐Ÿ‡ธUnited States smustgrave

    Removing needs tests tag as it seems they were added in the MR with some assertions to existing tests.

    This change seems like a no brainer and works as described!

    Updated change record slightly with screenshots and branch numbers.

  • last update 10 months ago
    30,098 pass
  • ๐Ÿ‡ซ๐Ÿ‡ฎFinland lauriii Finland

    Opened a follow-up to address this in the edit menu links use case: ๐Ÿ› The menu UI selector is not scalable Active .

    Committed c90f093 and pushed to 11.x. Thanks!

  • Status changed to Fixed 10 months ago
  • Automatically closed - issue fixed for 2 weeks with no activity.

  • Status changed to Fixed 9 months ago
  • ๐Ÿ‡จ๐Ÿ‡ฆCanada joseph.olstad

    Great to see this one go in!
    Thanks @omkar.podey for the work on the automated tests.

  • ๐Ÿ‡ต๐Ÿ‡ฑPoland tivi22

    Hey, could you please make this patch working with Drupal 10.2?

  • ๐Ÿ‡ณ๐Ÿ‡ฑNetherlands idebr

    This issue was released in Drupal 10.2.0.

    To limit the parent menu for both create and edit, use attached patch.

  • ๐Ÿ‡ต๐Ÿ‡ฑPoland tivi22

    Thanks @idebr

Production build 0.69.0 2024