If you go to admin/structure/menu/manage/management, for example, and click the save button, you get no feedback that anything happened. A drupal_set_message() should display something generic like "Your configuration has been saved." Check what other places in core are doing for a hint.

Files: 
CommentFileSizeAuthor
#16 609108_menu_admin_page_16.patch1.09 KBSivaji
PASSED: [[SimpleTest]]: [MySQL] 20,141 pass(es). View
#13 609108_menu_admin_page_feedback_2.patch4.86 KBSivaji
PASSED: [[SimpleTest]]: [MySQL] 17,375 pass(es). View
#8 609108_menu_admin_page_feedback_1.patch4.06 KBSivaji
Passed on all environments. View
#5 609108_menu_admin_page_feedback.patch2.51 KBSivaji
Failed on MySQL 5.0 ISAM, with: 15,327 pass(es), 5 fail(s), and 0 exception(es). View
#2 609108_menu_admin_page_feedback.patch1.68 KBSivaji
Passed on all environments. View

Comments

lisarex’s picture

Sivaji’s picture

Status: Active » Needs review
FileSize
1.68 KB
Passed on all environments. View

Attached patch provides appropriate status message when you add or edit a menu or menu item.

Dries’s picture

Can we simplify the message and make it consistent with #620592: Taxonomy admin page ... need feedback when saving?

Sivaji’s picture

Assigned: Unassigned » Sivaji
Status: Needs review » Needs work

That sounds good. I had the same thought when i am rolling this patch, but other status messages in the menu module is using something similar to "The new menu %name has been created". Anyways i will roll it again.

Sivaji’s picture

FileSize
2.51 KB
Failed on MySQL 5.0 ISAM, with: 15,327 pass(es), 5 fail(s), and 0 exception(es). View

Attached patch simplifies the status message and makes it consistent with #620592: Taxonomy admin page ... need feedback when saving.

Sivaji’s picture

Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch failed testing.

Sivaji’s picture

Status: Needs work » Needs review
FileSize
4.06 KB
Passed on all environments. View

Attached patch should work

Status: Needs review » Needs work
Issue tags: -Novice

The last submitted patch failed testing.

Status: Needs work » Needs review
Issue tags: +Novice

sivaji requested that failed test be re-tested.

baronmunchowsen’s picture

I applied the patch in #8 and created a link from admin/structure/menu/manage/management and got a feedback message 'Created new menu link Test Link.', so it seems to work . I have not reviewed the PHP code. I also noticed that in the menu.test file that some of the UI text strings were inconsistent - some with periods ('.'), some without. Not sure if this is a concern or not, or deserving of its own issue.

flickerfly’s picture

I applied the patch in #8 and it was grumpy.

flickerfly$ patch -p0 < 609108_menu_admin_page_feedback_1_0.patch 
patching file modules/menu/menu.admin.inc
Hunk #1 succeeded at 161 (offset -11 lines).
Hunk #2 FAILED at 381.
Hunk #3 FAILED at 534.
Hunk #4 FAILED at 582.
Hunk #5 succeeded at 552 (offset -51 lines).
Hunk #6 FAILED at 585.
4 out of 6 hunks FAILED -- saving rejects to file modules/menu/menu.admin.inc.rej
can't find file to patch at input line 74
Perhaps you used the wrong -p or --strip option?
The text leading up to this was:
--------------------------
|Index: modules/menu/menu.test
|===================================================================
|RCS file: /cvs/drupal/drupal/modules/menu/menu.test,v
|retrieving revision 1.27
|diff -u -p -r1.27 menu.test
|--- modules/menu/menu.test	2 Dec 2009 19:26:22 -0000	1.27
|+++ modules/menu/menu.test	4 Dec 2009 09:43:12 -0000
--------------------------
File to patch: q  	
q: No such file or directory
Skip this patch? [y] y
Skipping patch.
2 out of 2 hunks ignored

flickerfly$ cat modules/menu/menu.admin.inc.rej 
*************** function menu_edit_item_submit($form, &$
*** 380,385 ****
    if (!menu_link_save($item)) {
      drupal_set_message(t('There was an error saving the menu link.'), 'error');
    }
    $form_state['redirect'] = 'admin/structure/menu/manage/' . $item['menu_name'];
  }
  
--- 381,396 ----
    if (!menu_link_save($item)) {
      drupal_set_message(t('There was an error saving the menu link.'), 'error');
    }
+ 
+   // display status message
+   $op = $form_state['build_info']['args'][0];
+   if ($op === 'add') {
+     drupal_set_message(t('Created new menu link %name.', array('%name' => $item['link_title'])));
+   }
+   elseif ($op === 'edit') {
+     drupal_set_message(t('Updated menu link %name.', array('%name' => $item['link_title'])));
+   }
+ 
    $form_state['redirect'] = 'admin/structure/menu/manage/' . $item['menu_name'];
  }
  
*************** function menu_delete_menu_confirm_submit
*** 523,529 ****
    menu_delete($menu);
  
    $t_args = array('%title' => $menu['title']);
-   drupal_set_message(t('The custom menu %title has been deleted.', $t_args));
    watchdog('menu', 'Deleted custom menu %title and all its menu links.', $t_args, WATCHDOG_NOTICE);
  }
  
--- 534,540 ----
    menu_delete($menu);
  
    $t_args = array('%title' => $menu['title']);
+   drupal_set_message(t('Deleted custom menu %title and all its menu links.', $t_args));
    watchdog('menu', 'Deleted custom menu %title and all its menu links.', $t_args, WATCHDOG_NOTICE);
  }
  
*************** function menu_edit_menu_submit($form, &$
*** 571,576 ****
  
      menu_link_save($link);
      menu_save($menu);
    }
    else {
      menu_save($menu);
--- 582,588 ----
  
      menu_link_save($link);
      menu_save($menu);
+     drupal_set_message(t('Created new menu %name.', array('%name' => $menu['title'])));
    }
    else {
      menu_save($menu);
*************** function menu_item_delete_form_submit($f
*** 572,578 ****
    $item = $form['#item'];
    menu_link_delete($item['mlid']);
    $t_args = array('%title' => $item['link_title']);
-   drupal_set_message(t('The menu link %title has been deleted.', $t_args));
    watchdog('menu', 'Deleted menu link %title.', $t_args, WATCHDOG_NOTICE);
    $form_state['redirect'] = 'admin/structure/menu/manage/' . $item['menu_name'];
  }
--- 585,591 ----
    $item = $form['#item'];
    menu_link_delete($item['mlid']);
    $t_args = array('%title' => $item['link_title']);
+   drupal_set_message(t('Deleted menu link %title.', $t_args));
    watchdog('menu', 'Deleted menu link %title.', $t_args, WATCHDOG_NOTICE);
    $form_state['redirect'] = 'admin/structure/menu/manage/' . $item['menu_name'];
  }

Sivaji’s picture

FileSize
4.86 KB
PASSED: [[SimpleTest]]: [MySQL] 17,375 pass(es). View

Attached patch should work.

flickerfly’s picture

Status: Needs review » Reviewed & tested by the community

#13 applied properly and provided a message saying that menu items had changed when saved. It checks out nicely for me.

webchick’s picture

Version: 7.x-dev » 8.x-dev
Component: menu system » user interface text

Hey, thanks for this. Unfortunately, we're not making any more non-critical changes to the user interface text this release. :( Bumping to Drupal 8.

Sivaji’s picture

Version: 8.x-dev » 7.x-dev
Component: user interface text » menu system
Status: Reviewed & tested by the community » Needs review
FileSize
1.09 KB
PASSED: [[SimpleTest]]: [MySQL] 20,141 pass(es). View

I don't think it is good to leave this bug open for UI text freeze. I agree that #13 makes unwanted checks and UI text changes. Attached patch will fix this bug just by adding three drupal_set_message(t('Your configuration has been saved.')); in menu_foo_bar_submit(). I am sure its worth adding this now.

For Testing
1. Go to List menu links page eg. admin/structure/menu/manage/user-menu
2. Go to menu item edit page eg. admin/structure/menu/item/28/edit
3. Go to menu description edit page eg. admin/structure/menu/manage/user-menu/edit

Submitting menu form in any of the above pages should display drupal_set_message(t('Your configuration has been saved.')); message.

ghills88’s picture

Patch file from #16 applies cleanly to CVS HEAD for me and has the desired effect. Those different edit pages all show a message when the configuration changes are saved where they did not before.

joachim’s picture

Status: Needs review » Reviewed & tested by the community

Same here.

RTBC :)

yoroy’s picture

I did notice this lack of feedback on certain admin pages, too. Definitely worth fixing for D7.

Sivaji’s picture

@yoroy That's true, One of the admin pages i recently noticed is reported here #787684: Role admin page should say something when you save hope someone here will have time to put some effort.

Dries’s picture

Status: Reviewed & tested by the community » Fixed

Committed to CVS HEAD. Thanks.

Status: Fixed » Closed (fixed)
Issue tags: -Novice

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