Problem/Motivation
- The interface is simpler.
- The functionality provided is not very helpful. The majority of users have no idea of the weight assigned to other menu items. This is a frustrating experience for users and a definite "DrupalWTF". Drag and drop menus are an intuitive experience.
- Weight has been removed from other user facing systems. Are there any other areas of core that still show a weight selector like this?
- The change is consistent with blocks. You can not set the weight of a block under its configuration page.
Steps to reproduce
Proposed resolution
Removes weight from the user interface for adding/editing menu items
Remaining tasks
User interface changes
API changes
Data model changes
Release notes snippet
Comment | File | Size | Author |
---|
Issue fork drupal-491022
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
Comments
Comment #1
alexanderpas CreditAttribution: alexanderpas commentedif 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
Comment #3
sunSee also #605894: Replace menu weight selector on node/add with relative item positioning
Comment #4
dodorama CreditAttribution: dodorama commentedI like the idea of removing the weight selector from content creation form. It's not that useful, as it is.
Comment #5
rickvug CreditAttribution: rickvug commentedUnfortunately at this point I think this is D8 material.
Comment #6
Bojhan CreditAttribution: Bojhan commentedCan 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.
Comment #7
idflood CreditAttribution: idflood commentedI 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.
Comment #9
Sivaji_Ganesh_Jojodae CreditAttribution: Sivaji_Ganesh_Jojodae commented+1 for removing.
Rerolled version of rickvug's patch from branch 8.x with test case modification.
Comment #11
Sivaji_Ganesh_Jojodae CreditAttribution: Sivaji_Ganesh_Jojodae commentedChanged weight type to value in function menu_edit_item().
Comment #13
idflood CreditAttribution: idflood commentedI'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.
Comment #14
nod_need reroll.
Comment #15
idflood CreditAttribution: idflood commentedHere 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.
Comment #16
nod_if tests are ok that's RTBC for me.
Comment #18
idflood CreditAttribution: idflood commentedMy bad, eventually there was something to change in the test. Bot should be happier now.
Comment #19
idflood CreditAttribution: idflood commentedComment #20
nod_still working.
Comment #21
webchickWe need to check this for accessibility, because obviously people who cannot see cannot use the drag and drop interface.
Comment #22
webchickSpecifically, 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.
Comment #23
Everett Zufelt CreditAttribution: Everett Zufelt commentedI 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.
Comment #24
nod_Yes the table with show row weight/hide row weights will still be available.
Comment #25
sunHm. 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.
Comment #26
mgiffordI'm not sure what the accessibility implications of this are, but the patch no longer applies.
Comment #27
superspring CreditAttribution: superspring commentedSame patch, rerolled.
Comment #28
mgiffordWhere do I go to test this patch? I thought it would be admin/structure/menu/manage/admin/add
A screenshot would be great too.
Comment #29
pwolanin CreditAttribution: pwolanin commentedLest 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
Comment #31
mgiffordSeems like there's no reason to leave this as postponed.
Comment #33
sirtetI 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:
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?
Comment #34
dawehnerI agree, that this weight there doesn't help that much
Comment #35
alvar0hurtad0Reroll
Comment #39
dpiClarified title. Confused with menu list on first glance.
Reformatted IS, no new content.
Comment #52
bnjmnm"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.
Comment #56
quietone CreditAttribution: quietone at PreviousNext commentedComment #57
quietone CreditAttribution: quietone at PreviousNext commentedI started with the patch in #35 and then simply worked on the tests.
Comment #58
smustgrave CreditAttribution: smustgrave at Mobomo commentedSo 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.
Comment #59
lauriiiMaybe 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.
Comment #60
Wim LeersThis 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.Comment #61
Wim Leers(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.)