Localize expressions more thoroughly #11651

1ec5 · 2018-04-10T20:41:51Z

Replaced the MGLStyle.localizesLabels property with a -localizeLabelsIntoLocale: method that allows the caller to specify the locale to localize into. This method relies on a new -[NSExpression(MGLAdditions) mgl_expressionLocalizedIntoLocale:] method, for developers who want to vary behavior from layer to layer. (This will simplify mapbox/mapbox-navigation-ios#1076 considerably.)

Due to #11659, this PR also adds some logic to automatically upgrade the legacy {token} syntax to key path expressions for string-typed property getters.

More to come:

Fixes #10713.

/cc @fabian-guerra @anandthakker @bsudekum @nickidlugash @langsmith

fabian-guerra

This PR is looking good.

fabian-guerra · 2018-04-11T21:41:07Z

platform/darwin/src/MGLStyle.mm

+        }
+    }
+    if (!locale) {
+        locale = [NSLocale localeWithLocaleIdentifier:@"mul"];


fabian-guerra · 2018-04-11T21:44:48Z

platform/darwin/src/NSExpression+MGLAdditions.mm

+
+        case NSFunctionExpressionType: {
+            NSExpression *operand = self.operand;
+            NSExpression *localizedOperand = [operand mgl_expressionLocalizedIntoLocale:locale replacingTokens:NO];


According to this:

@param replacesTokens YES to localize {token} syntax in constant value subexpressions; NO otherwise.

NSExpression *localizedOperand = [operand mgl_expressionLocalizedIntoLocale:locale replacingTokens:NO]; overrides the value passed in replacesTokens. What would happen if the function expression contains a constant value? should not this be behave accordingly?

Ah, this deserves clarification: the replacesTokens parameter is only meant to replace the legacy token syntax, but that’s a subset of the syntax that we want to localize. The only reason that the replacesTokens functionality is supported at all in functions is that three functions in particular may represent style functions, which can contain tokens. Nothing else in the expression language can contain tokens.

Having said all that, #11659 means that tokens are only supported for the MGLSymbolStyleLayer.text and iconImageName properties’ getters, not their setters. So as long as we come up with an expression to set, we need to upgrade all the tokens within the expression to key path expressions. 023624360aa37c4d984b3f49484c8d3552b2b414 does that, but only as a post-localization step on individual function calls.

We may end up needing to unconditionally upgrade the entire expression upfront, before even looking at whether to localize anything. If we do that, then the separate replacesTokens functionality will no longer be necessary.

f3efae542788ff0978a3ba833a0b52a8e20e5af1 removes the replacesTokens parameter outright. Instead, token upgrading happens as part of the property’s getter, which limits the scope and makes it easy to remove later on.

1ec5 · 2018-04-13T00:44:01Z

platform/darwin/src/MGLVectorTileSource.mm

@@ -96,7 +96,7 @@ + (NSString *)preferredMapboxStreetsLanguage {
            mostSpecificLanguage = language;
        }
    }
-    return mostSpecificLanguage ?: @"en";
+    return mostSpecificLanguage;


This fallback turned out to be redundant: preferredLanguages is never empty, so mostSpecificLanguage is never nil.

+[NSBundle preferredLocalizationsFromArray:forPreferences:] never returns an empty array of languages, because it always falls back to English if none of the available locales matches one of the preferred locales. To maximize the flexibility of the label localization functionality, I’ve been thinking it would be better if we fall back to name rather than name_en. That way, for instance, if a user prefers Hebrew – which is supported by the map SDK’s UI – they can at least get Hebrew labels in Israel rather than English everywhere.

The trick will be to distinguish between the case where +[NSBundle preferredLocalizationsFromArray:forPreferences:] returns English because the locale really does match English (en, en-US, eng-AU, etc.) from the case where it chose English as a last resort. I’m open to doing this as a followup task, potentially in a patch release, unless a solution comes to mind.

/cc @friedbunny @nickidlugash @langsmith

fabian-guerra

Tests LGTM 👍

anandthakker · 2018-04-13T11:14:25Z

platform/darwin/src/NSExpression+MGLAdditions.mm

+                }
+            }
+            return self;
+        }


