[iOS] Add fallbacks for name fields #12387

lloydsheng · 2018-07-16T07:07:53Z

Aiming to fix this issue #12164 and compatible with name fields of Mapbox Streets v8.

1ec5

Thanks, this is a much more elegant approach to implementing v8 support than what I had been working on for #11867. (#11867 covers some additional things like VoiceOver that are unrelated to localization.)

In addition to the feedback below, we should figure out the edge case mentioned in #12164 (comment), that the same expression may be localized more than once. When localizing a function expression, perhaps we check whether the function is mgl_coalesce: and the argument array consists entirely of name_ key path expressions; if so, then we can replace the coalescing function instead of localizing each argument as we normally do.

1ec5 · 2018-07-17T16:31:35Z

platform/darwin/src/NSExpression+MGLAdditions.mm

@@ -1507,7 +1507,23 @@ - (NSExpression *)mgl_expressionLocalizedIntoLocale:(nullable NSLocale *)locale
                        localizedKeyPath = [NSString stringWithFormat:@"name_%@", preferredLanguage];
                    }
                }
-                return [NSExpression expressionForKeyPath:localizedKeyPath];
+                // If the keypath is `name` or `name_en`, no need to fallback
+                if ([localizedKeyPath isEqualToString:@"name"] || [localizedKeyPath isEqualToString:@"name_en"]) {


As this documentation states, name_en can be null in Streets source v8, in which case we need to fall back to name.

1ec5 · 2018-07-17T16:52:06Z

platform/darwin/src/NSExpression+MGLAdditions.mm

+                // If the keypath is `name_zh-Hans`, fallback to `name_zh`. If `name_zh` is empty, fallback to `name`
+                // The `name_zh-Hans` field was added since Mapbox Streets v7
+                // See the documentation of name fields for detail https://www.mapbox.com/vector-tiles/mapbox-streets-v7/#overview
+                if ([localizedKeyPath isEqualToString:@"name_zh-Hans"]) {


Should we also have name_zh-Hant attempt to fall back to name_zh-Hans and vice versa, for Streets source v8?

1ec5 · 2018-07-17T17:09:16Z

platform/darwin/src/NSExpression+MGLAdditions.mm

+                // If the keypath is `name_zh-Hans`, fallback to `name_zh`. If `name_zh` is empty, fallback to `name`
+                // The `name_zh-Hans` field was added since Mapbox Streets v7
+                // See the documentation of name fields for detail https://www.mapbox.com/vector-tiles/mapbox-streets-v7/#overview
+                if ([localizedKeyPath isEqualToString:@"name_zh-Hans"]) {


Apparently the same mapbox.mapbox-streets-v7 tileset on api.mapbox.cn uses name_zh-CN instead of name_zh-Hans: mapbox/mapbox-navigation-ios#1554. Could we address that here by opportunistically falling back to name_zh-CN if name_zh-Hans is nil?

/cc @m-stephen

1ec5 · 2018-07-17T17:10:22Z

platform/ios/CHANGELOG.md

@@ -16,6 +16,7 @@ Mapbox welcomes participation and contributions from everyone. Please read [CONT
 * Added the `collator` and `resolved-locale` expression operators to more precisely compare strings in style JSON. A subset of this functionality is available through predicate options when creating an `NSPredicate`. ([#11869](https://github.com/mapbox/mapbox-gl-native/pull/11869))
 * Fixed a crash when trying to parse expressions containing legacy filters. ([#12263](https://github.com/mapbox/mapbox-gl-native/pull/12263))
 * Fixed a crash that occurred when creating an `MGL_MATCH` expression using non-expressions as arguments. ([#12332](https://github.com/mapbox/mapbox-gl-native/pull/12332))
+* Fixed an issue that no localized low-zoom labels when system language is Simplified Chinese ([#12164](https://github.com/mapbox/mapbox-gl-native/issues/12164))


Fixed an issue causing country and ocean labels to disappear after calling -[MGLStyle localizeLabelsIntoLocale:] when the system language is set to Simplified Chinese.

1ec5 · 2018-07-17T17:11:53Z

platform/darwin/src/NSExpression+MGLAdditions.mm

+                    return [NSExpression expressionWithFormat:@"mgl_coalesce(%@)",
+                            @[[NSExpression expressionForKeyPath:localizedKeyPath],
+                              [NSExpression expressionForKeyPath:@"name_zh"],
+                              [NSExpression expressionForKeyPath:@"name"],]];


This can be written more succinctly as:

[NSExpression expressionWithFormat:@"mgl_coalesce({%K, name_zh, name})", localizedKeyPath]

1ec5 · 2018-07-24T14:42:00Z

platform/darwin/src/NSExpression+MGLAdditions.mm

+                // CN tiles might using `name_zh-CN` for Simplified Chinese
+                if ([localizedKeyPath isEqualToString:@"name_zh-Hans"]) {
+                    return [NSExpression expressionWithFormat:@"mgl_coalesce({%K, %K, %K, %K})",
+                            localizedKeyPath, @"name_zh-CN", @"name_zh", @"name"];


Of these four key paths, only name_zh-CN really needs to be escaped using %K, due to the hyphen. But this is better because it preserves the correct fallback order in the code.

A more generic approach would be to build up an array of key path expressions, then create a coalescing expression in only one place. Something like this:

NSArray *keyPaths; if (…) { keyPaths = @[localizedKeyPath, @"name_zh-CN", @"name_zh", @"name"]; } else if (…) { … } NSMutableArray *keyPathExpressions = [NSMutableArray array]; for (NSString *keyPath in keyPaths) { [keyPathExpressions addObject:[NSExpression expressionForKeyPath:keyPath]]; } return [NSExpression expressionWithFormat:@"mgl_coalesce(%@)", keyPathExpressions];

This might make it easier to address the edge case in #12387 (review) with the solution in #12164 (comment):

[keyPathExpressions addObject: [NSExpression expressionForKeyPath:@"com.mapbox.expressions.localized"]];

Given the complexity of handling the relocalization case, we can address it in a subsequent PR if you think it’s out of scope for this PR.

@1ec5 Good suggestion, but I think it might happen in another PR.

1ec5 · 2018-07-24T14:43:34Z

platform/ios/CHANGELOG.md

@@ -16,6 +16,7 @@ Mapbox welcomes participation and contributions from everyone. Please read [CONT
 * Added the `collator` and `resolved-locale` expression operators to more precisely compare strings in style JSON. A subset of this functionality is available through predicate options when creating an `NSPredicate`. ([#11869](https://github.com/mapbox/mapbox-gl-native/pull/11869))
 * Fixed a crash when trying to parse expressions containing legacy filters. ([#12263](https://github.com/mapbox/mapbox-gl-native/pull/12263))
 * Fixed a crash that occurred when creating an `MGL_MATCH` expression using non-expressions as arguments. ([#12332](https://github.com/mapbox/mapbox-gl-native/pull/12332))
+* Fixed an issue causing country and ocean labels to disappear after calling -[MGLStyle localizeLabelsIntoLocale:] when the system language is set to Simplified Chinese. ([#12164](https://github.com/mapbox/mapbox-gl-native/issues/12164))


Surround -[MGLStyle localizeLabelsIntoLocale:] in backticks so that jazzy will automatically link it to the method’s documentation.

1ec5 · 2018-07-24T15:36:20Z

platform/darwin/src/NSExpression+MGLAdditions.mm

+                    return [NSExpression expressionWithFormat:@"mgl_coalesce({%K, %K, %K, %K})",
+                            localizedKeyPath, @"name_zh-CN", @"name_zh", @"name"];
+                }
+                // Mapbox Streets v8 has `name_zh-Hant`, we should fallback to Simplified Chinese if the filed has no value


Note that label localization is currently disabled for Streets source v8 because this code only recognizes v6 and v7:

mapbox-gl-native/platform/darwin/src/MGLVectorTileSource.mm

Line 144 in cb714d5

return [identifiers containsObject:@"mapbox.mapbox-streets-v7"] || [identifiers containsObject:@"mapbox.mapbox-streets-v6"];

But this fallback is good to have in preparation for #11867 anyhow.

lloydsheng added the ⚠️ DO NOT MERGE Work in progress, proof of concept, or on hold label Jul 16, 2018

lloydsheng mentioned this pull request Jul 16, 2018

No localized low-zoom labels when system language is Simplified Chinese #12164

Closed

1ec5 reviewed Jul 17, 2018

View reviewed changes

This was referenced Jul 17, 2018

Update MGLVectorTileSource for Mapbox Streets v8 compatibility #11867

Closed

Road and POI labels text cannot display when switch endpoint to .cn mapbox/mapbox-navigation-ios#1554

Closed

lloydsheng removed the ⚠️ DO NOT MERGE Work in progress, proof of concept, or on hold label Jul 18, 2018

lloydsheng added this to the ios-v4.2.0 milestone Jul 18, 2018

lloydsheng changed the title ~~[WIP] Add fallbacks for name fields~~ [iOS] Add fallbacks for name fields Jul 18, 2018

1ec5 modified the milestones: ios-v4.2.0, ios-v4.3.0 Jul 18, 2018

friedbunny added iOS Mapbox Maps SDK for iOS localization Human language support and internationalization labels Jul 19, 2018

friedbunny assigned lloydsheng Jul 19, 2018

1ec5 approved these changes Jul 24, 2018

View reviewed changes

lloydsheng force-pushed the lloyd-label-localization branch from b11c139 to 70cac0e Compare July 26, 2018 03:56

lloydsheng added 2 commits July 26, 2018 13:16

Add fallbacks for name fields

d3223e1

Add a fallback for name_zh-Hant

601f0b7

lloydsheng force-pushed the lloyd-label-localization branch from 70cac0e to 601f0b7 Compare July 26, 2018 05:19

Update changelog

3eba49d

lloydsheng merged commit 9c90509 into master Jul 26, 2018

jfirebaugh deleted the lloyd-label-localization branch July 27, 2018 22:45

1ec5 mentioned this pull request Aug 22, 2018

Localization plugin should transform expressions mapbox/mapbox-plugins-android#374

Closed

1ec5 mentioned this pull request Dec 7, 2018

Update automatic Streets localization for v8/v11 data/styles #13481

Merged

1ec5 mentioned this pull request Feb 21, 2020

Update label languages for Mapbox Streets v8.3 mapbox/mapbox-gl-native-ios#173

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[iOS] Add fallbacks for name fields #12387

[iOS] Add fallbacks for name fields #12387

lloydsheng commented Jul 16, 2018 •

edited

Loading

1ec5 left a comment

1ec5 Jul 17, 2018

1ec5 Jul 17, 2018

1ec5 Jul 17, 2018

1ec5 Jul 17, 2018

1ec5 Jul 17, 2018

1ec5 Jul 24, 2018

lloydsheng Jul 26, 2018

1ec5 Jul 24, 2018

1ec5 Jul 24, 2018

[iOS] Add fallbacks for name fields #12387

[iOS] Add fallbacks for name fields #12387

Conversation

lloydsheng commented Jul 16, 2018 • edited Loading

1ec5 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lloydsheng commented Jul 16, 2018 •

edited

Loading