- ๐บ๐ธ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.
- First commit to issue fork.
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 themain
route param), I shouldn't be able to create a new menu link in thefooter
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
over 1 year ago 8:57am 23 August 2023 - ๐ฌ๐ง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.
- ๐ซ๐ทFrance dqd London | N.Y.C | Paris | Hamburg | Berlin
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.
43:30 15:28 Running- ๐ฎ๐ณIndia omkar.podey
omkar.podey โ made their first commit to this issueโs fork.
- last update
over 1 year ago 30,047 pass, 3 fail - @omkarpodey opened merge request.
- last update
over 1 year ago 30,058 pass - last update
over 1 year ago 30,058 pass - last update
over 1 year ago 30,063 pass - Status changed to Needs review
over 1 year ago 10:19am 29 August 2023 - Status changed to RTBC
over 1 year ago 6:15pm 30 August 2023 - ๐บ๐ธ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
over 1 year ago 30,098 pass -
lauriii โ
committed c90f093c on 11.x
Issue #3110371 by omkar.podey, idebr, gabesullice, joseph.olstad,...
-
lauriii โ
committed c90f093c on 11.x
- ๐ซ๐ฎ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
over 1 year ago 7:51pm 30 August 2023 Automatically closed - issue fixed for 2 weeks with no activity.
- Status changed to Fixed
over 1 year ago 4:07pm 15 September 2023 - ๐จ๐ฆ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.
- ๐ณ๐ฑNetherlands idebr
To limit the parent menu for both create and edit for Drupal 10.3.x, use attached patch.
Rerolled after ๐ When adding a new menu link, restrict the available parents to the current menu Fixed was committed.