Redesign the menu link add form to be less overwhelming

While the 'weight' element is grouped as a secondary item, it unexpectedly appears in the primary section of the menu link edit form, rather than being grouped with the other secondary elements in the column. I'm unsure about the reason behind this behavior and would appreciate some insights.

The weight element in the menu link edit form needed to be wrapped in a container to be grouped along with the other secondary elements.

Still needs usability sign off I believe.

But testing MR 4792 and the form does update correctly, matching screenshot in #33.

The space between the center column and right is awkward but realize that's just a claro feature, but wanted to mention.

Think this is a nice little improvement for creating menu links.

Usability review

We discussed this issue at 📌 Drupal Usability Meeting 2023-10-06 Active . 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.

@srishtiiee, thanks for working on this issue. It has been idle for too long. We like the general idea a lot, but would like to see several changes:

1. The organizing principle should be that we put data in the main area of the form and metadata in the sidebar. The more important, or frequently edited, elements should be put at the top of each region. The specific recommendation is that we move the "Parent link" select list to the top of the sidebar.
2. Other than the Parent link, the sidebar elements should be put in details elements, collapsed by default. This is consistent with the node edit form, and it will reduce visual clutter. The long descriptions (help text) are the main clutter. We do not have strong opinions on the exact order, but we suggest
   • Parent link (no details element)
   • Checkboxes ("Enabled" and "Show as expanded"). To be decided: the label for this details element.
   • Description
   • Weight
3. On a narrow screen, the Save button is between the main section and the sidebar. It should be at the bottom of the page.
4. Follow up: let's review the description text.
5. Follow up: on wide screens (or with small text) the main region and the sidebar have a lot of whitespace between them. Move the whitespace to the left and right. This applies to the node edit form as well.

I have added the issue tag for follow-ups because of the last two points. I also added the tag for an issue summary update: I would like more detail (something like my list in (2)) in the "Proposed resolution" section and "before" and "after" screenshots in the "User interface changes" section.

Addressed the feedback and update issue summary.

Test failure may be related to issue. And can the follow up be opened too. Leaving in review though and will look more tomorrow

Back to NW for test failure.

With regards to #5 follow up going to have and dig through some digs but there may be one already for that.

I think the suggestions from the usability review have been implemented. Thanks!

We did not recommend a label for the details element containing the "Enabled" and "Show as expanded" checkboxes. But I think that "Configuration" is too generic.

I am setting the status to NW for a better label and for the comments I made on the MR.

Ok so next step is to rework menu-link-form render to match as much as possible node-edit-form render

Here are the screens after my last changes:

There is an issue with bold border between first an second section, which isn't a case for node-edit form.

While testing 📌 Improve menu parent link selection Needs work i still had the changes for this issue applied by accident. Turns out both issues are intertwined:

Quickly chatted with @benjifisher in the #accessibility channel where he posted the other issue and we were in agreement that the menu select field should be moved over to the sidebar right above the parent link select field. He recommended to post this comment on both issues and whichever issue is fixed second should adapt for the other.

@kostyashupenko:

Thanks for the updates. You resolved many of the comments I made yesterday. I do not have the GitLab permission to mark threads as resolved, but I gave a :+1: reaction (thumb up) for the ones that are complete.

There are a few points from that review that should still be fixed, and I added a few new ones.

If you make further updates, please comment here and change the status to NR to make sure that I see when the MR is ready for re-review.

Addressed the feedback.
Also fixed the border to match node edit form.

Follow up: let's review the description text.

- 📌 Review menu form description text Active

Follow up: on wide screens (or with small text) the main region and the sidebar have a lot of whitespace between them. Move the whitespace to the left and right. This applies to the node edit form as well.

- 🐛 Layout Issues with Claro theme Needs work

Attached the follow ups requested in #36

There are still a few (3?) points on the MR that need to be addressed.

We still have to decide on a label for the fieldset containing he "Enabled" and "Show as expanded" checkboxes. I think "Configuration" is too generic.

I discussed the label on Slack with @rkoller and @smustgrave. We agreed that "Display settings" is clearer than "Options". If you think of something even clearer, use that.

