Make it easier to add a child menu item

We discussed this issue briefly at 📌 Drupal Usability Meeting 2023-08-04 Needs work . That issue will have a link to a recording of the meeting.

For the record, the attendees at today's usability meeting were @AaronMcHale, @Emma Horrell, @benjifisher, @rkoller, @simohell, and @worldlinemine.

The group agreed that the proposed resolution is a good idea. We also noticed that there is a single link (Edit) for menu links provided by modules, but that custom menu links already use a drop button. (The options are Edit and Delete.) Even the links that cannot be deleted get a drop button with a Reset option if one of the editable fields is changed (such as the weight).

MR 4570 is ready for review.

I am a little nervous about adding Url objects to the form array: although I apply setOption() each time, isn't it still the same object? But it seems to work, giving different href values, in both manual and automated testing.

I was not sure about the condition for adding an "Add child" link based on the maximum depth of the menu, but the automated test confirms that the condition is not off by one.

So I tested this one out.

The child link operation appears. But when I select it the parent item appears to be selecting the wrong one. It selects the last level1 link.

@smustgrave:

Thanks for testing. I think the problem is what I wrote in #5 about adding the same Url object many times to the form array. I did not test enough: it looks as though all sibling links will get the same URL. I think the update to the MR will fix that.

I added a screenshot in Comment #5, but I forgot to add it to the issue summary. I am doing that now.

Two notes for the sake of the code reviewer:

1. The default actions are defined in Drupal\Core\Entity\EntityListBuilder::getDefaultOperations(). Edit has weight 10 and Delete has weight 100.
2. The sort is the same one used in Drupal\Core\Entity\EntityListBuilder::getOperations().

I wonder if it's worth, in this issue, also including links for adding sibling menu items, as in a "Add above" and "Add below" links. I've seen that the ability to quickly add a link above or below the current one could be useful if there are a lot of menu links at the same level, it would reduce the number of steps to getting the link in the position that you want it. I would also be happy to leave that to a follow-up issue if we want to keep the scope here quite tight, but I also suspect that it probably wouldn't be much additional effort to do in this issue.

This looks like a nice feature - the selection of the parent menu link can be tricky if you have a big number of menu links.

I have tested this and the default selection is working in case you are creating a child for a custom menu link. But for some default links it does not work. For example on a clean D10 install, go to main menu and try to add a child to the "Home" link. On the menu link form in the parent link, there is preselected "Main navigation", instead of "--Home". And the URL looks like admin/structure/menu/manage/admin/add?parent=entity.entity_form_mode.collection. On the other hand, if you create a new menu link "test" and then try to add a child to this link, it is preselected correctly. The same is happening also in other menus with the existig default links (for example in the "user menu", etc.).

Beside the menu items, do you think that possibly the similar tweak could be done to taxonomy terms (of course in a separate issue)? The parent terms selection is very similar to the parent menu item one.

@AaronMcHale:

It is tricky to add "Add above" and "Add below" links, since the order depends on weights and, when the weights are the same, alphabetical order. We might need to change weights on many menu items all at once to implement that. I am not sure that I want to implement that at all, and certainly not as part of this issue.

If you trigger the link to "Show row weights" on the listing page, then you can see the weights of the siblings-to-be. (You might have to scroll.) Then, after triggering the new "Add child" link, you can create a new menu item and set the weight on that form.

@poker10:

I am embarrassed that I missed that. Thanks for testing. The fix is simple: when sanitizing the query parameter, I was allowing alphanumeric characters and a few others, but I forgot to allow '.'.

I just added ✨ Make it easier to add a child taxonomy term Needs review as a follow-up issue. I do not have time right now to add screenshots, but that issue should be a little simpler than this one.

Thanks for creating the issue for taxonomy terms!

I can confirm that the issue from #11 is now fixed in the MR.

Should the tests be expanded to test when the "Add child" option is clicked?

Reviewed and applied the MR on local. In the list of menu items we have a parent link and provide two options: Edit and "Add child". It is working as expected. Here are the before and after screenshots for the same.

From #14:

Should the tests be expanded to test when the "Add child" option is clicked?

Yes. Let's add an issue tag for that and set the status to NW.

I rebased on the latest 11.x and resolved a little merge conflict (in the tests) from 🐛 When adding a new menu link, restrict the available parents to the current menu Fixed .

I also added test coverage as suggested in #14, so back to NR.

Tested again by adding 3 menu links.
Clicking Add child link to link 2 and verified it saved under link 2
Created one under link 1 and verified it saved under 1
Created a link under the child link of link 2 and verified it saved under the correct link.

This was perfect timing too so thank you!.

I confirm that the "Add child" functionality is still working on both custom and default menu links. This looks like a very nice usability improvement together with the 🐛 When adding a new menu link, restrict the available parents to the current menu Fixed already committed. LGTM, thanks!

I am setting the status back to NR because @lauriii added a commit and asked a question on the MR.

IMO, the extra commit is fine.

On second though, the preg_replace() is unnecessary, not just arbitrary. I added a commit that removes it (and rebases on the current 11.x).

Should this be a follow up? I see this with or without the patch.

But the MR 4570 tested the same way as before

Tested again by adding 3 menu links.
Clicking Add child link to link 2 and verified it saved under link 2
Created one under link 1 and verified it saved under 1
Created a link under the child link of link 2 and verified it saved under the correct link.

Still working!

The horizontal line through the drop button's select list is not related to this issue.

FWIW, I do not see it. (View the full-size version of the screenshot to see it clearly.)

I tested with Firefox/Linux and Drupal 11.x. I see the same thing with 10.1.2.

I was on Umami, not sure that would impact it though.

lauriii → closed merge request !4570

The bug in #23 is: 🐛 \Drupal\views\Plugin\Menu\ViewsMenuLink::isDeletable returns TRUE even though it doesn't specify delete route Needs review .

Committed 246b571 and pushed to 11.x. Thanks!

I have added this issue to the draft change record for ✨ Make it easier to add a child taxonomy term Needs review , and I am copying the release-notes snippet from that issue to the issue summary here.

Automatically closed - issue fixed for 2 weeks with no activity.