noticed in #1945226: Add language selector on menus

Updated: Comment #0

Problem/Motivation

The custom menu add form had a element to show the machine name while the title is being typed, after save, that machine name is not editable, but is shown on the edit menu form.

During creation (add), if the menu title is "hello" it shows the machine name will be hello. That's not what happens though, the machine name is actually "menu-hello", and "menu-hello" is what is shown after saving on edit of the menu

Proposed resolution

make the preview of the machine name match the actual machine name.

Remaining tasks

discuss a way to fix this.

User interface changes

No.

API changes

None anticipated.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

tstoeckler’s picture

From the referenced issue, something like the following should do the trick:

// In MenuFormController::form() after the 'id' element.

// If a new menu is being added prefix the machine name with 'menu-'.
if ($menu->isNew()) {
  $form['id']['#field_prefix'] = 'menu-';
}

// After MenuFormController::actions() there should be a new function validate()

  /**
   * {@inheritdoc}
   */
  public function validate(array $form, array &$form_state) {
    // The machine name is validated automatically, we only need to add the
    // 'menu-' prefix here.
    $form_state['values']['id'] = 'menu-' . $form_state['values']['id'];
  }

I think that should do it?!

mandarmbhagwat78’s picture

// In MenuFormController::form() here is what I am proposing.

    $form['id'] = array(
      '#type' => 'machine_name',
      '#title' => t('Menu name'),
      '#default_value' => str_replace("menu-", "", $menu->id()),
      '#maxlength' => MENU_MAX_MENU_NAME_LENGTH_UI,
      '#description' => t('A unique name to construct the URL for the menu. It must only contain lowercase letters, numbers and hyphens.'),
//      '#field_prefix' => $menu->isNew() ? 'menu-' : '',
      '#machine_name' => array(
        'exists' => 'menu_edit_menu_name_exists',
        'source' => array('label'),
        'replace_pattern' => '[^a-z0-9-]+',
        'replace' => '-',
      ),
      // A menu's machine name cannot be changed.
      '#disabled' => !$menu->isNew() || isset($system_menus[$menu->id()]),
    );

mandarmbhagwat78’s picture

Here is the patch file attached

rszrama’s picture

This appears to work just fine on Drupal 8; moving back to Drupal 7 where I can confirm the issue.

@mandarmbhagwat78 - fwiw, you shouldn't have any patches that just include commented out lines of code. If lines of code are no longer necessary, they should just be removed. : )

tstoeckler’s picture

Yeah in D8 this got fixed in #1945226: Add language selector on menus. Sorry for not updating this issue.

mandarmbhagwat78’s picture

yep @rszrama will take care of that next time. :)

ethomas08’s picture

Issue summary: View changes
Status: Needs review » Reviewed & tested by the community

Tested on a fresh install of D7 -- patch worked perfectly.

poker10’s picture

Issue tags: -medium
FileSize
17.41 KB
17.97 KB

+1 to patch #4. I think this issue is important, as the information is missing for the user. If we add this prefix, it would work the same way as in the Field UI (content type - manage fields form) - this part also uses the prefix when editing the field name manually.

Added screenshots how it looks like before and after the patch.

mcdruid’s picture

FileSize
45.62 KB
45.15 KB

I'm not sure that this has really been "fixed" in D8/9?

Using the form at /admin/structure/menu/add looks like this:

the form at /admin/structure/menu/add

...and the menu- prefix is not displayed in the edit form either once the menu has been saved:

the form at admin/structure/menu/manage/MACHINE-NAME

What are we actually fixing here? Making the display of the machine name consistent with e.g. the D7 Fields UI as mentioned by @poker10?

Note that @YesCT also filed #2029197: remove enforced menu- prefix to custom menu machine names around the same time as this issue, but there's not been any activity on that.

poker10’s picture

The prefix was added in D8 in this issue: #1945226: Add language selector on menus and this commit: https://git.drupalcode.org/project/drupal/-/commit/0a0942a060ebe8413c69b4b6fff1723ddfa979bb, but was removed by this issue: #2084197: Uninstalling menu module removes all custom menus , where plugins have been unified (https://drupal.org/node/2086185).

In D9 this works OK, because if you create the menu (like you have on screenshots), the real menu name is as it is shown - still "foobar" (see also the URL after the menu is created "admin/structure/menu/manage/foobar").

But if you create a menu in D7, it shows the "foobar" when creating, but the real menu name after the menu is created is "menu-foobar" (see URL "admin/structure/menu/manage/menu-foobar"). This is the problem this issue is trying to fix. The menu name shown in the UI does not correspond with the real menu name created after saving the menu.

The issue #2029197: remove enforced menu- prefix to custom menu machine names seems no longer relevant for D9.

mcdruid’s picture

Issue tags: +RTBM

Great, thanks for the detailed explanation and for closing that issue, which does indeed now seem to be outdated.

  • poker10 committed 32020ec on 7.x
    Issue #2021571 by rszrama: Preview of menu machine name is inaccurate on...
poker10’s picture

Status: Reviewed & tested by the community » Fixed
Issue tags: -RTBM

Thanks everyone!

Status: Fixed » Closed (fixed)

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