UI: Rename 'menu items' to 'menu links' for more consistency

Yes, this is a sad and confusing legacy of D5 which was not fully cleaned up in D6. The UI cleanup might help clarify the DX.

While basically everything in the UI could be thus renamed, menu_get_item() is actually getting a router item.

fixed

Sorry, thanks for correcting my horrible mistake :P

is this fixed? - I think we addressed much of the use in the UI elsewhere, but possible should still address the API

As far as I know did we change it to menu links - but I only know the UI.

If this was changed in the code, I do not know, but I do know that this won't get into 7.x. Moving to 8.x. Close if the changes have been made in the code as well.

There are still a handful of references to 'menu items', in menu_ui help text and permission descriptions. Aside from that, the move to 'menu links' is almost complete.

This seems like a simple task, but let's constrain it to UI strings.

Drupalcon La Sprint - Working on this

Looks fixed - May have been done already elsewhere

Here's a list of files that expose 'menu items' in the UI:

• core/modules/views/src/Plugin/views/display/Page.php
• core/modules/menu_ui/menu_ui.module

New contributor here, so any feedback is appreciated... #DrupalConLA

Replaced menu items verbiage for menu links in the following files:

• modified: core/modules/menu_link_content/menu_link_content.routing.yml
• modified: core/modules/menu_link_content/src/Entity/MenuLinkContent.php
• modified: core/modules/menu_link_content/src/Tests/MenuLinkContentUITest.php
• modified: core/modules/menu_ui/menu_ui.admin.js
• modified: core/modules/menu_ui/menu_ui.module
• modified: core/modules/menu_ui/menu_ui.permissions.yml
• modified: core/modules/menu_ui/src/Controller/MenuController.php
• modified: core/modules/menu_ui/src/Form/MenuDeleteForm.php
• modified: core/modules/menu_ui/src/Tests/MenuLanguageTest.php
• modified: core/modules/menu_ui/src/Tests/MenuNodeTest.php
• modified: core/modules/menu_ui/src/Tests/MenuTest.php
• modified: core/modules/menu_ui/src/Tests/MenuWebTestBase.php
• modified: core/modules/views/src/Plugin/views/display/Page.php

I took a swing at this. The problem with #17 seems to be that changes were made to routes and functions instead of just to the UI. This patch only changes the ui strings in the files mentioned in #16 along with a few tests that asserted the language that had been changed.

This is my first contributed patch, so I apologize in advance if I stepped on any toes or messed anything else up here.

@epophoto Tested via SimplyTest.me... This looks good to me. Thanks!

I am removing the Novice tag from this issue because it looks the task has been completed.

I’m using this documentation as a source: https://www.drupal.org/core-mentoring/novice-tasks#avoid

Also gave this a Beta Evaluation so we can get this committed.

This looks like the test bot failed to start the test. This is my first patch so I am not sure what to do here, do I re mark it for review or is this automated failure part of the Beta Evaluation process....

The tests had all passed so I am not sure how to proceed...

Patch no longer applies.

Re-rolling patch.

This looks great, thanks!

Thanks everyone for your work here! I've found a couple references to "menu item(s)" in user-facing strings still:

core/modules/forum/forum.module:      $output .= '<dd>' . t('Enabling the Forum module provides a default <em>Forums</em> menu item in the Tools menu that links to the <a href="!forums">Forums page</a>.', array('!forums' => \Drupal::url('forum.index'))) . '</dd>';
core/modules/toolbar/toolbar.info.yml:description: 'Provides a toolbar that shows the top-level administration menu items and links from other modules.'
core/modules/system/src/Plugin/Block/SystemMenuBlock.php:      '#description' => $this->t('The menu will only be visible if the menu item for the current page is at or below the selected starting level. Select level 1 to always keep this menu visible.'),
core/modules/shortcut/shortcut.module:      $output .= '<dd>' . t('You can display your shortcuts by enabling the <em>Shortcuts</em> block on the <a href="!blocks">Blocks administration page</a>. Certain administrative modules also display your shortcuts; for example, the core <a href="!toolbar-help">Toolbar module</a> provides a corresponding menu item.', array('!blocks' => (\Drupal::moduleHandler()->moduleExists('block')) ? \Drupal::url('block.admin_display') : '#', '!toolbar-help' => (\Drupal::moduleHandler()->moduleExists('toolbar')) ? \Drupal::url('help.page', array('name' => 'toolbar')) : '#')) . '</dd>';

