Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Translate strings in "Report" UI, et al #12009

Merged
merged 4 commits into from
Apr 24, 2018

Conversation

yashodha
Copy link
Contributor

@yashodha yashodha commented Apr 20, 2018

Perform translation (ts()) on an assortment of strings. Most of the strings are the CiviReport forms (although there are a few others).

);
if (!empty($graphRows)) {
foreach ($graphRows[$this->_interval] as $key => $val) {
$graph[$val] = $graphRows['value'][$key];
}
$chartInfo['values'] = $graph;
$chartInfo['tip'] = 'Participants : #val#';
$chartInfo['tip'] = ts('Participants : %1', array(1 => '#val#'));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think civilint is highlighting a double-space in this line.

@totten totten changed the title Translate strings Translate strings in "Report" UI, et al Apr 20, 2018
@totten
Copy link
Member

totten commented Apr 20, 2018

(CiviCRM Review Template WORD-1.1)

  • (r-explain) Pass: There isn't really a whole lot to say. I tweaked description to indicate where the changes are.
  • (r-test) Needs work: Currently blocked on a small error in code-style check
  • (r-code) Pass
  • (r-doc) Pass
  • (r-maint) Pass
  • (r-run) Not assessed:
  • (r-user) Pass:
  • (r-tech) Soft pass: There have been regressions in the past when someone translated a string (e.g. if an option-value is translated and piece of logic incidentally depends on the old label). However, most of these changes are in the CiviReport UI, and I don't think of that as a programmatic interface. (By contrast, if the changes were mostly in BAOs, it might merit higher scrutiny.) If someone else gives it a skim and agrees it's safe, that'd be more reassuring.

@@ -470,7 +470,7 @@ public function alterDisplay(&$rows) {
// If the email address has been deleted
if (array_key_exists('civicrm_email_email', $row)) {
if (empty($rows[$rowNum]['civicrm_email_email'])) {
$rows[$rowNum]['civicrm_email_email'] = '<del>Email address deleted</del>';
$rows[$rowNum]['civicrm_email_email'] = ts('<del>Email address deleted</del>');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@yashodha i think for the two mailing ones here i think it might better if it was like this '<del>' . ts('Email address deleted') . '</del>' what do you think @mlutfy

@yashodha
Copy link
Contributor Author

@totten @seamuslee001
made all the changes as per the comments.

@mlutfy mlutfy merged commit b1df120 into civicrm:master Apr 24, 2018
@mlutfy
Copy link
Member

mlutfy commented Apr 24, 2018

I merged the PR based on the reviews by Tim and Seamus. I also reviewed the code of the latest changes by Yashodha.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants