[localization] Add better support for Chinese languages (fix #877)#878
[localization] Add better support for Chinese languages (fix #877)#878LukasPaczos merged 3 commits intomapbox:masterfrom
Conversation
LukasPaczos
left a comment
LukasPaczos
left a comment
There was a problem hiding this comment.
Thanks for the contribution!
What do you think about adding various Chinese languages statically during MapLocale initialization with the most precise scenario, which would be the streets-v8 branch? This way, we could check if the provided language will work with the current source when setting the language, for example when setting the name_zh-Hant for streets-v6 we'd convert it to name_zh. Similarly to what we have in place here, but reversed. This approach would also work regardless of the style changes in the runtime (that might introduce new sources).
Also, CI is failing because of the inlined variables:
> Task :plugin-localization:checkstyle FAILED
[ant:checkstyle] [ERROR] /root/code/plugin-localization/src/main/java/com/mapbox/mapboxsdk/plugins/localization/LocalizationPlugin.java:143:5: Each variable declaration must be in its own statement. [MultipleVariableDeclarations]
[ant:checkstyle] [ERROR] /root/code/plugin-localization/src/main/java/com/mapbox/mapboxsdk/plugins/localization/LocalizationPlugin.java:144:5: Each variable declaration must be in its own statement. [MultipleVariableDeclarations]
| if (!mapLanguage.equals(MapLocale.ENGLISH)) { | ||
| if (mapLanguage.equals(MapLocale.CHINESE)) { | ||
| // in streets v8 tiles chinese is declared as "name_zh-Hant" instead of "name_zh" | ||
| mapLanguage = MapLocale.CHINESE_V8; |
There was a problem hiding this comment.
This is still needed, otherwise, we'll break the compatibility of MapLocale.CHINESE with streets-v8 sources.
There was a problem hiding this comment.
This can be ignored if the proposed solution is implemented.
There was a problem hiding this comment.
With the current implementation, this is still needed.
|
@LukasPaczos Hi. I think your suggestion is reasonable and have made the changes you requested. Also rebased this PR to current master. |
Locale PRC is the same as Locale CHINA as they are both aliases for Locale SIMPLIFIED_CHINESE. Changed Locale PRC to Locale TAIWAN so we can support Traditional Chinese locale. Traditional Chinese locale is supported in mapbox-streets-v8 as name_zh-Hant.
Different mapbox-streets source versions have different levels/types of support for Chinese languages. Android system's Chinese Locales also changed in different android versions. Add support for above conditions.
LukasPaczos
left a comment
LukasPaczos
left a comment
There was a problem hiding this comment.
Looks great, thanks for addressing the comments. Last 2 below and we should be ready to merge.
| if (!mapLanguage.equals(MapLocale.ENGLISH)) { | ||
| if (mapLanguage.equals(MapLocale.CHINESE)) { | ||
| // in streets v8 tiles chinese is declared as "name_zh-Hant" instead of "name_zh" | ||
| mapLanguage = MapLocale.CHINESE_V8; |
There was a problem hiding this comment.
With the current implementation, this is still needed.
| for (Source source : style.getSources()) { | ||
| if (sourceIsFromMapbox(source)) { | ||
| boolean isStreetsV8 = sourceIsStreetsV8(source); | ||
| boolean isStreetsV7 = sourceIsStreetsV7(source); |
There was a problem hiding this comment.
This check can be moved to line 242, otherwise, it's going to be redundant for streets-v8 sources.
There was a problem hiding this comment.
I have made above two changes(name_zh should be changed to name_zh_Hans in streets v8, as they both represent Simplified Chinese). I also changed Locale.CHINA's default MapLocale to MapLocale.CHINESE_HANS, since in streets v8 Simplified Chinese should use name_zh_Hans.
3 participants
This PR aims to add better support for Chinese languages in localization plugin. With this change the plugin can support more types of Android system Chinese Locales and has better support for mapbox-streets-v8's new name_zh_Hant Traditional Chinese labels.