Note that {Camera,Source,Composite}Function now track whether or not they were constructed directly from an expression (as opposed to being converted from a stop function):

mapbox-gl-native/include/mbgl/style/function/source_function.hpp

Line 70 in 582d0e5

bool isExpression;

It's likely not worth the extra complexity, but just noting that if needed, we could try to propagate that metadata to the NSExpression representation for an expression so that you could only do this replacement for those that originally came from stop functions.

I think fixing #11659 would be more effective than trying to make the workaround for it more robust, but it’s good to know, for future reference, that mbgl is capable of persisting these distinctions.

1ec5 · 2018-04-13T17:30:43Z

This PR drops the ability to “delocalize” a style, which was previously possible by setting MGLStyle.localizesLabels back to NO. The reason is twofold:

localizesLabels was actually something of a three-way option masquerading as a two-way flag. NO can either mean “not yet localized” or “delocalized”. Depending on the initial value in the style JSON, that might mean two different label languages.
Delocalization would interfere with other runtime styling changes: Automatic label localization should transform non-constant expressions #10713 (comment)

If the developer needs to delocalize a style, they can take either of the following approaches:

Call -[MGLStyle localizeLabelsIntoLocale:] again, this time passing in the original locale.
Call -[MGLMapView reloadStyle:] (which blows away any runtime styling changes).

fabian-guerra

👍🏼

The broader feature is new to v4.0.0 as well.

Replaced the MGLStyle.localizesLabels property with a -localizeLabelsIntoLocale: method that allows the caller to specify the locale to localize into. Also exposed a per-expression localization method for developers who want to vary behavior from layer to layer.

Separated token upgrading into a separate process that only happens as part of the MGLSymbolStyleLayer.text and MGLSymbolStyleLayer.iconImageName properties’ getters, so that it’s easy to remove later when mbgl changes obviate this workaround. Removed the replacesTokens parameter to the expression localization methods.

Fixed a build warning.

For consistency, replace tokens with key paths in all string-typed style paint and layout properties.

Added tests for replacement of tokens with key paths in expressions. Fixed token replacement for raw strings in stop dictionaries. Avoid sticking a single string inside an mgl_join: call.

Added unit tests of token replacement and localization of expressions. Only NSExpression is responsible for resolving the preferred language now, since NSLocale tends to tack a region code onto the locale identifier and the NSExpression method can be called independently anyways. Added a private variation of +[MGLVectorTileSource preferredMapboxStreetsLanguage] that takes an array of preferred languages. Fixed localization of non-expressions in stop dictionaries.

1ec5 added iOS Mapbox Maps SDK for iOS refactor macOS Mapbox Maps SDK for macOS localization Human language support and internationalization labels Apr 10, 2018

1ec5 added this to the ios-v4.0.0 milestone Apr 10, 2018

1ec5 self-assigned this Apr 10, 2018

1ec5 requested a review from fabian-guerra April 10, 2018 20:41

1ec5 mentioned this pull request Apr 11, 2018

Serialize tokens in expressions as get expressions #11659

Closed

fabian-guerra reviewed Apr 11, 2018

View reviewed changes

1ec5 force-pushed the 1ec5-l10n-e8n-10713 branch from 38a53e7 to 0de40b7 Compare April 12, 2018 19:36

1ec5 commented Apr 13, 2018

View reviewed changes

fabian-guerra reviewed Apr 13, 2018

View reviewed changes

anandthakker reviewed Apr 13, 2018

View reviewed changes

1ec5 added the ✓ ready for review label Apr 16, 2018

fabian-guerra approved these changes Apr 16, 2018

View reviewed changes