@benji thanks for pointing that out that css file (node.module.css) is never used, created a follow-up to remove all references to it 📌 node.module.css is obsolete and can be removed Active . I removed the template_preprocess_hook from this MR. Also, the label for the details element containing the checkboxes is changed to "Display settings" as per the suggestion.

Believe this is good to go!

The MR is starting to look pretty good. I was testing this with bunch of contributed modules to make sure that this is at least somewhat compatible with them, and didn't find anything concerning.

However, I do have one concern regarding the current grouping, specifically regarding the enabled checkbox. Content editors for smaller sites (sites without strict editorial workflows) use this sometimes to create disabled menu links for the link to be enabled later. For this reason, this has different priority compared to the other field currently in the advanced section, especially on the form to create new menu links.

@lauriii:

If I remember correctly, we discussed the Enabled checkbox at 📌 Drupal Usability Meeting 2023-10-06 Active (see Comment #36 on this issue). There are reasons to show it in the main area or in the sidebar ("advanced") area, and we did not object to either decision.

In addition to the reason you gave in #56, some other advantages to keeping this checkbox in the main area are

1. Be more consistent with the node form.
2. Since there will be only one checkbox in the details element, we can just use "Expanded" as the label.

... especially on the form to create new menu links.

We did not consider the option of having the Enabled checkbox in one place on the add form and a different place on the edit form. Personally, I think we should make the two forms consistent.

Oops, I did not mean to change the status.

Moved the 'enabled' checkbox to main area, changed label for the details element that only has the 'expanded' checkbox and added support for 'description' in edit form along with the add form.

All the latest changes look great! the only minor nitpick i would have might be the label for the description field and field set. the field is basically providing the string that is used for the title attribute while descriptions are usually used in the admin ui as a description and explanation about a certain component - directly visible (in list views or on pages adding an entity) but not available on hover in a tooltip. that difference is outlined well if you compare the use of description here with 📌 Make Description Field Labels Consistent Needs work . strictly speaking the use of description makes this issue part of the discussion in 📌 Make Description Field Labels Consistent Needs work as well imho. I think it might be a good thing to discuss 📌 Make Description Field Labels Consistent Needs work including the use of description in this issue in the usability meeting next friday. but from my point of view that shouldn't hold back the issue here, potential naming changes could be covered in the already open followup issue 📌 Review menu form description text Active imho?

@rkoller: It seems to me that changing the label is out of scope for this issue. We could discuss it on #3365222 or #3395365.

I think the next step here is code review for the recent changes.

Applying the MR I can confirm I'm seeing the same as the screenshots in #59

Another difference between this template and node-edit-form.html.twig is that the other file has one line after this one:

<div class="divider"></div>

It would make the UX more consistent if we include that line here. Is there a reason to leave it out?

I see in this comment 📌 Redesign the menu link add form to be less overwhelming Fixed that specifically asks for adding the line because the node edit form has it.
The function of the line on the node form was to visually help identify the end of complex forms (assumed for content). This small form doesn’t need a visual clue that identifies the ends, and on the other side it add visual noise. So moving this back to needs work to remove the line.

On a side note, I'd love to clarify that this solution should be considered an exception and not a pattern. The reason is that in the new designs that are coming (because of the layout changes 📌 [META] Layout redesign Active and the visual design updates it will force) the visual relation between the main left form and the existing sidebar will be smaller and it will make it complicated to perceive them as part of the same form if it's small. The current purpose of the sidebar is to contain "settings", not content as a "description" as is happening here. I 100% agree with the need to simplify the form and prioritize elements in forms, though. This is something we need to fix with new design patterns instead of limiting ourselves to the ones existing today. So for now I think this is a visual improvement, but let's try to solve it holistically before replicating this pattern elsewhere. And if anybody want to help with the design work please reach out or join the #admin-ui channel. :)

Addressed the feedback in #63 📌 Redesign the menu link add form to be less overwhelming Fixed . Marking it NR again.

Believe #63 has been addressed.

@srishtiiee and @Utkarsh_33: Thanks for addressing the feedback on this issue.

@ckrina: Thanks for the design guidance and the reference to 📌 [META] Layout redesign Active . I notice links on that issue to #3158854: Node form: address chasm between main form and meta → and 📌 Node form layout looks awkward on wide screens since #3158854 Fixed , so I guess #36.5 is a long-standing problem and a difficult design problem.

I did not do any testing, since that was done in #60, #62, and #65. I did review the code changes. +1 for RTBC.

Posted some feedback on the MR

This one turned more complicated then I thought. But appears all feedback has been addressed again.

Great work on this! What a great improvement.

Additional feedback on the MR from @lauriii needs to be addressed.

Additionally, I'm not sure that "Expanded" is a good label for the metadata accordion. It's confusing out of context. I see that "Expanded" replaced "Configuration" from before.

After manually testing this, I'm also not sure having one element per collapsed accordion element is the best UX. Then the user has to click to expand each element to see what it contains and modify it if desired.

Compare with the node form accordion:

Each accordion element's label is a noun ("settings", "information", "options" etc. being modified by what kind of setting it is). Some of the elements are grouped logically (like the promotion options) while others are standalone.

I think the accordion should be as follows:

1. Description
2. Display settings (containing both the weight and the expansion setting).

At a minimum, I think labeling each accordion element with a noun is required.

Let's try implementing that and then re-offer it for usability review. Let's also include a screenshot of the elements being both collapsed and expanded for easier review.

Thanks!

I have regrouped the form elements as per the comment in #70 📌 Redesign the menu link add form to be less overwhelming Fixed . Also attaching the screenshots for the same .

Thanks @Utkarsh_33. Let's see what the usability team thinks of that change.

The Needs Review Queue Bot → tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

This does not mean that the patch needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide → to find step-by-step guides for working with issues.

Rebased

@kostyashupenko, I checked with the MR #75 mentioned earlier, but it didn't apply successfully. There were errors in four files:

core/themes/claro/css/layout/form-two-columns.css
core/themes/claro/css/layout/form-two-columns.pcss.css
core/themes/claro/css/layout/menu-link-edit.css
core/themes/claro/css/layout/menu-link-edit.pcss.css.

@Nitin shrivastava:

I do not see any problem with MR 4796. If you still see something wrong, please give more details so that someone else can reproduce the problem.

Usability review

Thanks to @Utkarsh_33 and others for continuing to work on this issue.

We discussed this issue at 📌 Drupal Usability Meeting 2023-11-24 Needs work . That issue has a link to a recording of the meeting.

For the record, the attendees at the usability meeting were @AaronMcHale, @anushrikumari, @benjifisher, @ckrina, @rkoller, and @simohell.

We agree that the changes suggested in #70 make a further improvement. But we think it will be a little better if we rearrange things a bit.

It makes sense to hide the weight in a collapsed details element, since most people will use the overview page (like /admin/structure/menu/manage/admin) to rearrange menu items, not this form. On the other hand, the weight is logically connected to the parent link. Together, the parent link and the weight determine where the link will appear. (Alphabetical order also matters.) We think the best compromise between these two points is to rearrange the items in the "Advanced" section. Specifically,

1. Arrange the elements in the order Parent link, Description, Display settings.
2. Within Display settings, arrange Weight first, then "Show as expanded".

That way, when Display settings is expanded, Weight will be as close as possible to Parent link.

Keep the details elements collapsed by default, as they are now.

If you want more feedback from the usability team, a good way to reach out is in the #ux channel in Slack.

I have addressed the changes requested in #77 📌 Redesign the menu link add form to be less overwhelming Fixed . Attaching the screenshots for the same:

Woo fingers crossed this lands. UX feedback appears to have been addressed.

My mistake: in #77, I wrote, "... Parent link, Description, Display settings." I meant to say "... Parent link, Display settings, Description." Again, the goal is to have Weight next to Parent link, when the details element is open.

I also wrote, "Keep the details elements collapsed by default, as they are now." In the screenshot, and in my test with the current MR, the "Display settings" details element is open by default.

From the MR:

I am not a 100% sure about adding the weight to the element here. I tried to add this in claro.theme file but it was not rendering as expected.

Yes, this is a problem. Instead of '#group' => 'menu_link_weight',, try something like this in the helper function:

  $form['weight']['#weight'] = 0;
  $form['menu_link_display_settings']['weight'] = $form['weight'];
  unset($form['weight']);
  $form['menu_link_display_settings']['expanded'] = $form['expanded'];
  unset($form['expanded']);

It will take a little work to get it to work with both forms.

The solution provided in #81 works.Thanks @benjifisher. I have addressed all the feedbacks on this.
Default state:-

On expanding the detail elements:-

The screenshots look good. It will be a couple of days before I have time to look at the latest code changes.

In the mean time, see if you can simplify the code. I have a feeling that we can get rid of some of the weights, and I think there is one weight of 99 that could probably be a 10. If the two form-alter functions are similar enough, maybe one of them can call the other instead of having them both call a helper function.

Can you update the issue summary with the latest screenshots?

I did some more testing. Remember, there are two forms here: one for menu_link_content and one for the Core form. Just to be sure, I added this line to the helper function for debugging:

  \Drupal::messenger()->addMessage($form['#form_id']);

After installing Drupal with the Standard profile, /admin/structure/menu/manage/main/add uses menu_link_content_menu_link_content_form and /admin/structure/menu/link/system.admin/edit uses menu_link_edit.

This is what the second form looks like with the current MR:

Unless I am forgetting something, we want to move the weight into "Display settings" for both forms. I already added some comments on the MR for how to do that.

Thanks for updating the screenshots in the issue summary. I am adding the "expanded" screenshot from Comment #82 as well. I think the "before" screenshots show the two different forms. We should do the same with the "after" screenshots. Currently, they both show menu_link_content_menu_link_content_form.

Thanks for the updates. I asked for a little mode code clean-up on the MR.

We still have to update the third "after" screenshot in the issue description.

I am sorry I did not check this before, but the doc block for claro_form_menu_link_edit_alter() is not accurate. The form ID (not base form ID) is defined in MenuLinkEditForm::getFormId().

The other doc block is accurate. It took me a while to figure it out: MenuLinkContentForm extends ContentEntityForm, which extends ContentForm, and that is where getBaseFormId() is defined.

How did you get the screenshot in #89, with the "Enabled" checkbox in between "Link" and "Menu link title"? That is not what we want, and it is not what I see when I test the MR.

Applied the suggestions in the MR. The changes suggested looked good.

I realized that we have been assuming the menu_link_edit form is using MenuLinkDefaultForm, but we cannot rely on that. I suggested a safeguard on the MR.

I am updating the issue description. I still do not know why some of the screenshots I am removing show the "Expanded" checkbox between "Link" and "Menu link title".

Thanks for the updates. I think this issue is ready for another look from a core committer.

xjm → credited AaronMcHale → .

xjm → credited anushrikumari → .

xjm → credited Emma Horrell → .

xjm → credited NikMis → .

xjm → credited simohell → .

xjm → credited worldlinemine → .

One small issue with the current version (might just have been a typo?). Other than that, I think this is much better so far. Thanks everyone!

Adding missing credits for the Usability Meeting participants.

lol wrong status because @lauriii is fast!!

Doing a self-RTBC here since the proposal was done by @xjm and I just clicked a button 😇

+1 for fixing the "Displays Settings" typo. The new layout is great!

Saving remaining credits.

Manually tested one last time myself to make sure everything's hunky-dory.

I should also note that @lauriii's reviews are a factor in my comfort committing this WRT the CSS for the new sidebar. ;)

I found a couple small grammar things in the code comments but they're trivial enough to fix at RTBC, so doing that now.

Committed to 11.x. Thanks! I did not backport it to 10.2.x since it is a UI change.

Tagging for the 10.3.0 release highlights.

Thanks everyone for all your work on this!

xjm → closed merge request !4796

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

Change tag from '10.3.0 highlights' to '10.3.0 release highlights'