Also, there are still a ton of references to "menu items" in code comments and occasionally test assertion messages. Wouldn't it make sense to add those changes to this patch as well so that the code documentation reflects what the user sees? The table descriptions in MenuTreeStorage can also be considered documentation.

Then there is the Menu UI permission: core/modules/menu_ui/menu_ui.permissions.yml: title: 'Administer menus and menu items'

Renaming this permission would require an upgrade path, so I agree that should be skipped in this issue (and might not be acceptable during the beta). However, let's create a followup issue for it. Edit: what I said before is wrong I think; the permission name is 'administer menu'. So that could probably be changed too.

I'd also skip over anything to do with Migrate as that seems to have to do with the D6 source code.

Oh, just a general note, please provide interdiffs when updating existing patches. Thanks!

Updated many references to 'menu item(s)' through source. (Ignored core/modules/migrate_drupal references.)

Also, tagged as 'JavaScript' per recent suggestion from @nod_ in a similar thread (to track any changes to js files).

To do:
* Address 'administer menu' change suggested by xjm

This is awesome mikeburrelljr, thanks!

One small nit:

+++ b/core/modules/field_ui/src/Plugin/Derivative/FieldUiLocalTask.php
@@ -108,10 +108,10 @@ public function getDerivativeDefinitions($base_plugin_definition) {
+        // modes available for customisation. So we define menu link for all

This should be 'menu links' rather than just 'menu link'.

@xjm - UI should be done before string freeze, so let's not try to make this perfect in terms of code comments. I'd rather see UI changes happen in this issue, and address stray code stings in other (novice) issues.

Here's a re-roll to get the last patch to apply.

Following the directions here https://www.drupal.org/patch/reroll

I performed a git apply --check on the patch and received no messages which should mean the patch is fine.

Not able to apply patch #36.

This is a 8.1 or 9.0.x issue now due to string changes.

Reroll for patch #36

Needs reroll against 8.1.x

Here's the reroll, I've seen several uses of "menu item" on the code, but I've been focus on the reroll.

There are just some unoptimal changes but in general I like the idea of using menu link instead of menu item.

1. diff --git a/core/CHANGELOG.txt b/core/CHANGELOG.txt
   index 761ed1d..7f8ada2 100644
   
   index 761ed1d..7f8ada2 100644
   --- a/core/CHANGELOG.txt
   
   --- a/core/CHANGELOG.txt
   +++ b/core/CHANGELOG.txt
   
   +++ b/core/CHANGELOG.txt
   @@ -79,7 +79,7 @@ Drupal 8.0, xxxx-xx-xx (development version)
   
   @@ -79,7 +79,7 @@ Drupal 8.0, xxxx-xx-xx (development version)
            environments.
        * Improved content language support:
          * Made it possible to assign language to taxonomy terms, vocabularies,
   -        menu items, and files.
   +        menu links, and files.
          * Added a field translation based content translation module that applies
            to all content entities.
          * Removed the old node-copy based content translation module.
   @@ -395,7 +395,7 @@ Drupal 6.0, 2008-02-13
   
   @@ -395,7 +395,7 @@ Drupal 6.0, 2008-02-13
          database rights.
        * Dynamically check password strength and confirmation.
        * Refactored poll administration.
   -    * Implemented drag-and-drop positioning for blocks, menu items, taxonomy
   +    * Implemented drag-and-drop positioning for blocks, menu links, taxonomy
          vocabularies and terms, forums, profile fields, and input format filters.
    - Theme system:
        * Added .info files to themes and made it easier to specify regions and
   @@ -496,7 +496,7 @@ Drupal 5.0, 2007-01-15
   
   @@ -496,7 +496,7 @@ Drupal 5.0, 2007-01-15
    ----------------------
    - Completely retooled the administration page
        * /Admin now contains an administration page which may be themed
   -    * Reorganised administration menu items by task and by module
   +    * Reorganised administration menu links by task and by module
        * Added a status report page with detailed PHP/MySQL/Drupal information
    - Added web-based installer which can:
        * Check installation and run-time requirements
   @@ -524,7 +524,7 @@ Drupal 5.0, 2007-01-15
   
   @@ -524,7 +524,7 @@ Drupal 5.0, 2007-01-15
        * Made it easy to investigate popular search terms.
        * Added a 'select all' checkbox and a range select feature to administration tables.
        * Simplified the 'break' tag to split teasers from body.
   -    * Use proper capitalization for titles, menu items and operations.
   +    * Use proper capitalization for titles, menu links and operations.
    - Integrated urlfilter.module into filter.module
    - Block system:
        * Extended the block visibility settings with a role specific setting.
   @@ -818,7 +818,7 @@ Drupal 4.5.1, 2004-12-01
   
   @@ -818,7 +818,7 @@ Drupal 4.5.1, 2004-12-01
    Drupal 4.5.0, 2004-10-18
    ------------------------
    - Navigation:
   -    * Made it possible to add, delete, rename and move menu items.
   +    * Made it possible to add, delete, rename and move menu links.
        * Introduced tabs and subtabs for local tasks.
        * Reorganized the navigation menus.
   

   Let's not change CHANGELOG.txt

2. +++ b/core/LICENSE.txt
   @@ -320,7 +320,7 @@ when it starts in an interactive mode:
    The hypothetical commands `show w' and `show c' should show the appropriate
    parts of the General Public License.  Of course, the commands you use may
    be called something other than `show w' and `show c'; they could even be
   -mouse-clicks or menu items--whatever suits your program.
   +mouse-clicks or menu links--whatever suits your program.
    
   

   We should certainly not change the license.

3. +++ b/core/modules/field_ui/field_ui.module
   @@ -127,7 +127,16 @@ function field_ui_entity_type_build(array &$entity_types) {
      // When a new bundle is created, the menu needs to be rebuilt to add our
   -  // menu item tabs.
   +  // menu link tabs.
   +  \Drupal::service('router.builder')->setRebuildNeeded();
   +}
   +
   +/**
   + * Implements hook_entity_bundle_rename().
   + */
   +function field_ui_entity_bundle_rename($entity_type, $bundle_old, $bundle_new) {
   +  // When a bundle is renamed, the menu needs to be rebuilt to add our
   +  // menu link tabs.
      \Drupal::service('router.builder')->setRebuildNeeded();
    }
    
   

   While this change is nice, it feels a bit out of scope ... ?

I am working on it.

Re-rolled the patch.

The menu.html.twig file is updated in this ticket, so i think we need to update md5 value of file .
Fixing failed test case

I am working on it

Fixing test case failure

sorry @naveen433 i think i was uploading patch when you commented. I didnt see your message

my patch is below please look and review

When you tag an issue for usability review, please make it easy for the usability team to review the issue. Update the issue summary:

• The "Proposed resolution" section should describe all the changes made in the issue.
• The "User interface changes" should show the existing UI and the proposed UI.

Most of the time, I prefer to have plain text in the "Proposed resolution" section and screenshots in the "User interface changes" section.

You can also attend the weekly usability meeting to present an issue.

I am adding the tag for an issue summary update and setting the status to NW.

Menu items is more correct than menu links IMHO, you don't even have to add links to menus, you can add a bunch of buttons that don't link anywhere, or explicitly make a menu item a non link.

The issue summary is for fixing 'menu items' in the UI. I searched today and found no instances of 'menu items' in the UI.

(11.x)$ git grep -i 'menu item' | grep -v LICENSE.txt | grep -v migrate | grep -v ':\s*\*' | grep -v ':\s*\/\/' | grep -v ':\s*\/\*'
core/modules/menu_ui/tests/src/Functional/MenuUiTest.php:      $this->assertSession()->pageTextContains('Edit menu item');
core/modules/system/tests/modules/menu_test/menu_test.links.menu.yml:  title: 'Menu item title'
core/modules/system/tests/modules/menu_test/menu_test.links.menu.yml:  description: 'Menu item description parent'
core/modules/system/tests/modules/menu_test/menu_test.links.menu.yml:  title: 'Menu item with a regular description'
core/modules/system/tests/modules/menu_test/menu_test.links.menu.yml:  description: 'Menu item description text'
core/modules/system/tests/modules/menu_test/menu_test.links.menu.yml:  description: 'Menu item description parent'
core/modules/system/tests/modules/menu_test/menu_test.links.menu.yml:  description: 'Menu item description child2'
core/modules/system/tests/modules/menu_test/menu_test.links.menu.yml:  description: 'Menu item description child3'
core/modules/system/tests/modules/menu_test/menu_test.links.menu.yml:  description: 'Menu item description child4'
core/modules/system/tests/modules/menu_test/menu_test.links.menu.yml:  description: 'Menu item description child4 overview'
core/modules/system/tests/modules/menu_test/menu_test.links.menu.yml:  description: 'Menu item description grand child2'
core/modules/system/tests/modules/menu_test/menu_test.links.menu.yml:  description: 'Menu item description grand child3'
core/modules/system/tests/modules/menu_test/menu_test.links.menu.yml:  description: 'Menu item description grand child4'
core/modules/system/tests/modules/menu_test/menu_test.links.menu.yml:  description: 'Menu item description parent'
core/modules/system/tests/modules/menu_test/menu_test.links.menu.yml:  description: 'Menu item description child'
core/modules/system/tests/modules/menu_test/menu_test.links.menu.yml:  description: 'Menu item description parent'
core/modules/system/tests/modules/menu_test/menu_test.links.menu.yml:  description: 'Menu item description child'
core/modules/system/tests/modules/menu_test/menu_test.links.menu.yml:  description: 'Menu item description parent'
core/modules/system/tests/modules/menu_test/menu_test.links.menu.yml:  description: 'Menu item description child'
core/modules/system/tests/modules/menu_test/menu_test.routing.yml:    _title: 'Menu item with a regular description'
core/modules/system/tests/src/Functional/Menu/MenuRouterTest.php:    $this->assertSession()->pageTextContains('Menu item description text');
core/themes/claro/css/components/tabledrag.css:      using indentation (e.g. re-ordering menu items. */
core/themes/claro/css/components/tabledrag.pcss.css:      using indentation (e.g. re-ordering menu items. */

There are two instances in tests.

1. core/modules/menu_ui/tests/src/Functional/MenuUiTest.php: $this->assertSession()->pageTextContains('Edit menu item');
2. core/modules/system/tests/src/Functional/Menu/MenuRouterTest.php: $this->assertSession()->pageTextContains('Menu item description text');

The first one is in an if block in a test that is never reached. That method was committed years ago and may need to examined in another issue. The second is is testing strings from a test module, but the method, doTestDescriptionMenuItems, the assertion is in is never called.

I think this should be closed and another opened to look at the tests.

Opened #3500775: Replace use of 'menu items' in tests

Moving this to RTBC because I'm wonder if it should be marked fixed for all the work that happened 10 years ago

If we're going to do that let's also credit the reviewers. :) Saving credit for historical reviewers.