Text in forms are not translatable
BenKewell - October 22, 2009 - 09:43
| Project: | Notify |
| Version: | 6.x-1.x-dev |
| Component: | Code |
| Category: | bug report |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | needs review |
Description
Some text in below forms are not translatable as t() is not called on them:
notify_admin_settings()
notify_user_settings_form()
also text in form _notify_user_reg_fields() are not translatable, because of improper use of t() :
// Add the checkbox to the fieldset
$fields['notify_agree']['notify_decision'] = array(
'#type' => 'checkbox',
'#title' => t('Receive email notifications of news posted to this site. Notifications are sent every ' . format_interval($period).'.'),
'#return_value' => 1,
'#default_value' => 1,
'#description' => t('By accepting to receive <em>daily email news notifications</em>, you will be kept informed of news associated with ' . variable_get('site_name', 'Drupal') . '.')
);should be:
// Add the checkbox to the fieldset
$fields['notify_agree']['notify_decision'] = array(
'#type' => 'checkbox',
'#title' => t('Receive email notifications of news posted to this site. Notifications are sent every !interval.', array('!interval' => format_interval($period))),
'#return_value' => 1,
'#default_value' => 1,
'#description' => t('By accepting to receive <em>daily email news notifications</em>, you will be kept informed of news associated with !sitename.', array('!sitename' => variable_get('site_name', 'Drupal'))),
);the original way of calling t() by directly inserting variable into string is a bad practice of using t().
it also blocks the text to be recognized for translation.
#1
attached with the patch to fix the items i described above and a few more, which includes:
1.) added t() to several text in notify_admin_settings() and notify_user_settings_form() to make them translatable
2.) fixed 2 calls to t() in _notify_user_reg_fields() to use placeholders,
in order to comply with Drupal coding regulations (which is the correct way of using t() with variables)
3.) fixed 1 call to format_plural() in _notify_send() to use placeholders
4.) in notify_user_settings_form(), changed fieldset title 'Master switch' to 'Status',
as this would be much more clear in meaning to non-technical users.
#2
Patch looks good. Only #4 seems to warrant further discussion.
I think the 'Master switch' label has exists since before I became maintainer; that or it was changed when 'Notify by Views' became available and added even more choice to this page. IMO, 'Status' is vague. The intent is to clarify for for the user, "No matter what you do with all the choice below, this turns everything on or off."
Also, the selector actual selector is already labeled 'Notify Status', so renaming the grouping is redundant and actually reduces the amount of description that might help the end user understand the purpose of the switch.
Is there another term that would be clearer in identifying this as the master override?
Or else, would you like to whip up some jquery to hide all the other fields when this is set 'disabled' ? That would make it pretty clear.