Add MenuLinkInterface::getAdminLabel()

Are we sure we can still do that at this point in time without breaking any BC? An alternative would be to make somehow link objects aware of the fact that we render it on the admin URL at the moment.

IMHO though we already have the capability to do that, see the patch

Shouldn't we introduce a new "admin_description" available in the *.links.menu.yml files?
That behavior could be used by contrib/custom developpers and seems more testable to me.

Plus, to review the patch in #4 more precisely, I think it introduces a small but unwanted change: with the patch, the link wraps the menu item title and the help text instead of only wrapping the menu item title.

More reason to remove it.

@dawehner's patch in #4 no longer applied to 8.6.x. A reroll with no further modifications is attached. Issues raised in #7 still outstanding.

Interdiff is flaky due to new file. I'll try to re-upload one tomorrow.

Fixed the test and removed markup from the title because it gets encoded.

The patches don't seem to do what the issue summary (or the title!) says.

Attached patch against Drupal 10.1.x
Patch #15 is not applied for Drupal 10.1.x so Inter-diff file is not added.

Checking patch core/modules/menu_ui/src/MenuForm.php...
Hunk #1 succeeded at 234 (offset 14 lines).
error: while searching for:
      ]),
    ]);
    $links = $this->buildOverviewTreeForm($tree, $delta);
    foreach (Element::children($links) as $id) {
      if (isset($links[$id]['#item'])) {
        $element = $links[$id];

error: patch failed: core/modules/menu_ui/src/MenuForm.php:273
error: core/modules/menu_ui/src/MenuForm.php: patch does not apply
Checking patch core/modules/user/src/Plugin/Menu/LoginLogoutMenuLink.php...
Checking patch core/modules/user/src/Plugin/Menu/UserPageMenuLink.php...
Checking patch core/modules/user/tests/src/Functional/UserAccountLinksTest.php...
error: while searching for:
    // auto-increment is 1. Use XPath to obtain input element id and name using
    // the consistent label text.
    $this->drupalGet('admin/structure/menu/manage/account');
    $label = $this->xpath('//label[contains(.,:text)]/@for', [':text' => 'Enable My account menu link']);
    $this->assertFieldChecked($label[0]->getText(), "The 'My account' link is enabled by default.");

    // Disable the 'My account' link.

error: patch failed: core/modules/user/tests/src/Functional/UserAccountLinksTest.php:88
error: core/modules/user/tests/src/Functional/UserAccountLinksTest.php: patch does not apply

Created MR for the patch changes in #27 for the 11.x branch, please review it.

Thanks!

@mrinalini9 please don’t take patches and just turn to MRs without checking.

fix should use constructor promotion and type hints for and that’s just from 5 second glance.

Also please read the comments and tags. This was tagged for issue summary which still hasn’t happened

Thanks

We were working with colleagues on that issue and we would like to propose an alternative approach. The current patch is adding the description inside the link itself and we do not think it's the best approach.

What about if we use the metadata property already available in the MenuLinkFieldDefinitions.php ?
We could have user.link.menu.yml looking like:

user.page:
  title: 'My account'
  weight: -10
  route_name: user.page
  menu_name: account
  metadata:
    suffix: '(logged in users only)'

Then in the MenuForm.php, we could get the suffix property of metadata. That way, any contrib or custom module could do it the same way to display the message they want beside the menu link.

Something like this in MenuForm.php:

if ( isset($link->getMetaData()['suffix']) ) {
  $suffix = $link->getMetaData()['suffix'];
}

I'd be happy to provide with a patch if you think that's helping.

The idea in #34 seems interesting and the description could in fact be put under the "metadata" key. However, the "metadata" property doesn't seem to have a clear description or definition of purpose. It would be good to know what that property is expected to be used for before making a decision.

That said, if we look at the block module, the BlockContent entity has a property named "info" which is used for a similar purpose, i.e. to add administrative description. If we want to continue using that pattern, we could add a property named "info. IMHO, the name "info" seems very generic so I'd suggest using something like "admin_description" or even "context" (as we have in string translations).

Answers/opinions/suggestions are welcome (=

This update takes a shot at implementing the admin_description field for MenuLink entities to give better admin context.

What's been done:
- Added admin_description field to MenuLinkInterface and implementing classes
- Updated MenuTreeStorage schema and field definitions
- Updated User account menu so it uses the new setup (see screenshot attached)
- Fixed a couple performance tests that had hardcoded SQL queries

Current situation:
Got 3 tests failing that seem unrelated to the menu thing:
- 2 settings_tray tests with off-canvas issues
- 1 MySQL installer test about database isolation levels

Not really sure what's up with these - they don't seem connected to the admin_description changes. The menu functionality itself looks like it's working since the performance tests show the new field is getting included in queries properly.

Any thoughts on how to handle these test failures would be helpful!

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 necessarily 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.