-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Simplified Chinese localization #7316
Conversation
@YunjieLi, thanks for your PR! By analyzing this pull request, we identified @1ec5, @boundsj and @friedbunny to be potential reviewers. |
A few notes:
|
"COMPASS_A11Y_LABEL" = "指南针"; | ||
|
||
/* Compass abbreviation for north */ | ||
"COMPASS_NORTH" = "N"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This string is the text that is displayed at the top of the compass image. Apple Maps uses 北 for Chinese/Japanese (and I’m likely to do the same when we add a Japanese localization to this SDK) — any objections to us using 北 in Chinese, too?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I personally prefer N for brevity but no objections!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
北 fits and is consistent with MapKit’s Chinese localizations, so let’s go with that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The translations appear to be consistent with CLDR and the Chinese Wikipedia.
Would you do the honors and mention the new Simplified Chinese localization in the relevant section of platform/ios/CHANGELOG.md and platform/macos/CHANGELOG.md?
@@ -1,6 +1,16 @@ | |||
{ | |||
"images" : [ | |||
{ | |||
"idiom" : "iphone", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These changes look unrelated. If they’re needed, would you mind splitting them out into a separate PR? Thanks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did not make that change manually. can I just revert the changes manually?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure.
<key>COORD_DEG_LONG</key> | ||
<dict> | ||
<key>NSStringLocalizedFormatKey</key> | ||
<string>%#@degrees@</string> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since Chinese doesn’t need to pluralize any strings, leave this file out so Chinese speakers don’t see English. COORD_DEG_LONG
etc. are already defined in Localizable.strings.
"FIRST_STEPS_URL" = "mapbox.com/help/first-steps-ios-sdk"; | ||
|
||
/* Accessibility hint */ | ||
"INFO_A11Y_HINT" = "显示致谢,用户反馈及更多"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should the list items be separated by ,
or 、
?
"MAP_A11Y_LABEL" = "地图"; | ||
|
||
/* Map accessibility value */ | ||
"MAP_A11Y_VALUE" = "Zoom %1$dx\n%2$ld annotation(s) visible"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After the user zooms the map, VoiceOver says, for example:
Zoom 2x
12 annotations visible
%1d
is a placeholder for the zoom scale (but x
is pronounced literally in English). \n
is a newline. %2$ld
is a placeholder for the number of visible annotations.
If “annotations” doesn’t translate well, you can try something like “pins”, “markers”, or “map items”.
<key>MAP_A11Y_VALUE</key> | ||
<dict> | ||
<key>NSStringLocalizedFormatKey</key> | ||
<string>Zoom %dx |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Likewise, you can leave this file out too.
"ANNOTATION_A11Y_HINT" = "显示信息"; | ||
|
||
/* No comment provided by engineer. */ | ||
"API_CLIENT_400_DESC" = "The session data task failed. Original request was: %@"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These error messages come from Mapbox Telemetry. In general, error messages should be translated whenever possible. However, in this case, they aren’t ever presented to the user, so it’s fine to leave these messages in English.
"COMPASS_A11Y_LABEL" = "指南针"; | ||
|
||
/* Compass abbreviation for north */ | ||
"COMPASS_NORTH" = "N"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
北 fits and is consistent with MapKit’s Chinese localizations, so let’s go with that.
This PR is targeted at the master branch. It’s probably too late to take this PR for v3.4.0, but I think we could get it into v3.4.1 as long as we rebase the branch and retarget the PR upon the release-ios-v3.4.0 branch. |
@friedbunny @1ec5 just incorporated the feedbacks above. Want to take another look? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
@@ -5,6 +5,7 @@ | |||
* Labels written in Arabic, Hebrew, and other scripts are written right-to-left. Arabic characters are correctly shaped. ([#6984](https://github.com/mapbox/mapbox-gl-native/pull/6984), [#7123](https://github.com/mapbox/mapbox-gl-native/pull/7123)) | |||
* Improved the line wrapping behavior of point-placed labels written in Chinese, Japanese, and Yi. ([#6828](https://github.com/mapbox/mapbox-gl-native/pull/6828)) | |||
* Fixed an issue where translucent point annotations along tile boundaries would be drawn darker than expected. ([#6832](https://github.com/mapbox/mapbox-gl-native/pull/6832)) | |||
* Added the Simplified Chinese localization. ([#7316](https://github.com/mapbox/mapbox-gl-native/pull/7316)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just realized this PR doesn’t actually localize the macOS SDK. It could – you’d add the localization to macos.xcodeproj, reuse platform/darwin/resources/zh-Hans.lproj/Foundation.strings, and add platform/macos/resources/zh-Hans.lproj/Localizable.strings (just 10 easy strings). But we can take care of that in a separate PR. Would you mind removing this changelog entry in the meantime?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
→ #7371
Oh, sorry, we forgot to mention one thing: because this is such a busy repo with so many different platforms, we prefix commits messages with the affected platforms. E.g., Just something to keep in mind for next time. 👍 |
Roger that @friedbunny |
Per chat w/ @1ec5 adding the Chinese localization.