Use of translation functions in menu logic is... odd.
See this screenshot of the CiviCRM nav menu with the site locale set to Spanish:
The bulk of the labels are translated, except for the middle "Hide menu". My first thought was that this was just down to a lack of translation, but it turns out things are not that simple.
The home menu items themselves are defined as hardcoded strings in app/public/wp-content/plugins/civicrm/civicrm/CRM/Core/BAO/Navigation.php
(CRM_Core_BAO_Navigation::buildHomeMenu
):
$item['child'] = [
[
'attributes' => [
'label' => 'CiviCRM Home',
'name' => 'CiviCRM Home',
'url' => 'civicrm/dashboard?reset=1',
'weight' => 1,
],
],
[
'attributes' => [
'label' => 'Hide Menu',
'name' => 'Hide Menu',
'url' => '#hidemenu',
'weight' => 2,
],
],
[
'attributes' => [
'label' => 'Log out',
'name' => 'Log out',
'url' => 'civicrm/logout?reset=1',
'weight' => 3,
],
],
];
Later in CRM_Admin_Page_AJAX::formatMenuItems
the labels are passed through the ts()
function:
if (!empty($props['label'])) {
$item['label'] = ts($props['label'], ['context' => 'menu']);
}
Some of the strings are used elsewhere (e.g. 'CiviCRM Home') and so are caught by the translation string extraction, and therefore get translated when passed into ts()
as a variable $props['label']
. This isn't the case for the string 'Hide Menu' however.
I suggest that CRM_Core_BAO_Navigation::buildHomeMenu
is updated so that each label is passed through ts()
. I'm not sure if this should include ['context' => 'menu']
to match CRM_Admin_Page_AJAX::formatMenuItems
.
Best practice says that variables should not be passed as the first argument to ts()
. However, CRM_Admin_Page_AJAX::formatMenuItems
already works in this way, so it could be seen as a break to backwards compatiability to change this. If we don't change this, then there is the edge-case where a string would be double-translated, which could lead to unexpected results when combined with word replacement. So I'm not sure if this method should be updated or not.