Skip to content
This repository has been archived by the owner on Aug 8, 2023. It is now read-only.

[iOS] Add fallbacks for name fields #12387

Merged
merged 3 commits into from
Jul 26, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 19 additions & 1 deletion platform/darwin/src/NSExpression+MGLAdditions.mm
Original file line number Diff line number Diff line change
Expand Up @@ -1401,7 +1401,25 @@ - (NSExpression *)mgl_expressionLocalizedIntoLocale:(nullable NSLocale *)locale
localizedKeyPath = [NSString stringWithFormat:@"name_%@", preferredLanguage];
}
}
return [NSExpression expressionForKeyPath:localizedKeyPath];
// If the keypath is `name`, no need to fallback
if ([localizedKeyPath isEqualToString:@"name"]) {
return [NSExpression expressionForKeyPath:localizedKeyPath];
}
// If the keypath is `name_zh-Hans`, fallback to `name_zh` 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
// 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"];
Copy link
Contributor

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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

}
// Mapbox Streets v8 has `name_zh-Hant`, we should fallback to Simplified Chinese if the filed has no value
Copy link
Contributor

Choose a reason for hiding this comment

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

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

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.

if ([localizedKeyPath isEqualToString:@"name_zh-Hant"]) {
return [NSExpression expressionWithFormat:@"mgl_coalesce({%K, %K, %K, %K, %K})",
localizedKeyPath, @"name_zh-Hans", @"name_zh-CN", @"name_zh", @"name"];
}
// Other keypath fallback to `name`
return [NSExpression expressionWithFormat:@"mgl_coalesce({%K, %K})", localizedKeyPath, @"name"];
}
return self;
}
Expand Down
9 changes: 5 additions & 4 deletions platform/darwin/test/MGLExpressionTests.mm
Original file line number Diff line number Diff line change
Expand Up @@ -1013,7 +1013,7 @@ - (void)testLocalization {
}
{
NSExpression *original = [NSExpression expressionForKeyPath:@"name_en"];
NSExpression *expected = original;
NSExpression *expected = [NSExpression expressionWithFormat:@"mgl_coalesce({%K, %K})", @"name_en", @"name"];
XCTAssertEqualObjects([original mgl_expressionLocalizedIntoLocale:nil], expected);
}
{
Expand All @@ -1023,12 +1023,13 @@ - (void)testLocalization {
}
{
NSExpression *original = [NSExpression expressionForKeyPath:@"name_en"];
NSExpression *expected = [NSExpression expressionForKeyPath:@"name_fr"];
NSExpression *expected = [NSExpression expressionWithFormat:@"mgl_coalesce({%K, %K})", @"name_fr", @"name"];
XCTAssertEqualObjects([original mgl_expressionLocalizedIntoLocale:[NSLocale localeWithLocaleIdentifier:@"fr-CA"]], expected);
}
{
NSExpression *original = [NSExpression expressionForKeyPath:@"name_en"];
NSExpression *expected = [NSExpression expressionForKeyPath:@"name_zh-Hans"];
NSExpression *expected = [NSExpression expressionWithFormat:@"mgl_coalesce({%K, %K, %K, %K})",
@"name_zh-Hans", @"name_zh-CN", @"name_zh", @"name"];
XCTAssertEqualObjects([original mgl_expressionLocalizedIntoLocale:[NSLocale localeWithLocaleIdentifier:@"zh-Hans"]], expected);
}
{
Expand All @@ -1045,7 +1046,7 @@ - (void)testLocalization {
NSExpression *expected = [NSExpression expressionWithFormat:@"mgl_step:from:stops:($zoomLevel, short, %@)", @{
@1: [NSExpression expressionForKeyPath:@"abbr"],
@2: @"…",
@3: [NSExpression expressionForKeyPath:@"name_es"],
@3: [NSExpression expressionWithFormat:@"mgl_coalesce({%K, %K})", @"name_es", @"name"]
}];
XCTAssertEqualObjects([original mgl_expressionLocalizedIntoLocale:[NSLocale localeWithLocaleIdentifier:@"es-PR"]], expected);
}
Expand Down
1 change: 1 addition & 0 deletions platform/ios/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ Mapbox welcomes participation and contributions from everyone. Please read [CONT
* Added an `MGLMapView.locationManager` property and `MGLLocationManager` protocol for tracking user location using a custom alternative to `CLLocationManager`. ([#12013](https://github.com/mapbox/mapbox-gl-native/pull/12013))
* Fixed a crash when switching between two styles having layers with the same identifier but different layer types. ([#12432](https://github.com/mapbox/mapbox-gl-native/issues/12432))
* Fixed an issue where the symbols for `MGLMapPointForCoordinate` could not be found. ([#12445](https://github.com/mapbox/mapbox-gl-native/issues/12445))
* 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))

## 4.2.0 - July 18, 2018

Expand Down