1ec5 added 14 commits April 16, 2018 14:46

[ios] Removed changelog entry for Terrarium

39525a3

The broader feature is new to v4.0.0 as well.

[macos] Offer English labels if preferred language is unsupported

f974aa6

[ios, macos] Removed dead code

cd1a875

[ios] Use new localization method in iosapp

e8e188c

[ios, macos] Fixed local name labels

b9f0459

[ios, macos] Convert tokens to key path expressions in stop dictionaries

18d08bb

[ios, macos] Preserve whitespace between tokens

dee5d8f

[ios, macos] Moved token replacement to a consistent category

ded3e1e

Fixed a build warning.

[ios, macos] Replace tokens in all string-typed getters

e84dfa2

For consistency, replace tokens with key paths in all string-typed style paint and layout properties.

[ios, macos] Test token replacement

4f2367a

Added tests for replacement of tokens with key paths in expressions. Fixed token replacement for raw strings in stop dictionaries. Avoid sticking a single string inside an mgl_join: call.

[ios, macos] Dictionary keys aren’t necessarily zoom levels

6c87574

1ec5 force-pushed the 1ec5-l10n-e8n-10713 branch from 501dbf7 to 6c87574 Compare April 16, 2018 21:47

1ec5 removed the ✓ ready for review label Apr 16, 2018

1ec5 merged commit e16ce7d into release-boba Apr 16, 2018

1ec5 deleted the 1ec5-l10n-e8n-10713 branch April 16, 2018 22:24

This was referenced Apr 16, 2018

Automatic label localization should transform non-constant expressions #10713

Closed

Update to Maps SDK for iOS v4.0.0 mapbox/mapbox-navigation-ios#1076

Merged

friedbunny mentioned this pull request Apr 17, 2018

[ios] Provide MGLStyle.localizesLabels unavailability notice #11717

Merged

1ec5 mentioned this pull request Apr 26, 2018

Localize expressions mapbox/mapbox-gl-language#20

Open

This was referenced May 14, 2018

Label localization prioritizes other languages over English despite user preference #11906

Closed

Fix English priority during label localization #11907

Merged

1ec5 mentioned this pull request Jul 18, 2018

[ios, macos] Adapt Mapbox Streets–sourced layers for Dynamic Type. #9734

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Localize expressions more thoroughly #11651

Localize expressions more thoroughly #11651

1ec5 commented Apr 10, 2018 •

edited

Loading

fabian-guerra left a comment

fabian-guerra Apr 11, 2018

fabian-guerra Apr 11, 2018

1ec5 Apr 11, 2018

1ec5 Apr 12, 2018

1ec5 Apr 13, 2018 •

edited

Loading

fabian-guerra left a comment

anandthakker Apr 13, 2018

1ec5 Apr 13, 2018

1ec5 commented Apr 13, 2018

fabian-guerra left a comment

Localize expressions more thoroughly #11651

Localize expressions more thoroughly #11651

Conversation

1ec5 commented Apr 10, 2018 • edited Loading

fabian-guerra left a comment

Choose a reason for hiding this comment

fabian-guerra Apr 11, 2018

Choose a reason for hiding this comment

fabian-guerra Apr 11, 2018

Choose a reason for hiding this comment

1ec5 Apr 11, 2018

Choose a reason for hiding this comment

1ec5 Apr 12, 2018

Choose a reason for hiding this comment

1ec5 Apr 13, 2018 • edited Loading

Choose a reason for hiding this comment

fabian-guerra left a comment

Choose a reason for hiding this comment

anandthakker Apr 13, 2018

Choose a reason for hiding this comment

1ec5 Apr 13, 2018

Choose a reason for hiding this comment

1ec5 commented Apr 13, 2018

fabian-guerra left a comment

Choose a reason for hiding this comment

1ec5 commented Apr 10, 2018 •

edited

Loading

1ec5 Apr 13, 2018 •

edited

Loading