Remove weight field from menu item entity forms

if you disable javascript, you'll see the weight secectors coming back in core.

yet, +1 for UI synching with blocks, on overview
not sure about the node/add chance

See also #605894: Replace menu weight selector on node/add with relative item positioning

I like the idea of removing the weight selector from content creation form. It's not that useful, as it is.

Can we get a reroll? I would definitely favor this, I have not seen one participant in all the testing we have done understand the effects of this feature.

I like the idea, but I think it would be better to keep the form item and only change it to a "hidden" type. This way modules can still use and change this value (I've created a module that does just that).

Here is a patch based on the first one but using "hidden" type.

+1 for removing.

Rerolled version of rickvug's patch from branch 8.x with test case modification.

Changed weight type to value in function menu_edit_item().

I've checked the failing test of the patch posted in #7. It was caused by assertOptionSelected, changing that to assertFieldByName seems to make the test pass. I've also switched back to #default_value instead of #value.

need reroll.

Here is a quick reroll. I'm not sure there is still something to modify in the test file so let's see what the testbot will tell.

if tests are ok that's RTBC for me.

My bad, eventually there was something to change in the test. Bot should be happier now.

still working.

We need to check this for accessibility, because obviously people who cannot see cannot use the drag and drop interface.

Specifically, I mean the menu.admin.inc hunk. Removing it from the node form is consistent with taxonomy terms, as nod_ pointed out in IRC. However, removing it from the admin form creates inconsistency w/ taxonomy terms, for example.

I didn't read through the entire issue.

I agree that removing the weight from the node form is a great idea, it's useless there. If the proposal is also to remove weight from the individual menu item edit pages then that is fine as well (how do you know looking at a single item what the weight should be).

If the proposal is to also remove weight from the menu item listing UI then I question how menu items will be reordered. If this is not part of the proposal then RTBC.

Yes the table with show row weight/hide row weights will still be available.

Hm. When doing this an related removals, then we should create a major follow-up to "fix" the tableDrag JS to "try hard to leave weights of other things in the table alone when dragging an item."

That is, because there are often situations in which only a single item is being moved within others. When doing that in tableDrag, then the weights of all other items that were "crossed" are changed, whereas only a single one was dragged.

Thus, doing so will cause an update to all the items for which the weight was manipulated. Especially for things that live in configuration, this will cause all config objects to be rewritten and changed for the new weights - and in turn, you will see differences for many config objects that need to be staged, even though only a single one was edited.

A typical way to address that is to make tableDrag use larger "gaps" between the weight steps it assigns (i.e., instead of reordering weights into 1,2,3 they should be reordered into 10,20,30). Thus, when another or a new item is later moved in between other items, then the chance for having to adjust surrounding weights significantly decreases, and if it happens, then only one of the direct siblings has to be adjusted.

Lastly, and this is probably a much larger problem than the tableDrag weight issue above:

What if not all items within an hierarchy can be shown on a single administrative page for reordering? AFAIK, this problem existed for menu links in earlier versions, and the only fix we were able to apply was to load all items on a single page (regardless of whether there might be 3,000 of them in a single tree). -- What I'm after is: With the weight element on an individual item, you are able to go into another item to check its weight, go back to the item you want to move, and fill in a similar weight.

I'm not trying to say that we shouldn't do this change, but instead I'd like to make sure that we are addressing all of the consequences of removing the weight selector here (and possibly elsewhere). I think the two issues I outlined above should at least be major tasks in case this lands.

I'm not sure what the accessibility implications of this are, but the patch no longer applies.

Same patch, rerolled.

Where do I go to test this patch? I thought it would be admin/structure/menu/manage/admin/add

A screenshot would be great too.

Lest postpone this issue until: #2256521: [META] New plan, Phase 2: Implement menu links as plugins, including static admin links and views, and custom links with menu_link_content entity, all managed via menu_ui module

Seems like there's no reason to leave this as postponed.

I totally disagree with simply removing the weight field!
Yes, the current state IS a Drupal WTF, but the proposed "fix" would still leave a smaller one.
It will be like saying:

Yes we are unable to create a UI where a user can create (and position) a menu item in one step.

Currently, that's also not possible, as about noone will know what value to use for weight, unless they visited the menu link listing first.
So, my initial thought was "I need to see the weights in the Parent Link select list".

Now, this would not be a really great UI yet, but it would repair the intended use for the weight, and allow users to create AND place a menu item in one step, which would be the expected UX i guess.

A more sophisticated approach was proposed on #605894: Replace menu weight selector on node/add with relative item positioning

PS, rant:
I get another WTF from having all other menus listed at the bottom of the parent link select.
When on the main-menu's add-link page, it makes no sense to me to be able to add the link to some other menu.
This just reduces my overview on selecting the parent.
Am i wrong here?

I agree, that this weight there doesn't help that much

Reroll

Clarified title. Confused with menu list on first glance.

Reformatted IS, no new content.

"needs accessibility review" was added when there was a patch that altered the menu list.

Since then, all iterations are just removing the weight from the menu section of entity forms. - There was no a11y concern about that and is arguably an accessibility improvement. The weights are only useful as they relate to weights of sibling items within a menu, and it's probably best to not give anyone the accidental impression that reordering items in a menu requires having multiple node form tabs open to compare weights.

I started with the patch in #35 and then simply worked on the tests.

So had to run locally as I couldn't run the test-only feature

Reverted the changes besides the 2 test files

Only ran MenuUiNodeTest::testMenuNodeFormWidget as that was the one that had new assertions added. Got

Behat\Mink\Exception\ElementNotFoundException : Form hidden field with id|name|value "menu[weight]" not found.

So test coverage is there

Issue summary appears complete and code change appears to line up.

Maybe we should consider opening a follow-up for some next steps. I think the first next step would be to always add the new menu item to the end of the list. Another next step would be trying to come up with a better UI for users to select where the new menu link should be added.

I think this also needs a change record.

This affects the UX (in a positive way!) and may break some contrib tests so it indeed needs a change record. Perhaps it should also get a Drupal 10.3 release note?

Also, the issue summary states there's no UI changes, whereas that's literally the goal of this issue 😅 Also, the title says "menu item entity forms" — but it's specifically only node forms? (This modifies menu_ui_form_node_form_alter() after all.) The current title would make for a confusing commit message.

(This issue piqued my curiosity because based on the title it seemed related to #3008064: Deprecate vocabulary weight property, but it is not. Hence my request to clarify all that.)