Phabricator project icon names are should also be translated.
Needs RevisionPublic
Actions

Authored by huangsj on Aug 22 2017, 8:52 AM.

Details

Reviewers

chad

Group Reviewers

Blessed Reviewers

Summary

I want to localize Phabricator in Chinese. PhutilCNChineseLocale.php and PhabricatorCNChineseTranslation.php are created and translations are provided. Project Icon Names such as Tag, Group are not translated even if corresponding translations are provided in PhabricatorCNChineseTranslation.php. This is an unexpected behavior. Phabricator uses pht function to translate its UI and Icon Names should be translated.

I have to pass non-constant php variable to pht function in this diff.

Test Plan

Add Chinese translation of Tag, Group, Project, Organization to PhabricatorCNChineseTranslation.php, such as 'Project' => '项目'.

Change Translation Settings from English to Chinese.
Navigate to https://secure.phabricator.com/project/query/all/.
All text 'Project' after project icons are translated to '项目'.

Change preference back to English again.
All text 'Project' after project icons are translated to 'Project'.

Diff Detail

Repository

rP Phabricator

Lint

Lint Skipped

Unit

Tests Skipped

Event Timeline

huangsj created this revision.Aug 22 2017, 8:52 AM

Herald added a reviewer: Blessed Reviewers. · View Herald TranscriptAug 22 2017, 8:52 AM

Herald added a subscriber: epriestley. · View Herald Transcript

I tested this patch on my local installation and it worked.

Awesome! I'll take a look at this today.

So, I kind of followed this from a ticket and noticed that there were a few mistakes in the code.

The larges one is line 135, where you are missing a $ for spec['name'].

Also, I think the foreach is redundant when you could pass the original name through pht without the loop (eg: $icon['name'] = ( $value !== null ) ? pht($icon['name']) : null;)

Also, if you follow the coding spec, I believe they want you to use === or !== when checking for null.

Just my 2c

This doesn't patch cleanly, doesn't look generated by arcanist. I'll poke at a local diff myself.

This revision now requires changes to proceed.Aug 24 2017, 4:00 PM

I can't figure out what this diff is supposed to do. It looks... generated in the wrong direction? And like @DanSheps mentions, is missing code that will allow it to be ran.

Revision Contents
Changeset List

Path

Size

src/

applications/

project/

icon/

PhabricatorProjectIconSet.php

22 lines

Diff 44332

View Options

src/applications/project/icon/PhabricatorProjectIconSet.php

Phabricator project icon names are should also be translated.Needs RevisionPublicActions