When the Menu module was renamed "Menu Ui" ... This was bad:

a) The hook_help() wasn't updated. There are probably other places in the code/docs where it is still called "the Menu module" instead of its new name, such as in other hook_help text output in other modules that refer to the menu module.

b) "Ui" is not a good name. It should be "UI" in the info.yml file. UI is an acronym for User Interface, not a word that should have only the first word capitalized.

Fixing this is probably a good Novice task? The task is: Find all the places in the code where it refers to "the Menu module" and change them to "the Menu UI module". And also fix the menu_ui.info.yml file to use the correct capitalization.

This should be done ASAP since it is a UI text issue and affects translation.

CommentFileSizeAuthor
#12 interdiff-10-12.txt5.04 KBcs_shadow
#12 drupal-menu_to_menu_ui-2243549-12.patch17.66 KBcs_shadow
#10 menu_to_menuui_text_changes_3_2243549.patch14.4 KBagerson
#7 menu_to_menuui_text_changes_2_2243549.patch8.24 KBagerson
#5 menu_to_menuui_text_changes_2243549.patch7.89 KBagerson

Comments

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented
Title: Menu UI module rename no good. :( » Text changes needed as follow-up to Menu UI module rename

"no good :("?

agerson’s picture

agerson commented

We do run into some awkward comments like changeing "Verifies a menu link using the menu module UI." to "Verifies a menu link using the menu UI module UI." Should it be "Verifies a menu link using the UI" or "Verifies a menu link using the menu UI module"?

dawehner’s picture

dawehner
Primary language German
commented

:( Well, actually the help text should not be required to change as the human readable name of the module did not changed. Maybe this is an oversimplification.

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented

#3 is incorrect. The human-readable name of the module changed from Menu to Menu UI in the yml file.

#2 - if this is in a code doc comment, then you can do something like "Verifies a menu link using the UI of menu_ui.module". Or this would also be a better wording: "Verifies a menu link using the UI of the Menu UI module.".

Note: When referring to the human-readable module, it should always be:

"the Menu UI module"

with that exact capitalization.

agerson’s picture

agerson commented
StatusFileSize
new menu_to_menuui_text_changes_2243549.patch7.89 KB

Ok. How does this look:

agerson’s picture

agerson commented
Status: Active » Needs review
agerson’s picture

agerson commented
StatusFileSize
new menu_to_menuui_text_changes_2_2243549.patch8.24 KB

Missed one important one!

agerson’s picture

agerson commented
dcrocks’s picture

dcrocks commented

I count 28 occurrences of the string 'menu module' in a current D8 install.

agerson’s picture

agerson commented
StatusFileSize
new menu_to_menuui_text_changes_3_2243549.patch14.4 KB

Updated the patch to include files outside of the Menu UI module folder

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented
Status: Needs review » Needs work

Thanks -- this is looking pretty good!

A few suggestions:

a) MenuTest.php:

+      'description' => 'Add a custom menu, add menu links to the custom menu and Tools menu, check their data, and delete them using the UI of the Menu UI module.',

How about just "... and delete them using the UI"? (My logic: this is a test in the Menu UI module, so I don't think we necessarily need to explicitly say we are testing the UI of the Menu UI module, as that would be assumed?) (and this could be similar for the other methods in this file)

b) I am certain that this does not cover all of the spots in other modules that refer to the old name of the module. For instance, the function menu_link_help() mentions the Menu module (should be Menu UI now). contextual.module help also has a reference. There may be others. Try something like this:

grep -R Menu * | grep -i module | more
# and also ...
grep -R "menu module" * | more
cs_shadow’s picture

cs_shadow commented
Status: Needs work » Needs review
StatusFileSize
new drupal-menu_to_menu_ui-2243549-12.patch17.66 KB
new interdiff-10-12.txt5.04 KB

Attached is a patch that adds the suggestions in #11 to the patch in #10. Most of the tests are now renamed to something like '...using the UI' instead of '...using the UI of the Menu UI module'.

Also, I've tried to find and replace all occurrences of 'menu module' with 'menu ui module'. Let me know if I missed out something.

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented
Status: Needs review » Reviewed & tested by the community

Thanks! I think the patch includes everything it should now, and it all looks good.

By the way, we're in a "criticals and majors only" pre-alpha period, so this cannot be committed for a few days.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Fixed

Committed 3937bda and pushed to 8.x. Thanks!

  • Commit 3937bda on 8.x by alexpott: 
    Issue #2243549 by agerson, cs_shadow | jhodgdon: Text changes needed as...

Status: Fixed » Closed (fixed)

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